git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* rebase preserve-merges: incorrect merge commits
@ 2018-01-08 10:24 Matwey V. Kornilov
  2018-01-08 13:56 ` Johannes Schindelin
  0 siblings, 1 reply; 11+ messages in thread
From: Matwey V. Kornilov @ 2018-01-08 10:24 UTC (permalink / raw)
  To: git

Hello,

I think that rebase preserve-merges algorithm needs further
improvements. Probably, you already know it.

I was playing with rebasing linux kernel tree and found that rebase
preserve-merges works in counterintuitive way while handling
merge-commits. I don't want to discuss arising merge conflicts which
can be illuminated with help of rerere trained at original branch. An
major issue here with silently auto-merging, some merge commits were
silently rebased with different content and it breaks the build :)

I crafted toy example to demonstrate an issue:
https://github.com/matwey/example-git-rebase-pm
Here I have commit range v0.1..v0.2, then I return to v0.1 and
introduce new commit abc-0.1 with new file on v0.1. Then I want to
rebase v0.1..v0.2 onto new abc-0.1. It is obvious that as soon as I
introduced single new file the difference between old v0.2 and new
v0.2 should have only this new file. However it actually contains
other changes.

Reproduce as the following:

git checkout v0.2
git branch abc-0.2
git checkout v0.1
git branch abc-0.1
git checkout abc-0.1
vim LICENSE
git add LICENSE
git commit -a
git rebase --preserve-merges --onto abc-0.1 v0.1 abc-0.2

Actual result:

diff --git a/LICENSE b/LICENSE
deleted file mode 100644
index 297edb3..0000000
--- a/LICENSE
+++ /dev/null
@@ -1,2 +0,0 @@
-Hello, world!
-
diff --git a/mod.c b/mod.c
index e6fa107..e339597 100644
--- a/mod.c
+++ b/mod.c
@@ -2,5 +2,5 @@
 #include "mod.h"

 int mod_fun(char* buf, int len) {
-       return -calc_fun(buf, len);
+       return -calc_fun(buf, len, 0);
 }

Expected result:

diff --git a/LICENSE b/LICENSE
deleted file mode 100644
index 297edb3..0000000
--- a/LICENSE
+++ /dev/null
@@ -1,2 +0,0 @@
-Hello, world!
-


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. This is why all manually introduced changes in
merge-commits are lost at rebase stage. I think that the following
improvement of merge-commit recreating stage would be possible:

1) Let we have merge commit in the source tree

-- A --\
        M--
-- B --/

And we want to rebase M to the new tree where new parents A' and B'
already existing.

-- A'

-- B'

2) Then I suggest to perform it in three steps. First, lets take all
possible differences between A and M, B and M and so on... and then
try to cherry-pick the diffs on A' and B' respectively. Here AM' and
BM' are temporary commits which is very close in their states (in
ideal case, equal).

-- A' -- (AM)'

-- B' -- (BM)'

3) Then we do git merge (AM)' (BM)' and ask user to resolve the
conflicts if any. Here we obtain the following tree. Where M" is
produced by git merge.

-- A' -- (AM)' --\
                  M"
-- B' -- (BM)' --/

4) Then we do merge between A' and B' but reset the state to be the
same as in M" before new merge commit is created:

-- A' -- (AM)' --\
     \            \
       M' (==M")    M"
     /            /
-- B' -- (BM)' --/

5) Then (AM)' (BM)' and M" are discarded from the tree.


Probably, I missed something important or incorrectly read
git-rebase--interactive.sh

-- 
With best regards,
Matwey V. Kornilov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 10:24 rebase preserve-merges: incorrect merge commits Matwey V. Kornilov
@ 2018-01-08 13:56 ` Johannes Schindelin
  2018-01-08 14:42   ` Matwey V. Kornilov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Schindelin @ 2018-01-08 13:56 UTC (permalink / raw)
  To: Matwey V. Kornilov; +Cc: git

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 ...)

Ciao,
Johannes


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 13:56 ` Johannes Schindelin
@ 2018-01-08 14:42   ` Matwey V. Kornilov
  2018-01-08 14:46     ` Matwey V. Kornilov
  0 siblings, 1 reply; 11+ messages in thread
From: Matwey V. Kornilov @ 2018-01-08 14:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> 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

>
> Ciao,
> Johannes
>



-- 
With best regards,
Matwey V. Kornilov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 14:42   ` Matwey V. Kornilov
@ 2018-01-08 14:46     ` Matwey V. Kornilov
  2018-01-08 16:32       ` Johannes Schindelin
  0 siblings, 1 reply; 11+ messages in thread
From: Matwey V. Kornilov @ 2018-01-08 14:46 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
> 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 14:46     ` Matwey V. Kornilov
@ 2018-01-08 16:32       ` Johannes Schindelin
  2018-01-08 17:24         ` Matwey V. Kornilov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Schindelin @ 2018-01-08 16:32 UTC (permalink / raw)
  To: Matwey V. Kornilov; +Cc: git

Hi,

