Patchwork [2,of,2,stable] rebase: handle rebasing of merges of destination ancestors

login
register
mail settings
Submitter Mads Kiilerich
Date Nov. 8, 2014, 1:27 a.m.
Message ID <4f80b7a3cdf219d1c8e5.1415410048@localhost.localdomain>
Download mbox | patch
Permalink /patch/6649/
State Rejected
Headers show

Comments

Mads Kiilerich - Nov. 8, 2014, 1:27 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1415409531 -3600
#      Sat Nov 08 02:18:51 2014 +0100
# Branch stable
# Node ID 4f80b7a3cdf219d1c8e5b3a7f2d539cd035cf8c4
# Parent  8f689654b7d3d6b7c2b370cd5e38198b625e4fc6
rebase: handle rebasing of merges of destination ancestors

A situation with several merges from another branch can be 'straightened out'
by rebasing (or grafting) on top of the other branch. One problem is merges
that merges an ancestor of the rebase destination. Graft will skip such merges
as 'ungraftable merge revision'. Rebase would try to rebase them ... and could
fail.

Now, the problem is solved by skipping such ancestor merges when rebasing.
Martin von Zweigbergk - Nov. 8, 2014, 5:38 a.m.
On Fri Nov 07 2014 at 5:28:34 PM Mads Kiilerich <mads@kiilerich.com> wrote:

> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1415409531 -3600
> #      Sat Nov 08 02:18:51 2014 +0100
> # Branch stable
> # Node ID 4f80b7a3cdf219d1c8e5b3a7f2d539cd035cf8c4
> # Parent  8f689654b7d3d6b7c2b370cd5e38198b625e4fc6
> rebase: handle rebasing of merges of destination ancestors
>
> A situation with several merges from another branch can be 'straightened
> out'
> by rebasing (or grafting) on top of the other branch. One problem is merges
> that merges an ancestor of the rebase destination. Graft will skip such
> merges
> as 'ungraftable merge revision'. Rebase would try to rebase them ... and
> could
> fail.
>
> Now, the problem is solved by skipping such ancestor merges when rebasing.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -547,7 +547,21 @@ def rebasenode(repo, rev, p1, state, col
>          base = repo[rev].p1().node()
>      else:
>          # In case of merge, we need to pick the right parent as merge
> base.
> -        #
> +
> +        margedancestors = list(repo.revs('parents(%d)&::%d',
> +                                         repo[rev].rev(), repo[p1].rev()))
> +        if margedancestors:
> +            repo.ui.warn(_('%s is merging %s which is an ancestor of '
> +                           'the rebase target and is ignored\n') %
> +                         (rev, margedancestors[0]))
> +            # TODO: If all ancestors all the way back to 'branching
> point' has
> +            # been rebased (no cherry picking), we could use
> +            #   base = repo[margedancestors[0]].node()
> +            # to use the merge snapshot as a checkpoint to verify that
> rebase
> +            # conflicts has been resolved as the merge conflicts was
> resolved.
> +            # If resolved the same way, the rebase of the merge willbe a
> noop.
> +            return None
> +
>          # Imagine we have:
>          # - M: currently rebase revision in this step
>          # - A: one parent of M
> diff --git a/tests/test-rebase-collapse.t b/tests/test-rebase-collapse.t
> --- a/tests/test-rebase-collapse.t
> +++ b/tests/test-rebase-collapse.t
> @@ -113,6 +113,7 @@ Rebasing E onto H:
>
>    $ hg phase --force --secret 6
>    $ hg rebase --source 4 --collapse
> +  6 is merging 5 which is an ancestor of the rebase target and is ignored
>    saved backup bundle to $TESTTMP/a2/.hg/strip-backup/*-backup.hg (glob)
>
>    $ hg tglog
> @@ -153,6 +154,7 @@ Rebasing G onto H with custom message:
>    > true
>    > EOF
>    $ HGEDITOR="sh $TESTTMP/checkeditform.sh" hg rebase --source 4
> --collapse -m 'custom message' -e
> +  6 is merging 5 which is an ancestor of the rebase target and is ignored
>    HGEDITFORM=rebase.collapse
>    saved backup bundle to $TESTTMP/a3/.hg/strip-backup/*-backup.hg (glob)
>
> diff --git a/tests/test-rebase-detach.t b/tests/test-rebase-detach.t
> --- a/tests/test-rebase-detach.t
> +++ b/tests/test-rebase-detach.t
> @@ -325,6 +325,7 @@ Verify that target is not selected as ex
>    $ hg ci -m "J"
>
>    $ hg rebase -s 8 -d 7 --collapse --config ui.merge=internal:other
> +  9 is merging 7 which is an ancestor of the rebase target and is ignored
>    saved backup bundle to $TESTTMP/a6/.hg/strip-backup/*-backup.hg (glob)
>
>    $ hg tglog
> diff --git a/tests/test-rebase-newancestor.t b/tests/test-rebase-
> newancestor.t
> --- a/tests/test-rebase-newancestor.t
> +++ b/tests/test-rebase-newancestor.t
> @@ -76,15 +76,11 @@ that often happens when trying to rebase
>    $ touch devstuff
>    $ hg ci -Aqm 'real dev stuff'
>    $ hg rebase -r 'branch(dev)' -d default
> -  remote changed f which local deleted
> -  use (c)hanged version or leave (d)eleted? c
> -  local changed f which remote deleted
> -  use (c)hanged version or (d)elete? c
> +  4 is merging 1 which is an ancestor of the rebase target and is ignored
> +  5 is merging 2 which is an ancestor of the rebase target and is ignored
>    saved backup bundle to $TESTTMP/repo2/.hg/strip-backup/*-backup.hg
> (glob)
>    $ hg tglog
> -  @  4: 'real dev stuff'
> -  |
> -  o  3: 'merge f stuff'
> +  @  3: 'real dev stuff'
>

It looks like the commit that creates the dev branch still gets lost. Is
that desirable? I don't see why it would be.


>    |
>    o  2: 'remove f'
>    |
> diff --git a/tests/test-rebase-scenario-global.t
> b/tests/test-rebase-scenario-global.t
> --- a/tests/test-rebase-scenario-global.t
> +++ b/tests/test-rebase-scenario-global.t
> @@ -117,6 +117,7 @@ E onto H - skip of G:
>    $ cd a3
>
>    $ hg rebase -s 4 -d 7
> +  6 is merging 5 which is an ancestor of the rebase target and is ignored
>    saved backup bundle to $TESTTMP/a3/.hg/strip-backup/*-backup.hg (glob)
>
>    $ hg tglog
> @@ -143,6 +144,7 @@ F onto E - rebase of a branching point (
>    $ cd a4
>
>    $ hg rebase -s 5 -d 4
> +  6 is merging 4 which is an ancestor of the rebase target and is ignored
>    saved backup bundle to $TESTTMP/a4/.hg/strip-backup/*-backup.hg (glob)
>
>    $ hg tglog
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - Nov. 8, 2014, 10:15 a.m.
On 11/08/2014 01:27 AM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1415409531 -3600
> #      Sat Nov 08 02:18:51 2014 +0100
> # Branch stable
> # Node ID 4f80b7a3cdf219d1c8e5b3a7f2d539cd035cf8c4
> # Parent  8f689654b7d3d6b7c2b370cd5e38198b625e4fc6
> rebase: handle rebasing of merges of destination ancestors

I think we need a schema about what is happening here. As more verbose 
and documented test could achieve that.
Mads Kiilerich - Nov. 8, 2014, 11:17 a.m.
On 11/08/2014 06:38 AM, Martin von Zweigbergk wrote:
>
>
> On Fri Nov 07 2014 at 5:28:34 PM Mads Kiilerich <mads@kiilerich.com 
> <mailto:mads@kiilerich.com>> wrote:
>
>     # HG changeset patch
>     # User Mads Kiilerich <madski@unity3d.com <mailto:madski@unity3d.com>>
>     # Date 1415409531 -3600
>     #      Sat Nov 08 02:18:51 2014 +0100
>     # Branch stable
>     # Node ID 4f80b7a3cdf219d1c8e5b3a7f2d539cd035cf8c4
>     # Parent  8f689654b7d3d6b7c2b370cd5e38198b625e4fc6
>     rebase: handle rebasing of merges of destination ancestors
>
>     A situation with several merges from another branch can be
>     'straightened out'
>     by rebasing (or grafting) on top of the other branch. One problem
>     is merges
>     that merges an ancestor of the rebase destination. Graft will skip
>     such merges
>     as 'ungraftable merge revision'. Rebase would try to rebase them
>     ... and could
>     fail.
>
>     Now, the problem is solved by skipping such ancestor merges when
>     rebasing.
>

>     diff --git a/tests/test-rebase-newancestor.t
>     b/tests/test-rebase-newancestor.t
>     --- a/tests/test-rebase-newancestor.t
>     +++ b/tests/test-rebase-newancestor.t
>     @@ -76,15 +76,11 @@ that often happens when trying to rebase
>        $ touch devstuff
>        $ hg ci -Aqm 'real dev stuff'
>        $ hg rebase -r 'branch(dev)' -d default
>     -  remote changed f which local deleted
>     -  use (c)hanged version or leave (d)eleted? c
>     -  local changed f which remote deleted
>     -  use (c)hanged version or (d)elete? c
>     +  4 is merging 1 which is an ancestor of the rebase target and is
>     ignored
>     +  5 is merging 2 which is an ancestor of the rebase target and is
>     ignored
>        saved backup bundle to
>     $TESTTMP/repo2/.hg/strip-backup/*-backup.hg (glob)
>        $ hg tglog
>     -  @  4: 'real dev stuff'
>     -  |
>     -  o  3: 'merge f stuff'
>     +  @  3: 'real dev stuff'
>
>
> It looks like the commit that creates the dev branch still gets lost. 
> Is that desirable? I don't see why it would be.

The test is not using --keepbranches so when the 'dev' changeset is 
rebased, it do nothing. One of the benefits of rebase is that it removes 
empty changesets - for instance when a change has been grafted to occur 
on both sides or when merging with an ancestor. It is thus correct 
behaviour that the branch creation changeset goes away.

/Mads
Mads Kiilerich - Nov. 8, 2014, 11:30 a.m.
On 11/08/2014 11:15 AM, Pierre-Yves David wrote:
>
>
> On 11/08/2014 01:27 AM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1415409531 -3600
>> #      Sat Nov 08 02:18:51 2014 +0100
>> # Branch stable
>> # Node ID 4f80b7a3cdf219d1c8e5b3a7f2d539cd035cf8c4
>> # Parent  8f689654b7d3d6b7c2b370cd5e38198b625e4fc6
>> rebase: handle rebasing of merges of destination ancestors
>
> I think we need a schema about what is happening here. As more verbose 
> and documented test could achieve that.

A "schema"? What do you mean?

/Mads
Pierre-Yves David - Nov. 8, 2014, 11:33 a.m.
On 11/08/2014 11:30 AM, Mads Kiilerich wrote:
> On 11/08/2014 11:15 AM, Pierre-Yves David wrote:
>>
>>
>> On 11/08/2014 01:27 AM, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1415409531 -3600
>>> #      Sat Nov 08 02:18:51 2014 +0100
>>> # Branch stable
>>> # Node ID 4f80b7a3cdf219d1c8e5b3a7f2d539cd035cf8c4
>>> # Parent  8f689654b7d3d6b7c2b370cd5e38198b625e4fc6
>>> rebase: handle rebasing of merges of destination ancestors
>>
>> I think we need a schema about what is happening here. As more verbose
>> and documented test could achieve that.
>
> A "schema"? What do you mean?

drawing.

a longer explanation of what you are trying to do, why the previous 
situation is bad and why the new situation is better.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -547,7 +547,21 @@  def rebasenode(repo, rev, p1, state, col
         base = repo[rev].p1().node()
     else:
         # In case of merge, we need to pick the right parent as merge base.
-        #
+
+        margedancestors = list(repo.revs('parents(%d)&::%d',
+                                         repo[rev].rev(), repo[p1].rev()))
+        if margedancestors:
+            repo.ui.warn(_('%s is merging %s which is an ancestor of '
+                           'the rebase target and is ignored\n') %
+                         (rev, margedancestors[0]))
+            # TODO: If all ancestors all the way back to 'branching point' has
+            # been rebased (no cherry picking), we could use
+            #   base = repo[margedancestors[0]].node()
+            # to use the merge snapshot as a checkpoint to verify that rebase
+            # conflicts has been resolved as the merge conflicts was resolved.
+            # If resolved the same way, the rebase of the merge willbe a noop.
+            return None
+
         # Imagine we have:
         # - M: currently rebase revision in this step
         # - A: one parent of M
diff --git a/tests/test-rebase-collapse.t b/tests/test-rebase-collapse.t
--- a/tests/test-rebase-collapse.t
+++ b/tests/test-rebase-collapse.t
@@ -113,6 +113,7 @@  Rebasing E onto H:
 
   $ hg phase --force --secret 6
   $ hg rebase --source 4 --collapse
+  6 is merging 5 which is an ancestor of the rebase target and is ignored
   saved backup bundle to $TESTTMP/a2/.hg/strip-backup/*-backup.hg (glob)
 
   $ hg tglog
@@ -153,6 +154,7 @@  Rebasing G onto H with custom message:
   > true
   > EOF
   $ HGEDITOR="sh $TESTTMP/checkeditform.sh" hg rebase --source 4 --collapse -m 'custom message' -e
+  6 is merging 5 which is an ancestor of the rebase target and is ignored
   HGEDITFORM=rebase.collapse
   saved backup bundle to $TESTTMP/a3/.hg/strip-backup/*-backup.hg (glob)
 
diff --git a/tests/test-rebase-detach.t b/tests/test-rebase-detach.t
--- a/tests/test-rebase-detach.t
+++ b/tests/test-rebase-detach.t
@@ -325,6 +325,7 @@  Verify that target is not selected as ex
   $ hg ci -m "J"
 
   $ hg rebase -s 8 -d 7 --collapse --config ui.merge=internal:other
+  9 is merging 7 which is an ancestor of the rebase target and is ignored
   saved backup bundle to $TESTTMP/a6/.hg/strip-backup/*-backup.hg (glob)
 
   $ hg tglog
diff --git a/tests/test-rebase-newancestor.t b/tests/test-rebase-newancestor.t
--- a/tests/test-rebase-newancestor.t
+++ b/tests/test-rebase-newancestor.t
@@ -76,15 +76,11 @@  that often happens when trying to rebase
   $ touch devstuff
   $ hg ci -Aqm 'real dev stuff'
   $ hg rebase -r 'branch(dev)' -d default
-  remote changed f which local deleted
-  use (c)hanged version or leave (d)eleted? c
-  local changed f which remote deleted
-  use (c)hanged version or (d)elete? c
+  4 is merging 1 which is an ancestor of the rebase target and is ignored
+  5 is merging 2 which is an ancestor of the rebase target and is ignored
   saved backup bundle to $TESTTMP/repo2/.hg/strip-backup/*-backup.hg (glob)
   $ hg tglog
-  @  4: 'real dev stuff'
-  |
-  o  3: 'merge f stuff'
+  @  3: 'real dev stuff'
   |
   o  2: 'remove f'
   |
diff --git a/tests/test-rebase-scenario-global.t b/tests/test-rebase-scenario-global.t
--- a/tests/test-rebase-scenario-global.t
+++ b/tests/test-rebase-scenario-global.t
@@ -117,6 +117,7 @@  E onto H - skip of G:
   $ cd a3
 
   $ hg rebase -s 4 -d 7
+  6 is merging 5 which is an ancestor of the rebase target and is ignored
   saved backup bundle to $TESTTMP/a3/.hg/strip-backup/*-backup.hg (glob)
 
   $ hg tglog
@@ -143,6 +144,7 @@  F onto E - rebase of a branching point (
   $ cd a4
 
   $ hg rebase -s 5 -d 4
+  6 is merging 4 which is an ancestor of the rebase target and is ignored
   saved backup bundle to $TESTTMP/a4/.hg/strip-backup/*-backup.hg (glob)
 
   $ hg tglog