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

[Igor Djordjevic <igor.d.djordjevic@gmail.com>]

Hi Dscho,

On 07/03/2018 08:26, Johannes Schindelin wrote:
> 
> > 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.

> 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... :)

> 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