Web lists-archives.com

Re: rebase preserve-merges: incorrect merge commits




Hi Matwey,

On Tue, 9 Jan 2018, Matwey V. Kornilov wrote:

> 2018-01-09 16:25 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@xxxxxx>:
> > Hi Matwey,
> >
> > On Tue, 9 Jan 2018, Matwey V. Kornilov wrote:
> >
> >> 2018-01-08 22:36 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@xxxxxx>:
> >> >
> >> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >> >
> >> >> 2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@xxxxxx>:
> >> >> >
> >> >> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >> >> >
> >> >> >> 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.
> >> >> >
> >> >> > Ah, but that is consistent with how non-merge-preserving rebase works: the
> >> >> > `pick` commands *also* do not record merge conflict resolution...
> >> >> >
> >> >>
> >> >> I am sorry, didn't get it. When I do non-merge-preserving rebase
> >> >> --interactive there is no way to `pick' merge-commit at all.
> >> >
> >> > Right, but you can `pick` commits and you can get merge conflicts. And you
> >> > need to resolve those merge conflicts and those merge conflict resolutions
> >> > are not preserved for future interactive rebases, unless you use `rerere`
> >> > (in which case it also extends to `pick`ing merge commits in
> >> > merge-preserving mode).
> >>
> >> Are you talking about merge conflicts arising due to commits reordering?
> >
> > Merge conflicts can arise from commit reordering, and they can also arise
> > from commits introduced in "upstream" in the meantime.
> 
> Then I am totally agree with you.
> But initially I said about conflict resolutions and amendments already
> contained in existing merge-commits. While rerere can at least learn
> conflict resolutions from existing merge-commits, rerere cannot learn
> and recover manual amendments.

Great, so the information is all there and you can implement it? :-)

Ciao,
Johannes