Patchwork D10240: rebase: skip obsolete commits even if they have pruned successors

login
register
mail settings
Submitter phabricator
Date March 19, 2021, 7:36 p.m.
Message ID <differential-rev-PHID-DREV-bljfn6p47abirdybiohw-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48556/
State Superseded
Headers show

Comments

phabricator - March 19, 2021, 7:36 p.m.
martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  Issue 5782 reported that `hg rebase -r <obsolete commit with pruned
  successor>` failed with an error saying that it would cause
  divergence. Commit a603a570cdbe <https://phab.mercurial-scm.org/rHGa603a570cdbeb19ae958585599200b05ac419d0c> (rebase: add a test case for
  issue5782, 2018-02-07) fixed it by letting you rebase the
  commit. However, that fix seems inconsistent with how we handle `hg
  rebase -r <pruned commit>`. To me, it should make no difference
  whether a commit is pruned itself or if it has (only) pruned
  successors. This patch changes it so we treat these two kinds of
  commits the same way. I let the message we print remain "note: not
  rebasing <commit>, it has no successor" even though that last part is
  not technically correct for commits with pruned successors. I doubt it
  will confuse users.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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

CHANGE DETAILS




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

Patch

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
@@ -1294,18 +1294,16 @@ 
   o  0:b173517d0057 a
   
   $ hg rebase -d 0 -r 2
-  rebasing 2:a82ac2b38757 c "c"
+  note: not rebasing 2:a82ac2b38757 c "c", it has no successor
   $ hg log -G -r 'a': --hidden
-  o  5:69ad416a4a26 c
+  *  4:76be324c128b d
   |
-  | *  4:76be324c128b d
+  | x  3:ef8a456de8fa c1 (pruned)
   | |
-  | | x  3:ef8a456de8fa c1 (pruned)
-  | | |
-  | x |  2:a82ac2b38757 c (rewritten using replace as 3:ef8a456de8fa rewritten using rebase as 5:69ad416a4a26)
-  | |/
-  | o  1:488e1b7e7341 b
+  x |  2:a82ac2b38757 c (rewritten using replace as 3:ef8a456de8fa)
   |/
+  o  1:488e1b7e7341 b
+  |
   o  0:b173517d0057 a
   
   $ cd ..
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -361,11 +361,9 @@ 
         (
             self.obsoletenotrebased,
             self.obsoletewithoutsuccessorindestination,
-            obsoleteextinctsuccessors,
         ) = _computeobsoletenotrebased(self.repo, obsoleteset, destmap)
         skippedset = set(self.obsoletenotrebased)
         skippedset.update(self.obsoletewithoutsuccessorindestination)
-        skippedset.update(obsoleteextinctsuccessors)
         _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
 
     def _prepareabortorcontinue(
@@ -2192,13 +2190,9 @@ 
 
     `obsoletewithoutsuccessorindestination` is a set with obsolete revisions
     without a successor in destination.
-
-    `obsoleteextinctsuccessors` is a set of obsolete revisions with only
-    obsolete successors.
     """
     obsoletenotrebased = {}
     obsoletewithoutsuccessorindestination = set()
-    obsoleteextinctsuccessors = set()
 
     assert repo.filtername is None
     cl = repo.changelog
@@ -2212,11 +2206,8 @@ 
         successors.remove(srcnode)
         succrevs = {get_rev(s) for s in successors}
         succrevs.discard(None)
-        if succrevs.issubset(extinctrevs):
-            # all successors are extinct
-            obsoleteextinctsuccessors.add(srcrev)
-        if not successors:
-            # no successor
+        if not successors or succrevs.issubset(extinctrevs):
+            # no successor, or all successors are extinct
             obsoletenotrebased[srcrev] = None
         else:
             dstrev = destmap[srcrev]
@@ -2231,11 +2222,7 @@ 
                 if srcrev in extinctrevs or any(s in destmap for s in succrevs):
                     obsoletewithoutsuccessorindestination.add(srcrev)
 
-    return (
-        obsoletenotrebased,
-        obsoletewithoutsuccessorindestination,
-        obsoleteextinctsuccessors,
-    )
+    return obsoletenotrebased, obsoletewithoutsuccessorindestination
 
 
 def abortrebase(ui, repo):