[PATCH v2] rebase: exclude descendants of obsoletes w/o a successor in dest (issue5300)

Denis Laxalde denis at laxalde.org
Tue Nov 14 16:50:59 EST 2017


# HG changeset patch
# User Denis Laxalde <denis at laxalde.org>
# Date 1510695970 -3600
#      Tue Nov 14 22:46:10 2017 +0100
# Node ID 5dae1b5b2877b300c224011233fcc9cb6aaa9144
# Parent  0564e7c7f4cdbb98ea0ffb3f2c26946dbc0599ac
# EXP-Topic rebase-obsolete
rebase: exclude descendants of obsoletes w/o a successor in dest (issue5300)

.. feature::

   Let 'hg rebase' avoid content-divergence by skipping obsolete
   changesets (and their descendants) when they are present in the rebase
   set along with one of their successors but none of their successors is
   in destination.

In the following example, when trying to rebase 3:: onto 2, the rebase
will abort with "this rebase will cause divergence from: 4":

    o  7 f
    |
    | o  6 e
    | |
    | o  5 d'
    | |
    x |  4 d (rewritten as 5)
    |/
    o  3 c
    |
    | o  2 x
    | |
    o |  1 b
    |/
    o  0 a

By excluding obsolete changesets without a successor in destination (4
in the example above) and their descendants, we make rebase work in this
case, thus giving:

    o  11 e
    |
    o  10 d'
    |
    o  9 c
    |
    o  8 b
    |
    | o  7 f
    | |
    | | x  6 e (rewritten using rebase as 11)
    | | |
    | | x  5 d' (rewritten using rebase as 10)
    | | |
    | x |  4 d
    | |/
    | x  3 c (rewritten using rebase as 9)
    | |
    o |  2 x
    | |
    | x  1 b (rewritten using rebase as 8)
    |/
    o  0 a

where branch 4:: is left behind while branch 5:: is rebased as expected.

The rationale is that users may not be interested in rebasing orphan
changesets when specifying a rebase set that include them but would
still want "stable" ones to be rebased. Currently, the user is suggested
to allow divergence (but probably does not want it) or they must specify
a rebase set excluding problematic changesets (which might be a bit
cumbersome). The approach proposed here corresponds to "Option 2" in
https://www.mercurial-scm.org/wiki/CEDRebase.


We extend _computeobsoletenotrebased() so that it also return a set of
obsolete changesets in rebase set without a successor in destination but
with at least one successor in rebase set. This
'obsoletewithoutsuccessorindestination' is then stored as an attribute
of rebaseruntime and used in _performrebasesubset() to:

* filter out descendants of these changesets from the revisions to
  rebase;
* issue a message about these revisions being skipped.

This only occurs if 'evolution.allowdivergence' option is off and
'rebaseskipobsolete' is on.

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -179,6 +179,7 @@ class rebaseruntime(object):
         # other extensions
         self.keepopen = opts.get('keepopen', False)
         self.obsoletenotrebased = {}
+        self.obsoletewithoutsuccessorindestination = set()
 
     @property
     def repo(self):
@@ -311,9 +312,10 @@ class rebaseruntime(object):
         if not self.ui.configbool('experimental', 'rebaseskipobsolete'):
             return
         obsoleteset = set(obsoleterevs)
-        self.obsoletenotrebased = _computeobsoletenotrebased(self.repo,
-                                    obsoleteset, destmap)
+        self.obsoletenotrebased, self.obsoletewithoutsuccessorindestination = \
+            _computeobsoletenotrebased(self.repo, obsoleteset, destmap)
         skippedset = set(self.obsoletenotrebased)
+        skippedset.update(self.obsoletewithoutsuccessorindestination)
         _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
 
     def _prepareabortorcontinue(self, isabort):
@@ -419,12 +421,26 @@ class rebaseruntime(object):
     def _performrebasesubset(self, tr, subset, pos, total):
         repo, ui, opts = self.repo, self.ui, self.opts
         sortedrevs = repo.revs('sort(%ld, -topo)', subset)
+        allowdivergence = self.ui.configbool(
+            'experimental', 'evolution.allowdivergence')
+        if not allowdivergence:
+            sortedrevs -= repo.revs(
+                'descendants(%ld) and not %ld',
+                self.obsoletewithoutsuccessorindestination,
+                self.obsoletewithoutsuccessorindestination,
+            )
         for rev in sortedrevs:
             dest = self.destmap[rev]
             ctx = repo[rev]
             desc = _ctxdesc(ctx)
             if self.state[rev] == rev:
                 ui.status(_('already rebased %s\n') % desc)
