Re: [PATCH] pull: conflict hint pull.rebase suggestion should offer "merges" vs "true"

On Sat, Feb 18, 2023 at 5:39 PM Phillip Wood [off-list ref] wrote:

On 18/02/2023 03:17, Elijah Newren wrote:

quoted

One concern I have is that "--rebase-merges" itself has negative user
surprises in store.  In particular, "--rebase-merges", despite its
name, does not rebase merges.  It uses the existing author & commit
message info, but otherwise just discards the existing merge and
creates a new one.  Any information it contained about fixing
conflicts, or making adjustments to make the two branches work
together, is summarily and silently discarded.

That's a good point. Another potentially surprising behavior is that
when I'm rebasing an integration branch with -rno-rebase-cousins then if
one of the topic branches merged into the integration branch happens to
share the same base as the integration branch itself the topic branch
gets rebased as well.

I've been trying to understand how this behavior is (potentially)
surprising - I imagine it's been discussed elsewhere but I'm having a
hard time understanding, sorry.

The situation you described is a boundary condition between two others, right?
* The topic branch could be branched from the integration branch
(potentially *after* some other change were made to the integration
branch, but not in this case) - in which case rebasing is what you
would expect
* The topic branch could be branched from the main branch (potentially
*before* the integration branch branched, but not in this case) - in
which case not rebasing is what you would expect.

If topic branched from main (at around the same time as integration),
it might be surprising that it rebases; if it branched from
integration (before that had any changes), then it is expected.

-rno-rebase-cousins is also slower that it needs
to be because it creates a todo list that contains all the commits on
the topic branches merged into the integration branch rather than just
the merges. The commits on the topic branches are fast-forwarded rather
than rewritten so long as they don't share the same base as the
integration branch but it noticeably slower than using a todo list with
just the merge commands.

This seems improvable, but no worse than a plain legacy rebase (as
Alex's new patch would have it, "rebase-merges=drop"), right? Insofar
as we're discussing why it might make sense to avoid promoting this
over a plain rebase, I don't understand the concern.

quoted

My personal opinion would be adding such a capability should be step
2.5 in your list, though I suspect that would make Tao unhappy (it's a
non-trivial amount of work, unlike the other steps in your list).

I've got a couple of patches[1] that cherry-pick the merge if only one
of the parents has changed. I've never tried upstreaming them as it is
only a partial solution to the problem of rebasing merges but that
approach should work well with "git pull --rebase=merges" as only the
upstream side will have changed (when rebasing my git integration branch
with that patch the merges are cherry-picked). They might make a useful
starting point if anyone wants to try and improve the rebasing of merges.

This is awesome!

It feels like the first step towards the general strategy that was (I
believe) best described by Buga at
https://public-inbox.org/git/a0cc88d2-bfed-ce7b-1b3f-3c447d2b32da@gmail.com/
!

(unless I'm missing something, the result of this is exactly the same
as the result of that strategy, in these "simple" cases where it kicks
in)

The one concern I have with this is that, *if I understand correctly*,
it sometimes throws away the existing merge information, and sometimes
doesn't, and there's no easy way to know which it is at runtime. Would
adding a warning on stderr when a both-parents merge is encountered
(and any merge resolutions or related changes are still discarded) be
enough to make this shippable?

Are there *any* circumstances where the new cherry-picking behavior
introduced here wouldn't be the right thing to have happen?