Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

From: Igor Djordjevic <igor.d.djordjevic@gmail.com>
Date: Mar 8, 2018, 15:16 UTC
Message-ID: <2749ce78-8917-c821-6116-0c8d67b5e16e@gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1803070810550.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

Hi Dscho,

On 07/03/2018 08:26, Johannes Schindelin wrote:

Show 24 quoted lines

> 
> > So, it could be something like:
> >
> > 	merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed
> 
> I like where this is heading, too, but I do not think that we can do this
> on a per-MERGE_HEAD basis. The vast majority of merge commits, in
> practice, have two parents. So the `merge` command would actually only
> have one revision to merge (because HEAD is the implicit first parent). So
> that is easy.
> 
> But as soon as you go octopus, you can either perform an octopus merge, or
> rebase the original merge commit. You cannot really mix and match here.
> 
> Unless we reimplement the octopus merge (which works quite a bit
> differently from the "rebase merge commit" strategy, even if it is
> incremental, too), which has its own challenges: if there are merge
> conflicts before merging the last MERGE_HEAD, the octopus merge will exit
> with status 2, telling you "Should not be doing an octopus.". While we
> will want to keep merge conflict markers and continue with the "rebase the
> original merge commit" strategy.
> 
> And it would slam the door shut for adding support for *other* merge
> strategies to perform a more-than-two-parents merge.

The thing is, in my opinion, as long as we are _rebasing_, you can`t pick any merge strategy, as it doesn`t really make much sense. If you do want a specific strategy, than that`s _recreating_ a merge, and it goes fine with what you already have for `--recreate-merges`.

On merge rebasing, the underline strategy we decide to use is just an implementation detail, picking the one that works best (or the only one that works, even), user should have nothing to do with it.

Show 5 quoted lines

> Also, I do not think that it makes a whole lot of sense in practice to let
> users edit what will be used for "original parent". If the user wants to
> do complicated stuff, they can already do that, via `exec`. The `merge`
> command really should be about facilitating common workflows, guiding the
> user to what is sane.

I thought of a situation like this:

(1) ---o---o---o---M--- (master)
        \         /
         X1--X2--X3 (topic)

Merge M was done with `-s ours`, obsoleting "topic" branch. But, I later realized that I actually do want that X2 commit in master.

Now, I guess the most obvious approach is just cherry-picking it, but what if I would like to do something like this instead, with an interactive rebase (and rebasing the merge, not recreating it):

(2) ---o---o---o---M'--- (master)
       |\         /|
       | X1'-X3'-/ | (topic)
       |           |
       \--X2'------/ (new)

This way, and having "topic" inherit original merge behavior due to merge rebasing, X1' and X3' would still be missing from M' as they did originally from M, but X2' would now be included, as it`s coming from a new branch, forged during interactive rebase.

(note - implementation wise, this still wouldn`t be an octopus merge ;)

> The vast majority of merge commits, in practice, have two parents. So
> the `merge` command would actually only have one revision to merge
> (because HEAD is the implicit first parent).

Now, this is something I actually overlooked :(

I guess order of parent commits could then be used to map to old commit parents, being a limitation in comparison to direct old-parent:new-parent mapping, but might be a more straightforward user experience...

Though in case of octopus merge, where one would like to drop a branch from the middle, being merged with `-s ours`, that would be impossible, as then the next branch would be taking over dropped branch merge parent, yielding an incorrect result.

So in this case:

(3) ---o---o---o---M--- (master)
       |\         /|
       | X1--X3--/ | (topic)
       |           |
       \--X2-------/ (new)

... where "topic" was merged using `-s ours`, we wouldn`t be able to just remove whole "topic" branch from the rebased merge without influencing it incorrectly.

With (any kind of) explicit old-parent:new-parent mapping, this is possible (and shouldn`t be any harder, implementation wise).

Now, it`s a different story if we`re interested in such exotic scenarios in the first place, but if possible, I would be all for it... :)

Show 13 quoted lines

> Currently my favorite idea is to introduce a new flag: -R (for "rebase the
> original merge commit"). It would look like this:
> 
> 	merge -R -C <original-merge> <merge-head> # <oneline>
> 
> This flag would of course trigger the consistency check (does the number
> of parents of the original merge commit agree with the parameter list? Was
> an original merge commit specified to begin with?), and it would not fall
> back to the recursive merge, but error out if that check failed.
> 
> Side note: I wonder whether we really need to perform the additional check
> that ensures that the <merge-head> refers to the rewritten version of the
> original merge commit's parent.

No, and even worse - I think we must not do that, as that merge parent might be moved elsewhere, which should be allowed.

When I say "merge parent", I really mean "merge parent _branch_" (or so to say, merge parent context), I was deliberately avoiding term "merge parent commit" as it`s more than that.

> Second side note: if we can fast-forward, currently we prefer that, and I
> think we should keep that behavior with -R, too.

I agree.

> If the user wants to force a new merge, they simply remove that -R flag.
> 
> What do you think?

Having more replies in the thread, I`ll comment the format there, might be more appropriate, as discussion already evolved since the last time I`ve checked, and I might have a new idea... ;) :P

Regards, Buga