Re: Inconsistancy with `git rebase --preserve-merges`

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 24, 2020, 21:35 UTC
Message-ID: <xmqqlforoegw.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20200224183648.GF1247035@alpha>

Kevin Daudt <me@ikke.info> writes:

Show 5 quoted lines

>> $ git rebase --preserve-merges -i 412f07a~
>
> Can you try `--rebase-merges` instead? Since v2.22.0 `--preseve-merges`
> is officially deprecated, but even before that it was already known to
> have flaws.

The options to "rebase" command seem to follow the usual "last one wins" pattern. There is a single opts->type field, and upon seeing --preserve-merges, the field gets assigned REBASE_PRESERVE_MERGES, and then when the command line option parser sees "-i", the field gets overwritten by assigning REBASE_INTERACTIVE. So

    git rebase --preserve-merges -i 412f07a~
    git rebase -i 412f07a~

both should do the same thing.

I suspect that "--rebase-merges -i" may hide the real issue (which is "the command line option parsing of rebase is messy"), but it may deserve to be cleaned up, now that the result of "rewrite it in C" efforts seems to have sufficiently stablized.

What to clean-up?  There could be two-and-half ways to view it:

 * The options should follow the usual "last one wins" pattern; if
   we take this stance, what happens with "--rebase-merges -i" is
   buggy and "--preserve-merges -i" that behaves as a mere "-i" is
   doing the right thing.  The part of the command line parser that
   implements "--rebase-merges" should be fixed so that its effect
   gets reverted when "-i" is seen later.

 * The options "--rebase-merges" and "--perserve-merges" (there may
   be others) and "--interactive" should be mutually exclusive; if
   we take this stance, we should error out when we see two or more
   of them on the command line.

 * The options "--rebase-merges" and "--preserve-merges" (there may
   be others) should be mutually exclusive or "last one wins", but
   "--interactive" can be combined with them to make them
   interactive.

I am not sure which one is the best. The impression I got from the current state of the code is that it started from "last one wins", and the code wanted to transition to "--interactive makes other options go interactive" but hasn't done a good job by leaving loose ends (like what we saw with "--preserve-merges"), so perhaps the last one is what we want to aim as a long term solution? I dunno.