Re: [PATCH] rebase --preserve-merges keeps empty merge commits

From: Martin von Zweigbergk <martinvonz@gmail.com>
Date: Feb 2, 2013, 08:21 UTC
Message-ID: <CANiSa6gxoHPWO85ZYu5iyHxzpE4HAprpEgG4305_jhyOE0LWGQ@mail.gmail.com>
In-Reply-To: <510C2E10.1050403@cisco.com>

On Fri, Feb 1, 2013 at 1:05 PM, Phil Hord <hordp@cisco.com> wrote:

>
> This is probably right, but it is not exactly the case that caused my itch.
> I think my branch looked like [...]

That also makes sense. I'll add tests for both cases. Your patch makes both of them pass.

Show 15 quoted lines

>> # a---b---c
>> #      \   \
>> #       d   \
>> #        \   \
>> #         e   \
>> #          \   \
>> #           C---l
>>
>> As you say, your patch doesn't try to handle this case, but at least
>> the new behavior seems better. I think we would ideally want the
>> recreated 'l' to have only 'C' as parent in this case. Does that make
>> sense?
>
> This is not what I meant, but it is a very interesting corner case.  I
> am not sure I have a solid opinion on what the result should be here.

Neither do I, so I'll just drop the test case. Thanks.

Show 26 quoted lines

> Here is the corner case I was thinking of.  I did not test this to see
> if this will happen, but I conceived that it might.  Suppose you have
> this tree where
>
> # a---b---c
> #      \
> #       d---g---l
> #        \ /
> #         C
>
> where 'C' introduced the same changes as 'c'.
>
> When I execute 'git rebase -p l c', I expect that I will end up with
> this:
>
> # a---b---c---d---
> #              \  \
> #               ---g---l
>
> That is, 'C' gets skipped because it introduces the same changes already
> seen in 'c'.  So 'g' now has two parents: 'd' and 'C^'.  But 'C^' is 'd',
> so 'g' now has two parents, both of whom are 'd'.
>
> I think it should collapse to this instead:
>
> # a---b---c---d---g---l

I think this is actually what you will get. But I think it will only be linearized if the branch that should be dropped is the second parent. I have two tests for this, but I need to simplify them a little to see that that (parent number) is the only difference.

> I hope this is clear, but please let me know if I made it too confusing.

Very clear. Thanks.