+            elif (not allowdivergence
+                  and rev in self.obsoletewithoutsuccessorindestination):
+                msg = _('note: not rebasing %s and its descendants as '
+                        'this would cause divergence\n') % desc
+                repo.ui.status(msg)
+                self.skipped.add(rev)
             elif rev in self.obsoletenotrebased:
                 succ = self.obsoletenotrebased[rev]
                 if succ is None:
@@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
     return set(r for r in revs if repo[r].obsolete())
 
 def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
-    """return a mapping obsolete => successor for all obsolete nodes to be
-    rebased that have a successors in the destination
+    """Return (obsoletenotrebased, obsoletewithoutsuccessorindestination).
+
+    `obsoletenotrebased` is a mapping mapping obsolete => successor for all
+    obsolete nodes to be rebased given in `rebaseobsrevs`.
 
-    obsolete => None entries in the mapping indicate nodes with no successor"""
+    `obsoletewithoutsuccessorindestination` is a set with obsolete revisions
+    without a successor in destination.
+    """
     obsoletenotrebased = {}
+    obsoletewithoutsuccessorindestination = set([])
 
     assert repo.filtername is None
     cl = repo.changelog
@@ -1641,8 +1662,15 @@ def _computeobsoletenotrebased(repo, reb
                 if cl.isancestor(succnode, destnode):
                     obsoletenotrebased[srcrev] = nodemap[succnode]
                     break
+            else:
+                # If 'srcrev' has a successor in rebase set but none in
+                # destination (which would be catched above), we shall skip it
+                # and its descendants to avoid divergence.
+                if any(nodemap[s] in destmap
+                       for s in successors if s != srcnode):
+                    obsoletewithoutsuccessorindestination.add(srcrev)
 
-    return obsoletenotrebased
+    return obsoletenotrebased, obsoletewithoutsuccessorindestination
 
 def summaryhook(ui, repo):
     if not repo.vfs.exists('rebasestate'):
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
@@ -987,6 +987,208 @@ Create the changes that we will rebase
   rebasing 21:7bdc8a87673d "dummy change" (tip)
   $ cd ..
 
+Divergence cases due to obsolete changesets
+-------------------------------------------
+
+We should ignore branches with unstable changesets when they are based on an
+obsolete changeset which successor is in rebase set.
+
+  $ hg init divergence
+  $ cd divergence
+  $ cat >> .hg/hgrc << EOF
+  > [extensions]
+  > strip =
+  > [alias]
+  > strip = strip --no-backup --quiet
+  > [templates]
+  > instabilities = '{rev}:{node|short} {desc|firstline}{if(instabilities," ({instabilities})")}\n'
+  > EOF
+
+  $ hg debugdrawdag <<EOF
+  >   e   f
+  >   |   |
+  >   d'  d # replace: d -> d'
+  >    \ /
+  >     c
+  >     |
+  >   x b
+  >    \|
+  >     a
+  > EOF
+  $ hg log -G -r 'a'::
+  o  7:1143e9adc121 f
+  |
+  | o  6:d60ebfa0f1cb e
+  | |
+  | o  5:027ad6c5830d d'
+  | |
+  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
+  |/
+  o  3:a82ac2b38757 c
+  |
+  | o  2:630d7c95eff7 x
+  | |
+  o |  1:488e1b7e7341 b
+  |/
+  o  0:b173517d0057 a
+  
+
+Changeset d and its descendants are excluded to avoid divergence of d, which
+would occur because the successor of d (d') is also in rebaseset. As a
+consequence f (descendant of d) is left behind.
+
+  $ hg rebase -b 'e' -d 'x'
+  rebasing 1:488e1b7e7341 "b" (b)
+  rebasing 3:a82ac2b38757 "c" (c)
+  rebasing 5:027ad6c5830d "d'" (d')
+  rebasing 6:d60ebfa0f1cb "e" (e)
+  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this would cause divergence
+  $ hg log -G -r 'a'::
+  o  11:eb6d63fc4ed5 e
+  |
+  o  10:44d8c724a70c d'
+  |
+  o  9:d008e6b4d3fd c
+  |
+  o  8:67e8f4a16c49 b
+  |
+  | o  7:1143e9adc121 f
+  | |
+  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
+  | | |
+  | | x  5:027ad6c5830d d' (rewritten using rebase as 10:44d8c724a70c)
+  | | |
+  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
+  | |/
+  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
+  | |
+  o |  2:630d7c95eff7 x
+  | |
+  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
+  |/
+  o  0:b173517d0057 a
+  
+  $ hg strip -r 8:
+
+If the rebase set has an obsolete (d) with a successor (d') outside the rebase
+set and none in destination, we still get the divergence warning.
+By allowing divergence, we can perform the rebase.
+
+  $ hg rebase -r 'c'::'f' -d 'x'
+  abort: this rebase will cause divergences from: 76be324c128b
+  (to force the rebase please set experimental.evolution.allowdivergence=True)
+  [255]
+  $ hg rebase --config experimental.evolution.allowdivergence=true -r 'c'::'f' -d 'x'
+  rebasing 3:a82ac2b38757 "c" (c)
+  rebasing 4:76be324c128b "d" (d)
+  rebasing 7:1143e9adc121 "f" (f tip)
+  $ hg log -G -r 'a':: -T instabilities
+  o  10:e1744ea07510 f
+  |
+  o  9:e2b36ea9a0a0 d (content-divergent)
+  |
+  o  8:6a0376de376e c
+  |
+  | x  7:1143e9adc121 f
+  | |
+  | | o  6:d60ebfa0f1cb e (orphan)
+  | | |
+  | | o  5:027ad6c5830d d' (orphan content-divergent)
+  | | |
+  | x |  4:76be324c128b d
+  | |/
+  | x  3:a82ac2b38757 c
+  | |
+  o |  2:630d7c95eff7 x
+  | |
+  | o  1:488e1b7e7341 b
+  |/
+  o  0:b173517d0057 a
+  
+  $ hg strip -r 8:
+
+(Not skipping obsoletes means that divergence is allowed.)
+
+  $ hg rebase --config experimental.rebaseskipobsolete=false -r 'c'::'f' -d 'x'
+  rebasing 3:a82ac2b38757 "c" (c)
+  rebasing 4:76be324c128b "d" (d)
+  rebasing 7:1143e9adc121 "f" (f tip)
+
+  $ hg strip -r 0:
+
+Similar test on a more complex graph
+
+  $ hg debugdrawdag <<EOF
+  >       g
+  >       |
+  >   f   e
+  >   |   |
+  >   e'  d # replace: e -> e'
+  >    \ /
+  >     c
+  >     |
+  >   x b
+  >    \|
+  >     a
+  > EOF
+  $ hg log -G -r 'a':
+  o  8:2876ce66c6eb g
+  |
+  | o  7:3ffec603ab53 f
+  | |
+  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
+  | |
+  | o  5:63324dc512ea e'
+  | |
+  o |  4:76be324c128b d
+  |/
+  o  3:a82ac2b38757 c
+  |
+  | o  2:630d7c95eff7 x
+  | |
+  o |  1:488e1b7e7341 b
+  |/
+  o  0:b173517d0057 a
+  
+  $ hg rebase -b 'f' -d 'x'
+  rebasing 1:488e1b7e7341 "b" (b)
+  rebasing 3:a82ac2b38757 "c" (c)
+  rebasing 5:63324dc512ea "e'" (e')
+  rebasing 7:3ffec603ab53 "f" (f)
+  rebasing 4:76be324c128b "d" (d)
+  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as this would cause divergence
+  $ hg log -G -r 'a':
+  o  13:a1707a5b7c2c d
+  |
+  | o  12:ef6251596616 f
+  | |
+  | o  11:b6f172e64af9 e'
+  |/
+  o  10:d008e6b4d3fd c
+  |
+  o  9:67e8f4a16c49 b
+  |
+  | o  8:2876ce66c6eb g
+  | |
+  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
+  | | |
+  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
+  | | |
+  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
+  | | |
+  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
+  | |/
+  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
+  | |
+  o |  2:630d7c95eff7 x
+  | |
+  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
+  |/
+  o  0:b173517d0057 a
+  
+
+  $ cd ..
+
 Rebase merge where successor of one parent is equal to destination (issue5198)
 
   $ hg init p1-succ-is-dest


More information about the Mercurial-devel mailing list