On Wed, 10 Oct 2018 14:43:46 +0900, Junio wrote:
> We haven't seen much complaints and breakages reported against the
> two big "rewrite in C" topics around "rebase"; perhaps it is a good
> time to merge them to 'next' soonish to cook them for a few weeks
> before moving them to 'master'?
In my opinion, the `--rebase-merges' feature has been broken since the
beginning, and the builtin version should be fixed before it is moved
ahead. In short: "labels" are brittle; see below for tests.
Also, here are some quick *additional* thoughts:
* Labels should be simply "r0", "r1", ... "rN".
* The current, long label names are just cumbersome.
* The embedded comments are already more than enough.
* "r" is short for "revision" or "reset" or "remember", etc.
* "r" is located on a QWERTY keyboard such that it's very
easy to type "rN", where "N" is a number.
* Why is the command "label" and not "branch"? Every other related
command looks like a normal git command: "reset" and "merge".
Make it "branch".
* In my experience, there's a lot of this boiler plate:
pick 12345
label r1
reset r0
merge r1
How about instead, use git's existing ideas:
pick 12345
reset r0
merge ORIG_HEAD
Or, maybe git in general should treat `-' as `ORIG_HEAD' (which
would be similar to how `git checkout' understands `-'), thereby
allowing a very quick idiomatic string of commands:
pick 12345
reset r0
merge -
In truth, I don't really know the semantics of `ORIG_HEAD', so
maybe those should be nailed down and documented more clearly;
I would like it to work as in the following:
pick 12345
# label r1 (pretend)
reset r0 # Store r1 in ORIG_HEAD
pick 67890 # Do NOT touch ORIG_HEAD
merge - # Same as merge -C abcde r1
Anyway, this kind of unspoken behavior would make *writing* a
new history by hand much more pleasant.
* Why not just `--merges' instead of `--rebase-merges'? Or, better
yet, just make it the default behavior; the special option
should instead be:
--flatten
This option would simply tell `git rebase' to prepare an initial
todo list without merges.
Thanks for this great feature.
I'm only complaining so much because it's such a useful feature, and I
want it to be even better, because I'll probably use it A LOT; it
should have been available since the start as a natural consequence of
the way git works.
Sincerely,
Michael Witten
---------------
Unfortunately, both the legacy version and the rewrite of
`--rebase-merges' display a bug that makes this feature fairly
unusable in practice; it tries to create a "label" (i.e., a branch
name) from a commit log summary line, and the result is often invalid
(or just plain irritating to work with). In particular, it fails on
typical characters, including at least these:
:/\?.*[]
To see this, first define some POSIX shell functions:
test()
{
(
set -e
summary=$1
d=/tmp/repo ##### WARNING. CHANGE IF NECESSARY.
rm -rf "$d"; mkdir -p "$d"; cd "$d"
git init -q
echo a > a; git add a; git commit -q -m a
git branch base
echo b > b; git add b; git commit -q -m b
git reset -q --hard HEAD^
git merge -q --no-ff -m "$summary" ORIG_HEAD
git log --graph --oneline
git rebase --rebase-merges base
); status=$?
echo
return "$status"
}
Test()
{
if test "$@" 1>/dev/null 2>&1; then
echo ' good'; return 0
else
echo ' fail'; return 1
fi
}
Then, try various commit summaries (see below for results):
test c
test 'combine these into a merge: a and b'
Test ab:
Test a:b
Test :
Test a/b
Test 'Now supports /regex/'
Test ab/
Test /ab
Test /
Test 'a\b'
Test '\'
Test 'Maybe this works?'
Test '?'
Test 'This does not work.'
Test 'This works. Strange!'
Test .git
Test .
Test 'Cast each pointer to *void'
Test '*'
Test 'return a[1] not a[0]'
Test '[ does not work'
Test '['
Test '] does work'
Test ']'
Here are the results of pasting the above commands into my terminal:
$ test c
warning: templates not found in ../install/share/git-core/templates
* 1992d07 (HEAD -> master) c
|\
| * 34555b5 b
|/
* 338db9b (base) a
Successfully rebased and updated refs/heads/master.
$ test 'combine these into a merge: a and b'
warning: templates not found in ../install/share/git-core/templates
* 4202c49 (HEAD -> master) combine these into a merge: a and b
|\
| * 34555b5 b
|/
* 338db9b (base) a
error: refusing to update ref with bad name 'refs/rewritten/combine-these-into-a-merge:-a-and-b'
hint: Could not execute the todo command
hint:
hint: label combine-these-into-a-merge:-a-and-b
hint:
hint: It has been rescheduled; To edit the command before continuing, please
hint: edit the todo list first:
hint:
hint: git rebase --edit-todo
hint: git rebase --continue
$ Test ab:
fail
$ Test a:b
fail
$ Test :
fail
$ Test a/b
good
$ Test 'Now supports /regex/'
fail
$ Test ab/
fail
$ Test /ab
fail
$ Test /
fail
$ Test 'a\b'
fail
$ Test '\'
fail
$ Test 'Maybe this works?'
fail
$ Test '?'
fail
$ Test 'This does not work.'
fail
$ Test 'This works. Strange!'
good
$ Test .git
fail
$ Test .
fail
$ Test 'Cast each pointer to *void'
fail
$ Test '*'
fail
$ Test 'return a[1] not a[0]'
fail
$ Test '[ does not work'
fail
$ Test '['
fail
$ Test '] does work'
good
$ Test ']'
good
