Re: [PATCH/RFC] rebase -p: do not redo the merge, but cherry-pick first-parent changes

From: Johannes Sixt <j6t@kdbg.org>
Date: May 24, 2012, 20:32 UTC
Message-ID: <4FBE9AC7.3010506@kdbg.org>
In-Reply-To: <CAOeW2eH85+qa2PXS55_xGwH+tpMDMEK76HywfpLTYrv_Dtg49Q@mail.gmail.com>

Am 24.05.2012 19:47, schrieb Martin von Zweigbergk:

Show 13 quoted lines

> On Thu, May 24, 2012 at 10:31 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:
>>
>>> Yes, I've had the same idea myself. Anyway, as Johannes said, that's
>>> probably something to consider for the sequencer.
>>
>> Are you saying that "rebase -any-variant" and the sequencer should behave
>> differently?  It is not immediately obvious to me why it is a good idea.
> 
> That's not what I meant to say. I thought the sequencer is supposed to
> replace much of git-rebase and I thought that's what Johannes was
> referring to as well when he said not to make git-rebase too
> intelligent.

You are probably refering to what I said here:

http://thread.gmane.org/gmane.comp.version-control.git/194434/focus=195074

When I wrote the post, I was not aware that rebase -p *is* indeed able to transplant a branchy topic to a new upstream. I was convinced that rebase -p can only move a (first-parent) topic line which may have merged in some unrelated other topics. So, you should take it with a large grain of salt.

-------

Today I was able to use rebase -i -p in the field. I used it to rebuild an integration branch (akin to git's pu branch). Guess what? It did not work as expected:

Two of the topic branches' early parts were already merged in the upstream. The instruction sheet had only 'pick' of merge commits for the topics. Except for these two; there, all commits (that were not yet in upstream) were offered to pick, including the merge commit.

I started with this:

    A--M--o--o   <- master
   /  /
--o--X--Y        <- side branch (partially merged in master)
   \     \
    R--S--N--T   <- integration (to be rebuilt on master)

I wanted this:

     A--M--o--o
    /  /       \
   /  /          R'--S'--N'--T'
--o--X--Y---------------´

But I got this:

    A--M--o--o-------Y'
   /  /       \       \
--o--X--Y      R'--S'--N'--T'

(Note that this has nothing to do with my patch; the badness happens already before any rebasing begins.)

Gah! I'm frustrated. When --preserve-merges was invented, it supported two very important use-cases:

1. Rebuild an integration branch.
2. Rebase a topic that merges an 'unrelated side branch'.

Then people came along thinking that "preserve merges" means that *any* sort of merges should be preserved, including a branchy-and-mergy topic like the example you gave. *Of course* it is much more difficult to support this case. And sure as hell with all the work-arounds needed to support it, a good deal of other good functionality became broken subtly. This is why I say that we should drop support for the complicated cases and resurrect correctness for the simpler, but important cases.

-- Hannes