Web lists-archives.com

Re: rebase preserve-merges: incorrect merge commits




2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@xxxxxxxxx>:
> 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@xxxxxx>:
>> Hi Matwey,
>>
>> On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
>>
>>> I think that rebase preserve-merges algorithm needs further
>>> improvements. Probably, you already know it.
>>
>> Yes. preserve-merges is a fundamentally flawed design.
>>
>> Please have a look here:
>>
>>         https://github.com/git/git/pull/447
>>
>> Since we are in a feature freeze in preparation for v2.16.0, I will
>> submit these patch series shortly after v2.16.0 is released.
>>
>>> As far as I understand the root cause of this that when new merge
>>> commit is created by rebase it is done simply by git merge
>>> $new_parents without taking into account any actual state of the
>>> initial merge commit.
>>
>> Indeed. preserve-merges does not allow commits to be reordered. (Actually,
>> it *does* allow it, but then fails to handle it correctly.) We even have
>> test cases that mark this as "known breakage".
>>
>> But really, I do not think it is worth trying to fix the broken design.
>> Better to go with the new recreate-merges. (I am biased, of course,
>> because I invented recreate-merges. But then, I also invented
>> preserve-merges, so ...)
>
> Well. I just checked --recreate-merges=no-rebase-cousins from the PR
> and found that it produces the same wrong result in my test example.
> The topology is reproduced correctly, but merge-commit content is
> broken.
> I did git rebase --recreate-merges=no-rebase-cousins --onto abc-0.1 v0.1 abc-0.2

Indeed, exactly as you still say in the documentation: "Merge conflict
resolutions or manual amendments to merge commits are not preserved."
My initial point is that they have to be preserved. Probably in
recreate-merges, if preserve-merges is discontinued.

>
>>
>> Ciao,
>> Johannes
>>
>
>
>
> --
> With best regards,
> Matwey V. Kornilov



-- 
With best regards,
Matwey V. Kornilov