Patchwork D4409: rebase: skip *all* obsolete revisions

login
register
mail settings
Submitter phabricator
Date Aug. 28, 2018, 7:33 a.m.
Message ID <differential-rev-PHID-DREV-llasgyo2tik56lvojui4-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/34115/
State New
Headers show

Comments

phabricator - Aug. 28, 2018, 7:33 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  We already skip obsolete revisions in some case: if there is no
  successor, or when the successor is in the rebase set or in the
  destination, I think. However, we instead error out if the successor
  is elsewhere (e.g. a child or a sibling of the destination). The
  previous commit fixed one specific case of this.
  
  Consider this history (based on a test case updated by this patch):
  
  o  D
  
  |
  | o  B' |
  | /     |
  | o  C  |
  |       |
  | x  B  |
  | /     |
  |
  
  o A
  
  If the user now runs `hg rebase -s B -d D` we would error out and tell
  them that it would cause divergence from B. This patch makes it so we
  instead ignore B and C (and tell the user that we're doing that
  because it would cause divergence). I think this simpler model will be
  easier for users to understand (I had also thought that all obsolete
  commits and their descendants were skipped until we got reports from
  users that that wasn't the case).
  
  This should also let us simplify the source quite a bit. I'll do that
  in later commits after I've heard people's thoughts on this one.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - Aug. 29, 2018, 12:55 p.m.
> @@ -1289,18 +1286,16 @@
>    o  0:b173517d0057 a
>    
>    $ hg rebase -d 0 -r 2
> -  rebasing 2:a82ac2b38757 "c" (c)
> +  note: not rebasing 2:a82ac2b38757 "c" (c) and its descendants as this would cause divergence

Looks like the fix for issue5782 regressed.

https://bz.mercurial-scm.org/show_bug.cgi?id=5782
phabricator - Aug. 29, 2018, 12:55 p.m.
yuja added a comment.


  > @@ -1289,18 +1286,16 @@
  > 
  >   o  0:b173517d0057 a
  >   
  >   $ hg rebase -d 0 -r 2
  > 
  > - rebasing 2:a82ac2b38757 "c" (c) +  note: not rebasing 2:a82ac2b38757 "c" (c) and its descendants as this would cause divergence
  
  Looks like the fix for issue5782 regressed.
  
  https://bz.mercurial-scm.org/show_bug.cgi?id=5782

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - Aug. 29, 2018, 2 p.m.
lothiraldan added a comment.


  Changing the behavior of `--src` to ignore obsolete and orphan unrelated to the destination seems like a good idea.
  
  However, this change seems to also affect `--rev`. The `--rev` allow for a precise selection of what the user wants to see rebased and it feels like we should stick to the user wishes here. Dropping explicitly selected revisions feels wrong and will make some use cases harder to perform.

REPOSITORY
  rHG Mercurial

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

To: martinvonz, #hg-reviewers
Cc: lothiraldan, yuja, 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
@@ -943,9 +943,7 @@ 
   phases: 8 draft
   orphan: 1 changesets
   $ hg rebase -s 10 -d 12
-  abort: this rebase will cause divergences from: 121d9e3bc4c6
-  (to force the rebase please set experimental.evolution.allowdivergence=True)
-  [255]
+  note: not rebasing 10:121d9e3bc4c6 "P" and its descendants as this would cause divergence
   $ hg log -G
   @  14:73568ab6879d bar foo
   |
@@ -1146,14 +1144,13 @@ 
 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]
+  rebasing 3:a82ac2b38757 "c" (c)
+  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this would cause divergence
+  2 new orphan changesets
   $ 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)
-  1 new orphan changesets
+  rebasing 7:1143e9adc121 "f" (f)
   2 new content-divergent changesets
   $ hg log -G -r 'a':: -T instabilities
   o  10:e1744ea07510 f
@@ -1289,18 +1286,16 @@ 
   o  0:b173517d0057 a
   
   $ hg rebase -d 0 -r 2
-  rebasing 2:a82ac2b38757 "c" (c)
+  note: not rebasing 2:a82ac2b38757 "c" (c) and its descendants as this would cause divergence
   $ 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 ..
@@ -1559,9 +1554,7 @@ 
   1 new orphan changesets
 
   $ hg rebase -b 'desc("D")' -d 'desc("J")'
-  abort: this rebase will cause divergences from: 112478962961
-  (to force the rebase please set experimental.evolution.allowdivergence=True)
-  [255]
+  note: not rebasing 2:112478962961 "B" (B) and its descendants as this would cause divergence
 
 Rebase merge where both parents have successors in destination
 
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1888,11 +1888,10 @@ 
                     obsoletenotrebased[srcrev] = succrev
                     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 srcrev in extinctrevs or any(s in destmap for s in succrevs):
-                    obsoletewithoutsuccessorindestination.add(srcrev)
+                # If 'srcrev' has no successor in destination (which would be
+                # caught above), we shall skip it and its descendants to
+                # avoid divergence.
+                obsoletewithoutsuccessorindestination.add(srcrev)
 
     return (
         obsoletenotrebased,