`--rebase-merges' still failing badly

[Michael Witten <mfwitten@gmail.com>]

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