D10249: rebase: filter out descendants of divergence-causing commits earlier

[martinvonz (Martin von Zweigbergk)]

martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  `hg rebase` treats obsolete commits differently depending what has
  happened to the commit:
  
  1. Obsolete commit without non-obsolete successors: Skipped, and a note is printed ("it has no successor").
  2. Obsolete commit with a successor in the destination (ancestor of it): Skipped, and a note is printed ("already in destination").
  3. Obsolete commit with a successor in the rebase set: Error ("this rebase will cause divergences"), unless `allowdivergence` config set.
  4. Obsolete commit with a succesor elsewhere: The commit and its descendants are skipped, and a note is printed ("not rebasing <commit> and its descedants as this would cause divergence"), unless `allowdivergence` config set.
  
  Before this patch, we did all those checks up front, except for (4),
  which was checked later. The later check consisted of two parts: 1)
  filtering out of descendants, and 2) conditionally printing message if
  the `allowdivergence` config was not set. This patch makes it so we do
  the filtering early.
  
  A consequence of filtering out divergence-causing commits earlier is
  that we rebase commits in slightly different order, which has some
  impact on tests.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D10249

AFFECTED FILES
  hgext/rebase.py
  tests/test-rebase-obsolete.t

CHANGE DETAILS

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
@@ -1098,9 +1098,9 @@
   $ hg rebase -b 'e' -d 'x'
   rebasing 1:488e1b7e7341 b "b"
   rebasing 3:a82ac2b38757 c "c"
+  note: not rebasing 4:76be324c128b d "d" and its descendants as this would cause divergence
   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
   |
@@ -1233,16 +1233,16 @@
   $ 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
+  rebasing 5:63324dc512ea e' "e'"
+  rebasing 7:3ffec603ab53 f "f"
   $ hg log -G -r 'a':
-  o  13:a1707a5b7c2c d
+  o  13:ef6251596616 f
   |
-  | o  12:ef6251596616 f
-  | |
-  | o  11:b6f172e64af9 e'
+  o  12:b6f172e64af9 e'
+  |
+  | o  11:a1707a5b7c2c d
   |/
   o  10:d008e6b4d3fd c
   |
@@ -1250,13 +1250,13 @@
   |
   | *  8:2876ce66c6eb g
   | |
-  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
+  | | x  7:3ffec603ab53 f (rewritten using rebase as 13:ef6251596616)
   | | |
   | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
   | | |
-  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
+  | | x  5:63324dc512ea e' (rewritten using rebase as 12:b6f172e64af9)
   | | |
-  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
+  | x |  4:76be324c128b d (rewritten using rebase as 11:a1707a5b7c2c)
   | |/
   | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
   | |
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -362,6 +362,19 @@
         skippedset = set(self.obsolete_with_successor_in_destination)
         skippedset.update(self.obsolete_would_cause_divergence)
         _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
+        allowdivergence = self.ui.configbool(
+            b'experimental', b'evolution.allowdivergence'
+        )
+        if allowdivergence:
+            self.obsolete_would_cause_divergence = set()
+        else:
+            for rev in self.repo.revs(
+                b'descendants(%ld) and not %ld',
+                self.obsolete_would_cause_divergence,
+                self.obsolete_would_cause_divergence,
+            ):
+                self.state.pop(rev, None)
+                self.destmap.pop(rev, None)
 
     def _prepareabortorcontinue(
         self, isabort, backup=True, suppwarns=False, dryrun=False, confirm=False
@@ -494,19 +507,10 @@
         def progress(ctx):
             p.increment(item=(b"%d:%s" % (ctx.rev(), ctx)))
 
-        allowdivergence = self.ui.configbool(
-            b'experimental', b'evolution.allowdivergence'
-        )
         for subset in sortsource(self.destmap):
             sortedrevs = self.repo.revs(b'sort(%ld, -topo)', subset)
-            if not allowdivergence:
-                sortedrevs -= self.repo.revs(
-                    b'descendants(%ld) and not %ld',
-                    self.obsolete_would_cause_divergence,
-                    self.obsolete_would_cause_divergence,
-                )
             for rev in sortedrevs:
-                self._rebasenode(tr, rev, allowdivergence, progress)
+                self._rebasenode(tr, rev, progress)
         p.complete()
         ui.note(_(b'rebase merging completed\n'))
 
@@ -568,15 +572,13 @@
 
             return newnode
 
-    def _rebasenode(self, tr, rev, allowdivergence, progressfn):
+    def _rebasenode(self, tr, rev, progressfn):
         repo, ui, opts = self.repo, self.ui, self.opts
         ctx = repo[rev]
         desc = _ctxdesc(ctx)
         if self.state[rev] == rev:
             ui.status(_(b'already rebased %s\n') % desc)
-        elif (
-            not allowdivergence and rev in self.obsolete_would_cause_divergence
-        ):
+        elif rev in self.obsolete_would_cause_divergence:
             msg = (
                 _(
                     b'note: not rebasing %s and its descendants as '



To: martinvonz, #hg-reviewers
Cc: mercurial-patches, mercurial-devel