Re: [PATCH v5 5/7] add tests for rebasing merged history

From: Martin von Zweigbergk <martinvonz@gmail.com>
Date: Jun 5, 2013, 05:44 UTC
Message-ID: <CANiSa6hGasdoeTtBp_DRaeo++LBPXL9D61EtmVLPz-YSGT-Jgw@mail.gmail.com>
In-Reply-To: <7vk3m994mu.fsf@alter.siamese.dyndns.org>

On Tue, Jun 4, 2013 at 10:18 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 24 quoted lines

> Martin von Zweigbergk <martinvonz@gmail.com> writes:
>
>> ---
>> +#TODO: make all flavors of rebase use --topo-order
>> +test_run_rebase success 'e n o' ''
>> +test_run_rebase success 'e n o' -m
>> +test_run_rebase success 'n o e' -i
>
> I do not quite follow this TODO.
>
> While I think it would be nice to update "rebase" so that all
> variants consider replaying the commits in the same order, in this
> history you have:
>
> +# a---b-----------c
> +#      \           \
> +#       d-------e   \
> +#        \       \   \
> +#         n---o---w---v
> +#              \
> +#               z
>
> as long as o comes after n and e is shown before n or after o, which
> all three expected results satisify, it is in --topo-order, isn't it?

True, the TODO was too specific. I intended to get the list of commits to rebase for all kinds of rebase by using 'git rev-list --topo-order', but it doesn't really matter how the order is decided; my goal was just to make it consistent. I'll update the TODOs.

Show 17 quoted lines

>> +test_expect_success "rebase -p re-creates merge from side branch" "
>> +     reset_rebase &&
>> +     git rebase -p z w &&
>> +     test_cmp_rev z HEAD^ &&
>> +     test_cmp_rev w^2 HEAD^2
>> +"
>
> Hmm, turning the left one to the right one?
>
> +#       d-------e               d-------e
> +#        \       \               \       \
> +#         n---o---w     ===>      n---o   \
> +#              \                       \   \
> +#               z                       z---W
>
> If w were a merge of o into e (i.e. w^1 were e not o), what should
> happen?  Would we get the same topology?

Yes, it seems like it does yield the same topology. That seems to be what I tested at first. Search for "wrong" in [1]. I think Johannes's point was that it was not realistic, not that he's against it working in the same way independent of parent order. I don't feel strongly on whether to include a test for each direction. Unless others do, I guess I'll leave it as is. (But I did add a test case just now to see, so it's very little work for me if someone does want it included.)

> In other words, when asked to replay w on top of z, how would we
> decide which parent to keep (in this case, e is kept)?

Keep any parent that is not an ancestor of the new base? Or something like that.

  [1] http://thread.gmane.org/gmane.comp.version-control.git/205796/focus=205806