Patchwork D7176: rebase: allow rebasing obsolete commit without successor

login
register
mail settings
Submitter phabricator
Date Oct. 26, 2019, 12:49 a.m.
Message ID <differential-rev-PHID-DREV-tb6hjeswylnhipdacz3y-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42594/
State New
Headers show

Comments

phabricator - Oct. 26, 2019, 12:49 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  When trying to rebase an obsolete whose successors are also all
  obsolete, we would skip it and tell the user that it would have caused
  divergence. That is not correct. We could just change the message, but
  I don't see any harm in rebasing such commits. It even seems like a
  reasonable way to make it not obsolete. So this patch makes that type
  of rebase allowed.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Oct. 30, 2019, 7:59 p.m.
marmoute added a comment.
marmoute added subscribers: khanchi97, marmoute.


  That is not necessarly True, If the changeset is pruned, but successors of a public changeset, rebasing it would create phase-divergence. @khanchi97 fixed similar cases in September.
  
  Overall, we should migrate the rebase code to use the `precheck` logic in rewrite util (and make sure things are well covered there).
  
  One can read more about current Sushil work in this area here: https://www.mercurial-scm.org/wiki/CEDPrecheckPlan.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7176/new/

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

To: martinvonz, #hg-reviewers
Cc: marmoute, khanchi97, mercurial-devel
phabricator - Jan. 30, 2020, 6:06 p.m.
This revision now requires changes to proceed.
baymax added a comment.
baymax requested changes to this revision.


  There seems to have been no activities on this Diff for the past 3 Months.
  
  By policy, we are automatically moving it out of the `need-review` state.
  
  Please, move it back to `need-review` without hesitation if this diff should still be discussed.
  
  :baymax:need-review-idle:

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7176/new/

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

To: martinvonz, #hg-reviewers, baymax
Cc: marmoute, khanchi97, 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
@@ -1538,9 +1538,8 @@ 
   1 new obsolescence markers
   obsoleted 1 changesets
   $ hg tag --local --remove E
-BROKEN: this rebase would not cause divergence
   $ hg rebase -d D -s C
-  note: not rebasing 3:26805aba1e60 "C" (C) and its descendants as this would cause divergence
+  rebasing 3:26805aba1e60 "C" (C)
 
   $ cd ..
 
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -204,6 +204,7 @@ 
         self.keepbranchesf = opts.get(b'keepbranches', False)
         self.obsoletenotrebased = {}
         self.obsoletewithoutsuccessorindestination = set()
+        self.obsoleteextinctsuccessors = set()
         self.inmemory = inmemory
         self.stateobj = statemod.cmdstate(repo, b'rebasestate')
 
@@ -357,11 +358,11 @@ 
         (
             self.obsoletenotrebased,
             self.obsoletewithoutsuccessorindestination,
-            obsoleteextinctsuccessors,
+            self.obsoleteextinctsuccessors,
         ) = _computeobsoletenotrebased(self.repo, obsoleteset, destmap)
         skippedset = set(self.obsoletenotrebased)
         skippedset.update(self.obsoletewithoutsuccessorindestination)
-        skippedset.update(obsoleteextinctsuccessors)
+        skippedset.update(self.obsoleteextinctsuccessors)
         _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
 
     def _prepareabortorcontinue(self, isabort, backup=True, suppwarns=False):
@@ -576,6 +577,7 @@ 
         elif (
             not allowdivergence
             and rev in self.obsoletewithoutsuccessorindestination
+            and rev not in self.obsoleteextinctsuccessors
         ):
             msg = (
                 _(