On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:

> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> 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...

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 16:32       ` Johannes Schindelin
@ 2018-01-08 17:24         ` Matwey V. Kornilov
  2018-01-08 19:36           ` Johannes Schindelin
  0 siblings, 1 reply; 11+ messages in thread
From: Matwey V. Kornilov @ 2018-01-08 17:24 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
>
>> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
>> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >> 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.

> Ciao,
> Johannes



-- 
With best regards,
Matwey V. Kornilov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 17:24         ` Matwey V. Kornilov
@ 2018-01-08 19:36           ` Johannes Schindelin
  2018-01-09  9:49             ` Matwey V. Kornilov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Schindelin @ 2018-01-08 19:36 UTC (permalink / raw)
  To: Matwey V. Kornilov; +Cc: git

Hi Matwey,

On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:

> 2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >
> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >
> >> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
> >> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> >> 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).

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-08 19:36           ` Johannes Schindelin
@ 2018-01-09  9:49             ` Matwey V. Kornilov
  2018-01-09 13:25               ` Johannes Schindelin
  0 siblings, 1 reply; 11+ messages in thread
From: Matwey V. Kornilov @ 2018-01-09  9:49 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

2018-01-08 22:36 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi Matwey,
>
> On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
>
>> 2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >
>> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
>> >
>> >> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
>> >> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >> >> 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?

>
> Ciao,
> Johannes



-- 
With best regards,
Matwey V. Kornilov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-09  9:49             ` Matwey V. Kornilov
@ 2018-01-09 13:25               ` Johannes Schindelin
  2018-01-09 14:19                 ` Matwey V. Kornilov
  0 siblings, 1 reply; 11+ messages in thread
From: Johannes Schindelin @ 2018-01-09 13:25 UTC (permalink / raw)
  To: Matwey V. Kornilov; +Cc: git

Hi Matwey,

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

> 2018-01-08 22:36 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >
> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >
> >> 2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> >
> >> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >> >
> >> >> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
> >> >> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> >> >> 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.

Ciao,
Johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-09 13:25               ` Johannes Schindelin
@ 2018-01-09 14:19                 ` Matwey V. Kornilov
  2018-01-09 16:29                   ` Johannes Schindelin
  0 siblings, 1 reply; 11+ messages in thread
From: Matwey V. Kornilov @ 2018-01-09 14:19 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

2018-01-09 16:25 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi Matwey,
>
> On Tue, 9 Jan 2018, Matwey V. Kornilov wrote:
>
>> 2018-01-08 22:36 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >
>> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
>> >
>> >> 2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >> >
>> >> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
>> >> >
>> >> >> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
>> >> >> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >> >> >> 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.

>
> Ciao,
> Johannes



-- 
With best regards,
Matwey V. Kornilov

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: rebase preserve-merges: incorrect merge commits
  2018-01-09 14:19                 ` Matwey V. Kornilov
@ 2018-01-09 16:29                   ` Johannes Schindelin
  0 siblings, 0 replies; 11+ messages in thread
From: Johannes Schindelin @ 2018-01-09 16:29 UTC (permalink / raw)
  To: Matwey V. Kornilov; +Cc: git

Hi Matwey,

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

> 2018-01-09 16:25 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> > Hi Matwey,
> >
> > On Tue, 9 Jan 2018, Matwey V. Kornilov wrote:
> >
> >> 2018-01-08 22:36 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> >
> >> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >> >
> >> >> 2018-01-08 19:32 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> >> >
> >> >> > On Mon, 8 Jan 2018, Matwey V. Kornilov wrote:
> >> >> >
> >> >> >> 2018-01-08 17:42 GMT+03:00 Matwey V. Kornilov <matwey.kornilov@gmail.com>:
> >> >> >> > 2018-01-08 16:56 GMT+03:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> >> >> >> >> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-01-09 16:29 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 10:24 rebase preserve-merges: incorrect merge commits Matwey V. Kornilov
2018-01-08 13:56 ` Johannes Schindelin
2018-01-08 14:42   ` Matwey V. Kornilov
2018-01-08 14:46     ` Matwey V. Kornilov
2018-01-08 16:32       ` Johannes Schindelin
2018-01-08 17:24         ` Matwey V. Kornilov
2018-01-08 19:36           ` Johannes Schindelin
2018-01-09  9:49             ` Matwey V. Kornilov
2018-01-09 13:25               ` Johannes Schindelin
2018-01-09 14:19                 ` Matwey V. Kornilov
2018-01-09 16:29                   ` Johannes Schindelin

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://7fh6tueqddpjyxjmgtdiueylzoqt6pt7hec3pukyptlmohoowvhde4yd.onion/inbox.comp.version-control.git
	nntp://ie5yzdi7fg72h7s4sdcztq5evakq23rdt33mfyfcddc5u3ndnw24ogqd.onion/inbox.comp.version-control.git
	nntp://4uok3hntl7oi7b4uf4rtfwefqeexfzil2w6kgk2jn5z2f764irre7byd.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git