Patchwork [2,of,2,V2] rebase: support multiple roots for rebaseset

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 15, 2013, 11:55 p.m.
Message ID <c8e37ef7699046fd03bb.1358294158@yamac.lan>
Download mbox | patch
Permalink /patch/637/
State Superseded
Headers show

Comments

Pierre-Yves David - Jan. 15, 2013, 11:55 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
# Date 1358293892 -3600
# Node ID c8e37ef7699046fd03bb2e5cef35cf0ea950ee2c
# Parent  470c5d72dac420aa91a45e61acf4894da7f22843
rebase: support multiple roots for rebaseset

We have all the necessary mechanism to rebase a set with multiple roots, we only
needed a proper handling of this case we preparing and concluding the rebase.
This changeset des that.

Rebase set with multiple root allows some awesome usage of rebase like:

- rebase all your draft on lastest upstream

  hg rebase --dest @ --rev 'draft()'

- exclusion of specific changeset during rebase

  hg rebase --rev '42:: - author(Babar)'

-  rebase a set of revision were multiple roots are later merged

  hg rebase --rev '(18+42)::'
Pierre-Yves David - Jan. 16, 2013, 7:28 p.m.
On 16 janv. 2013, at 04:52, Kevin Bullock wrote:

> On 15 Jan 2013, at 5:55 PM, Pierre-Yves David wrote:
> 
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
>> # Date 1358293892 -3600
>> # Node ID c8e37ef7699046fd03bb2e5cef35cf0ea950ee2c
>> # Parent  470c5d72dac420aa91a45e61acf4894da7f22843
>> rebase: support multiple roots for rebaseset
>> 
>> We have all the necessary mechanism to rebase a set with multiple roots, we only
>> needed a proper handling of this case we preparing and concluding the rebase.
>> This changeset des that.
> 
> And when you rebase a set with multiple roots, where do they get rebased _to_? How are the semantics of a multiple-root rebase defined?

On the destination as if you do multiple rebase.
This changeset allows to rebase more comcplexe set, but as the original rebase it keeps the original topology.
tonfa - Jan. 16, 2013, 10:29 p.m.
I'll push once testing finishes.

(I removed the s/-r/--rev/ on the test).


On Wed, Jan 16, 2013 at 8:28 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
> On 16 janv. 2013, at 04:52, Kevin Bullock wrote:
>
> > On 15 Jan 2013, at 5:55 PM, Pierre-Yves David wrote:
> >
> >> # HG changeset patch
> >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> >> # Date 1358293892 -3600
> >> # Node ID c8e37ef7699046fd03bb2e5cef35cf0ea950ee2c
> >> # Parent  470c5d72dac420aa91a45e61acf4894da7f22843
> >> rebase: support multiple roots for rebaseset
> >>
> >> We have all the necessary mechanism to rebase a set with multiple
> roots, we only
> >> needed a proper handling of this case we preparing and concluding the
> rebase.
> >> This changeset des that.
> >
> > And when you rebase a set with multiple roots, where do they get rebased
> _to_? How are the semantics of a multiple-root rebase defined?
>
> On the destination as if you do multiple rebase.
> This changeset allows to rebase more comcplexe set, but as the original
> rebase it keeps the original topology.
>
> --
> Pierre-Yves
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
tonfa - Jan. 16, 2013, 10:39 p.m.
Actually it still looks fishy.


