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

[Johannes Sixt <j.sixt@viscovery.net>]

Am 6/4/2013 19:18, schrieb Junio C Hamano:
> 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?

The comment is really just about the inconsistency, not about a request to
have a guaranteed order among the parents of a merge commit.

Having said that, wouldn't it be useful (generally, not just in this
context) to have a guarantee in which order --topo-order lists parents of
a merge?

>> +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?

'git rebase -p z w' is a nonsense request in this situation. (I.e., there
is no requirement on the result.) At best, we could detect it and bail out
or warn.

-- Hannes