On Wed, Jan 16, 2013 at 12:55 AM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org>
> # Date 1358293892 -3600
> # Node ID c8e37ef7699046fd03bb2e5cef35cf0ea950ee2c
> # Parent  470c5d72dac420aa91a45e61acf4894da7f22843
> rebase: support multiple roots for rebaseset
>
> We have all the necessary mechanism to rebase a set with multiple roots,
> we only
> needed a proper handling of this case we preparing and concluding the
> rebase.
> This changeset des that.
>
> Rebase set with multiple root allows some awesome usage of rebase like:
>
> - rebase all your draft on lastest upstream
>
>   hg rebase --dest @ --rev 'draft()'
>
> - exclusion of specific changeset during rebase
>
>   hg rebase --rev '42:: - author(Babar)'
>
> -  rebase a set of revision were multiple roots are later merged
>
>   hg rebase --rev '(18+42)::'
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -574,13 +574,13 @@ def abort(repo, originalwd, target, stat
>      else:
>          # Strip from the first rebased revision
>          merge.update(repo, repo[originalwd].rev(), False, True, False)
>          rebased = filter(lambda x: x > -1 and x != target, state.values())
>          if rebased:
> -            strippoint = min(rebased)
> +            strippoints = [c.node()  for c in repo.set('roots(%ld)',
> rebased)]
>              # no backup of rebased cset versions needed
> -            repair.strip(repo.ui, repo, repo[strippoint].node())
> +            repair.strip(repo.ui, repo, strippoints)
>          clearstatus(repo)
>          repo.ui.warn(_('rebase aborted\n'))
>          return 0
>
>  def buildstate(repo, dest, rebaseset, collapse):
> @@ -599,24 +599,21 @@ def buildstate(repo, dest, rebaseset, co
>          raise util.Abort(_('cannot rebase onto an applied mq patch'))
>
>      roots = list(repo.set('roots(%ld)', rebaseset))
>      if not roots:
>          raise util.Abort(_('no matching revisions'))
> -    if len(roots) > 1:
> -        raise util.Abort(_("can't rebase multiple roots"))
> -    root = roots[0]
> +    for root in roots:
> +        commonbase = root.ancestor(dest)
> +        if commonbase == root:
> +            raise util.Abort(_('source is ancestor of destination'))
> +        if commonbase == dest:
> +            samebranch = root.branch() == dest.branch()
> +            if not collapse and samebranch and root in dest.children():
> +                repo.ui.debug('source is a child of destination\n')
> +                return None
>

commonbase is defined in the loop.

>
> -    commonbase = root.ancestor(dest)
> -    if commonbase == root:
> -        raise util.Abort(_('source is ancestor of destination'))
> -    if commonbase == dest:
> -        samebranch = root.branch() == dest.branch()
> -        if not collapse and samebranch and root in dest.children():
> -            repo.ui.debug('source is a child of destination\n')
> -            return None
> -
> -    repo.ui.debug('rebase onto %d starting from %d\n' % (dest, root))
> +    repo.ui.debug('rebase onto %d starting from %s\n' % (dest, roots))
>      state = dict.fromkeys(rebaseset, nullrev)
>      # Rebase tries to turn <dest> into a parent of <root> while
>      # preserving the number of parents of rebased changesets:
>      #
>      # - A changeset with a single parent will always be rebased as a
> @@ -651,17 +648,21 @@ def buildstate(repo, dest, rebaseset, co
>      #
> +--------------------+----------------------+-------------------------+
>      # | unrelated source   | new parent is <dest> | ambiguous, abort
>    |
>      #
> +--------------------+----------------------+-------------------------+
>      #
>      # The actual abort is handled by `defineparents`
> -    if len(root.parents()) <= 1:
> -        # ancestors of <root> not ancestors of <dest>
> -        detachset = repo.changelog.findmissingrevs([commonbase.rev()],
> -                                                   [root.rev()])
> -        state.update(dict.fromkeys(detachset, nullmerge))
> -        # detachset can have root, and we definitely want to rebase that
> -        state[root.rev()] = nullrev
> +    roots.sort()
> +    for root in roots:
> +        if len(root.parents()) <= 1:
> +            # ancestors of <root> not ancestors of <dest>
> +            detachset = repo.changelog.findmissingrevs([commonbase.rev()],
> +                                                       [root.rev()])
>

And used here. I think dest could be used instead of commonbase.


>  +            for r in detachset:
> +                if r not in state:
> +                    state[r] = nullmerge
> +            # detachset can have root, and we definitely want to rebase
> that
> +            state[root.rev()] = nullrev
>      return repo['.'].rev(), dest.rev(), state
>
>  def clearrebased(ui, repo, state, collapsedas=None):
>      """dispose of rebased revision at the end of the rebase
>
> @@ -677,16 +678,20 @@ def clearrebased(ui, repo, state, collap
>          if markers:
>              obsolete.createmarkers(repo, markers)
>      else:
>          rebased = [rev for rev in state if state[rev] != nullmerge]
>          if rebased:
> -            if set(repo.changelog.descendants([min(rebased)])) -
> set(state):
> -                ui.warn(_("warning: new changesets detected "
> -                          "on source branch, not stripping\n"))
> -            else:
> +            stripped = []
> +            for root in repo.set('roots(%ld)', rebased):
> +                if set(repo.changelog.descendants([root.rev()])) -
> set(state):
> +                    ui.warn(_("warning: new changesets detected "
> +                              "on source branch, not stripping\n"))
> +                else:
> +                    stripped.append(root.node())
> +            if stripped:
>                  # backup the old csets by default
> -                repair.strip(ui, repo, repo[min(rebased)].node(), "all")
> +                repair.strip(ui, repo, stripped, "all")
>
>
>  def pullrebase(orig, ui, repo, *args, **opts):
>      'Call rebase after pull if the latter has been invoked with --rebase'
>      if opts.get('rebase'):
> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
> --- a/tests/test-rebase-obsolete.t
> +++ b/tests/test-rebase-obsolete.t
> @@ -282,11 +282,11 @@ not be rebased.
>  Test rewriting leaving unstability behind is allowed
>  ---------------------------------------------------------------------
>
>    $ hg log -r 'children(8)'
>    9:cf44d2f5a9f4 D (no-eol)
> -  $ hg rebase -r 8
> +  $ hg rebase --rev 8
>    $ hg log -G
>    @  11:0d8f238b634c C
>    |
>    o  10:7c6027df6a99 B
>    |
> @@ -304,5 +304,28 @@ Test rewriting leaving unstability behin
>    |/
>    o  0:cd010b8cd998 A
>
>
>
> +Test multiple root handling
> +------------------------------------
> +
> +  $ hg rebase --dest 4 --rev '7+11+9'
> +  $ hg log -G
> +  @  14:00891d85fcfc C
> +  |
> +  | o  13:102b4c1d889b D
> +  |/
> +  | o  12:bfe264faf697 H
> +  |/
> +  | o  10:7c6027df6a99 B
> +  | |
> +  | x  7:02de42196ebe H
> +  | |
> +  +---o  6:eea13746799a G
> +  | |/
> +  | o  5:24b6387c8c8c F
> +  | |
> +  o |  4:9520eea781bc E
> +  |/
> +  o  0:cd010b8cd998 A
> +
> 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
> @@ -540,8 +540,25 @@ We rebase E and G on B
>  We would expect heads are I, F if it was supported
>
>    $ hg clone -q -u . ah ah6
>    $ cd ah6
>    $ hg rebase -r '(4+6)::' -d 1
> -  abort: can't rebase multiple roots
> -  [255]
> -  $ cd ..
> +  saved backup bundle to
> $TESTTMP/ah6/.hg/strip-backup/3d8a618087a7-backup.hg (glob)
> +  $ hg tglog
> +  @  8: 'I'
> +  |
> +  o  7: 'H'
> +  |
> +  o  6: 'G'
> +  |
> +  | o  5: 'F'
> +  | |
> +  | o  4: 'E'
> +  |/
> +  | o  3: 'D'
> +  | |
> +  | o  2: 'C'
> +  | |
> +  o |  1: 'B'
> +  |/
> +  o  0: 'A'
> +
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - Jan. 16, 2013, 11:01 p.m.
On 16 janv. 2013, at 23:39, Benoit Boissinot wrote:

> Actually it still looks fishy.
> 
>  def buildstate(repo, dest, rebaseset, collapse):
> @@ -599,24 +599,21 @@ def buildstate(repo, dest, rebaseset, co
>          raise util.Abort(_('cannot rebase onto an applied mq patch'))
> 
>      roots = list(repo.set('roots(%ld)', rebaseset))
>      if not roots:
>          raise util.Abort(_('no matching revisions'))
> -    if len(roots) > 1:
> -        raise util.Abort(_("can't rebase multiple roots"))
> -    root = roots[0]
> +    for root in roots:
> +        commonbase = root.ancestor(dest)
> +        if commonbase == root:
> +            raise util.Abort(_('source is ancestor of destination'))
> +        if commonbase == dest:
> +            samebranch = root.branch() == dest.branch()
> +            if not collapse and samebranch and root in dest.children():
> +                repo.ui.debug('source is a child of destination\n')
> +                return None
> 
> commonbase is defined in the loop. 
> 
> -    commonbase = root.ancestor(dest)
> -    if commonbase == root:
> -        raise util.Abort(_('source is ancestor of destination'))
> -    if commonbase == dest:
> -        samebranch = root.branch() == dest.branch()
> -        if not collapse and samebranch and root in dest.children():
> -            repo.ui.debug('source is a child of destination\n')
> -            return None
> -
> -    repo.ui.debug('rebase onto %d starting from %d\n' % (dest, root))
> +    repo.ui.debug('rebase onto %d starting from %s\n' % (dest, roots))
>      state = dict.fromkeys(rebaseset, nullrev)
>      # Rebase tries to turn <dest> into a parent of <root> while
>      # preserving the number of parents of rebased changesets:
>      #
>      # - A changeset with a single parent will always be rebased as a
> @@ -651,17 +648,21 @@ def buildstate(repo, dest, rebaseset, co
>      # +--------------------+----------------------+-------------------------+
>      # | unrelated source   | new parent is <dest> | ambiguous, abort        |
>      # +--------------------+----------------------+-------------------------+
>      #
>      # The actual abort is handled by `defineparents`
> -    if len(root.parents()) <= 1:
> -        # ancestors of <root> not ancestors of <dest>
> -        detachset = repo.changelog.findmissingrevs([commonbase.rev()],
> -                                                   [root.rev()])
> -        state.update(dict.fromkeys(detachset, nullmerge))
> -        # detachset can have root, and we definitely want to rebase that
> -        state[root.rev()] = nullrev
> +    roots.sort()
> +    for root in roots:
> +        if len(root.parents()) <= 1:
> +            # ancestors of <root> not ancestors of <dest>
> +            detachset = repo.changelog.findmissingrevs([commonbase.rev()],
> +                                                       [root.rev()])
> 
> And used here. I think dest could be used instead of commonbase.
>  
> +            for r in detachset:
> +                if r not in state:
> +                    state[r] = nullmerge
> +            # detachset can have root, and we definitely want to rebase that
> +            state[root.rev()] = nullrev
>      return repo['.'].rev(), dest.rev(), state


Good catch, we need single loop. For clarity detachset may be accumulated and processed after the loop.

Patch

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -574,13 +574,13 @@  def abort(repo, originalwd, target, stat
     else:
         # Strip from the first rebased revision
         merge.update(repo, repo[originalwd].rev(), False, True, False)
         rebased = filter(lambda x: x > -1 and x != target, state.values())
         if rebased:
-            strippoint = min(rebased)
+            strippoints = [c.node()  for c in repo.set('roots(%ld)', rebased)]
             # no backup of rebased cset versions needed
-            repair.strip(repo.ui, repo, repo[strippoint].node())
+            repair.strip(repo.ui, repo, strippoints)
         clearstatus(repo)
         repo.ui.warn(_('rebase aborted\n'))
         return 0
 
 def buildstate(repo, dest, rebaseset, collapse):
@@ -599,24 +599,21 @@  def buildstate(repo, dest, rebaseset, co
         raise util.Abort(_('cannot rebase onto an applied mq patch'))
 
     roots = list(repo.set('roots(%ld)', rebaseset))
     if not roots:
         raise util.Abort(_('no matching revisions'))
-    if len(roots) > 1:
-        raise util.Abort(_("can't rebase multiple roots"))
-    root = roots[0]
+    for root in roots:
+        commonbase = root.ancestor(dest)
+        if commonbase == root:
+            raise util.Abort(_('source is ancestor of destination'))
+        if commonbase == dest:
+            samebranch = root.branch() == dest.branch()
+            if not collapse and samebranch and root in dest.children():
+                repo.ui.debug('source is a child of destination\n')
+                return None
 
-    commonbase = root.ancestor(dest)
-    if commonbase == root:
-        raise util.Abort(_('source is ancestor of destination'))
-    if commonbase == dest:
-        samebranch = root.branch() == dest.branch()
-        if not collapse and samebranch and root in dest.children():
-            repo.ui.debug('source is a child of destination\n')
-            return None
-
-    repo.ui.debug('rebase onto %d starting from %d\n' % (dest, root))
+    repo.ui.debug('rebase onto %d starting from %s\n' % (dest, roots))
     state = dict.fromkeys(rebaseset, nullrev)
     # Rebase tries to turn <dest> into a parent of <root> while
     # preserving the number of parents of rebased changesets:
     #
     # - A changeset with a single parent will always be rebased as a
@@ -651,17 +648,21 @@  def buildstate(repo, dest, rebaseset, co
     # +--------------------+----------------------+-------------------------+
     # | unrelated source   | new parent is <dest> | ambiguous, abort        |
     # +--------------------+----------------------+-------------------------+
     #
     # The actual abort is handled by `defineparents`
-    if len(root.parents()) <= 1:
-        # ancestors of <root> not ancestors of <dest>
-        detachset = repo.changelog.findmissingrevs([commonbase.rev()],
-                                                   [root.rev()])
-        state.update(dict.fromkeys(detachset, nullmerge))
-        # detachset can have root, and we definitely want to rebase that
-        state[root.rev()] = nullrev
+    roots.sort()
+    for root in roots:
+        if len(root.parents()) <= 1:
+            # ancestors of <root> not ancestors of <dest>
+            detachset = repo.changelog.findmissingrevs([commonbase.rev()],
+                                                       [root.rev()])
+            for r in detachset:
+                if r not in state:
+                    state[r] = nullmerge
+            # detachset can have root, and we definitely want to rebase that
+            state[root.rev()] = nullrev
     return repo['.'].rev(), dest.rev(), state
 
 def clearrebased(ui, repo, state, collapsedas=None):
     """dispose of rebased revision at the end of the rebase
 
@@ -677,16 +678,20 @@  def clearrebased(ui, repo, state, collap
         if markers:
             obsolete.createmarkers(repo, markers)
     else:
         rebased = [rev for rev in state if state[rev] != nullmerge]
         if rebased:
-            if set(repo.changelog.descendants([min(rebased)])) - set(state):
-                ui.warn(_("warning: new changesets detected "
-                          "on source branch, not stripping\n"))
-            else:
+            stripped = []
+            for root in repo.set('roots(%ld)', rebased):
+                if set(repo.changelog.descendants([root.rev()])) - set(state):
+                    ui.warn(_("warning: new changesets detected "
+                              "on source branch, not stripping\n"))
+                else:
+                    stripped.append(root.node())
+            if stripped:
                 # backup the old csets by default
-                repair.strip(ui, repo, repo[min(rebased)].node(), "all")
+                repair.strip(ui, repo, stripped, "all")
 
 
 def pullrebase(orig, ui, repo, *args, **opts):
     'Call rebase after pull if the latter has been invoked with --rebase'
     if opts.get('rebase'):
diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
--- a/tests/test-rebase-obsolete.t
+++ b/tests/test-rebase-obsolete.t
@@ -282,11 +282,11 @@  not be rebased.
 Test rewriting leaving unstability behind is allowed
 ---------------------------------------------------------------------
 
   $ hg log -r 'children(8)'
   9:cf44d2f5a9f4 D (no-eol)
-  $ hg rebase -r 8
+  $ hg rebase --rev 8
   $ hg log -G
   @  11:0d8f238b634c C
   |
   o  10:7c6027df6a99 B
   |
@@ -304,5 +304,28 @@  Test rewriting leaving unstability behin
   |/
   o  0:cd010b8cd998 A
   
 
 
+Test multiple root handling
+------------------------------------
+
+  $ hg rebase --dest 4 --rev '7+11+9'
+  $ hg log -G
+  @  14:00891d85fcfc C
+  |
+  | o  13:102b4c1d889b D
+  |/
+  | o  12:bfe264faf697 H
+  |/
+  | o  10:7c6027df6a99 B
+  | |
+  | x  7:02de42196ebe H
+  | |
+  +---o  6:eea13746799a G
+  | |/
+  | o  5:24b6387c8c8c F
+  | |
+  o |  4:9520eea781bc E
+  |/
+  o  0:cd010b8cd998 A
+  
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
@@ -540,8 +540,25 @@  We rebase E and G on B
 We would expect heads are I, F if it was supported
 
   $ hg clone -q -u . ah ah6
   $ cd ah6
   $ hg rebase -r '(4+6)::' -d 1
-  abort: can't rebase multiple roots
-  [255]
-  $ cd ..
+  saved backup bundle to $TESTTMP/ah6/.hg/strip-backup/3d8a618087a7-backup.hg (glob)
+  $ hg tglog
+  @  8: 'I'
+  |
+  o  7: 'H'
+  |
+  o  6: 'G'
+  |
+  | o  5: 'F'
+  | |
+  | o  4: 'E'
+  |/
+  | o  3: 'D'
+  | |
+  | o  2: 'C'
+  | |
+  o |  1: 'B'
+  |/
+  o  0: 'A'
+