changeset 34026:9422107a6b64

rebase: move working parent and bookmark for obsoleted revs (BC)

Previously, obsoleted revs with successors in destination are completely
ignored. That caused some inconvenience when working copy is obsoleted. Most
commands avoid working copy being obsoleted, but `hg pull` is an exception.

This patch makes rebase able to move bookmarks or working parent for those
obsoleted revs. It does so by keeping the obsoleted revs in `state` and
marking them as "skipped, rebased to desired destination" during run-time.

This reverts part of the behavior change of 3b7cb3d17137 and D24.

Differential Revision: https://phab.mercurial-scm.org/D527

--- a/hgext/rebase.py	Tue Aug 29 17:49:13 2017 -0700
+++ b/hgext/rebase.py	Sun Aug 27 02:47:47 2017 -0700
@@ -327,11 +327,7 @@
                   " unrebased descendants"),
                 hint=_('use --keep to keep original changesets'))
 
-        obsrevs = _filterobsoleterevs(self.repo, rebaseset)
-        self._handleskippingobsolete(obsrevs, destmap)
-
-        result = buildstate(self.repo, destmap, self.collapsef,
-                            self.obsoletenotrebased)
+        result = buildstate(self.repo, destmap, self.collapsef)
 
         if not result:
             # Empty state built, nothing to rebase
@@ -375,6 +371,10 @@
                         raise error.Abort(_('cannot collapse multiple named '
                             'branches'))
 
+        # Calculate self.obsoletenotrebased
+        obsrevs = _filterobsoleterevs(self.repo, self.state)
+        self._handleskippingobsolete(obsrevs, self.destmap)
+
         # Keep track of the active bookmarks in order to reset them later
         self.activebookmark = self.activebookmark or repo._activebookmark
         if self.activebookmark:
@@ -401,13 +401,34 @@
             desc = _ctxdesc(ctx)
             if self.state[rev] == rev:
                 ui.status(_('already rebased %s\n') % desc)
+            elif rev in self.obsoletenotrebased:
+                succ = self.obsoletenotrebased[rev]
+                if succ is None:
+                    msg = _('note: not rebasing %s, it has no '
+                            'successor\n') % desc
+                else:
+                    succctx = repo[succ]
+                    succdesc = '%d:%s "%s"' % (
+                        succctx.rev(), succctx,
+                        succctx.description().split('\n', 1)[0])
+                    msg = (_('note: not rebasing %s, already in '
+                             'destination as %s\n') % (desc, succdesc))
+                repo.ui.status(msg)
+                # Make clearrebased aware state[rev] is not a true successor
+                self.skipped.add(rev)
+                # Record rev as moved to its desired destination in self.state.
+                # This helps bookmark and working parent movement.
+                dest = max(adjustdest(repo, rev, self.destmap, self.state,
+                                      self.skipped))
+                self.state[rev] = dest
             elif self.state[rev] == revtodo:
                 pos += 1
                 ui.status(_('rebasing %s\n') % desc)
                 ui.progress(_("rebasing"), pos, ("%d:%s" % (rev, ctx)),
                             _('changesets'), total)
                 p1, p2, base = defineparents(repo, rev, self.destmap,
-                                             self.state)
+                                             self.state, self.skipped,
+                                             self.obsoletenotrebased)
                 self.storestatus(tr=tr)
                 storecollapsemsg(repo, self.collapsemsg)
                 if len(repo[None].parents()) == 2:
@@ -462,7 +483,8 @@
         repo, ui, opts = self.repo, self.ui, self.opts
         if self.collapsef and not self.keepopen:
             p1, p2, _base = defineparents(repo, min(self.state), self.destmap,
-                                          self.state)
+                                          self.state, self.skipped,
+                                          self.obsoletenotrebased)
             editopt = opts.get('edit')
             editform = 'rebase.collapse'
             if self.collapsemsg:
@@ -935,7 +957,7 @@
         copies.duplicatecopies(repo, rev, p1rev, skiprev=dest)
     return stats
 
-def adjustdest(repo, rev, destmap, state):
+def adjustdest(repo, rev, destmap, state, skipped):
     """adjust rebase destination given the current rebase state
 
     rev is what is being rebased. Return a list of two revs, which are the
@@ -989,7 +1011,8 @@
     """
     # pick already rebased revs with same dest from state as interesting source
     dest = destmap[rev]
-    source = [s for s, d in state.items() if d > 0 and destmap[s] == dest]
+    source = [s for s, d in state.items()
+              if d > 0 and destmap[s] == dest and s not in skipped]
 
     result = []
     for prev in repo.changelog.parentrevs(rev):
@@ -1037,7 +1060,7 @@
         if s in nodemap:
             yield nodemap[s]
 
-def defineparents(repo, rev, destmap, state):
+def defineparents(repo, rev, destmap, state, skipped, obsskipped):
     """Return new parents and optionally a merge base for rev being rebased
 
     The destination specified by "dest" cannot always be used directly because
@@ -1064,7 +1087,7 @@
     dest = destmap[rev]
     oldps = repo.changelog.parentrevs(rev) # old parents
     newps = [nullrev, nullrev] # new parents
-    dests = adjustdest(repo, rev, destmap, state) # adjusted destinations
+    dests = adjustdest(repo, rev, destmap, state, skipped)
     bases = list(oldps) # merge base candidates, initially just old parents
 
     if all(r == nullrev for r in oldps[1:]):
@@ -1191,7 +1214,8 @@
             # If those revisions are covered by rebaseset, the result is good.
             # A merge in rebaseset would be considered to cover its ancestors.
             if siderevs:
-                rebaseset = [r for r, d in state.items() if d > 0]
+                rebaseset = [r for r, d in state.items()
+                             if d > 0 and r not in obsskipped]
                 merges = [r for r in rebaseset
                           if cl.parentrevs(r)[1] != nullrev]
                 unwanted[i] = list(repo.revs('%ld - (::%ld) - %ld',
@@ -1408,7 +1432,7 @@
         srcset -= set(result)
         yield result
 
-def buildstate(repo, destmap, collapse, obsoletenotrebased):
+def buildstate(repo, destmap, collapse):
     '''Define which revisions are going to be rebased and where
 
     repo: repo
@@ -1469,23 +1493,6 @@
         # if all parents of this revision are done, then so is this revision
         if parents and all((state.get(p) == p for p in parents)):
             state[rev] = rev
-    unfi = repo.unfiltered()
-    for r in obsoletenotrebased:
-        desc = _ctxdesc(unfi[r])
-        succ = obsoletenotrebased[r]
-        if succ is None:
-            msg = _('note: not rebasing %s, it has no successor\n') % desc
-            del state[r]
-            del destmap[r]
-        else:
-            destctx = unfi[succ]
-            destdesc = '%d:%s "%s"' % (destctx.rev(), destctx,
-                                       destctx.description().split('\n', 1)[0])
-            msg = (_('note: not rebasing %s, already in destination as %s\n')
-                   % (desc, destdesc))
-            del state[r]
-            del destmap[r]
-        repo.ui.status(msg)
     return originalwd, destmap, state
 
 def clearrebased(ui, repo, destmap, state, skipped, collapsedas=None):

--- a/tests/test-rebase-obsolete.t	Tue Aug 29 17:49:13 2017 -0700
+++ b/tests/test-rebase-obsolete.t	Sun Aug 27 02:47:47 2017 -0700
@@ -205,8 +205,8 @@
   o  0:cd010b8cd998 A
   
   $ hg rebase --source 'desc(B)' --dest 'tip' --config experimental.rebaseskipobsolete=True
+  rebasing 8:8877864f1edb "B"
   note: not rebasing 9:08483444fef9 "D", already in destination as 11:4596109a6a43 "D"
-  rebasing 8:8877864f1edb "B"
   rebasing 10:5ae4c968c6ac "C"
   $ hg debugobsolete
   42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
@@ -736,8 +736,8 @@
   $ hg debugobsolete `hg log -r 7 -T '{node}\n'` --config experimental.stabilization=all
   obsoleted 1 changesets
   $ hg rebase -d 6 -r "4::"
+  rebasing 4:ff2c4d47b71d "C"
   note: not rebasing 7:360bbaa7d3ce "O", it has no successor
-  rebasing 4:ff2c4d47b71d "C"
   rebasing 8:8d47583e023f "P" (tip)
 
 If all the changeset to be rebased are obsolete and present in the destination, we
@@ -769,10 +769,8 @@
 If a rebase is going to create divergence, it should abort
 
   $ hg log -G
-  @  11:f44da1f4954c nonrelevant
+  @  10:121d9e3bc4c6 P
   |
-  | o  10:121d9e3bc4c6 P
-  |/
   o  9:4be60e099a77 C
   |
   o  6:9c48361117de D
@@ -904,7 +902,6 @@
   |
   ~
   $ hg rebase -r ".^^ + .^ + ." -d 19
-  note: not rebasing 21:8b31da3c4919 "dummy change", already in destination as 19:601db7a18f51 "dummy change successor"
   rebasing 20:b82fb57ea638 "willconflict second version"
   merging willconflict
   warning: conflicts while merging willconflict! (edit, then use 'hg resolve --mark')
@@ -916,6 +913,7 @@
   continue: hg rebase --continue
   $ hg rebase --continue
   rebasing 20:b82fb57ea638 "willconflict second version"
+  note: not rebasing 21:8b31da3c4919 "dummy change", already in destination as 19:601db7a18f51 "dummy change successor"
   rebasing 22:7bdc8a87673d "dummy change" (tip)
   $ cd ..
 
@@ -1062,8 +1060,8 @@
   > EOF
 
   $ hg rebase -d C -b F
+  rebasing 2:b18e25de2cf5 "D" (D)
   note: not rebasing 3:7fb047a69f22 "E" (E), already in destination as 1:112478962961 "B"
-  rebasing 2:b18e25de2cf5 "D" (D)
   rebasing 5:66f1a38021c9 "F" (F tip)
   note: rebase of 5:66f1a38021c9 created no changes to commit
   $ hg log -G
@@ -1179,8 +1177,8 @@
   > A B C  # D/D = D
   > EOS
   $ hg rebase -r B+A+D -d Z
+  rebasing 0:426bada5c675 "A" (A)
   note: not rebasing 1:fc2b737bb2e5 "B" (B), already in destination as 2:96cc3511f894 "C"
-  rebasing 0:426bada5c675 "A" (A)
   rebasing 3:b8ed089c80ad "D" (D)
 
   $ rm .hg/localtags
@@ -1226,17 +1224,46 @@
   2:1e9a3c00cbe9 b (no-eol)
   $ hg rebase -r 2 -d 3 --config experimental.stabilization.track-operation=1
   note: not rebasing 2:1e9a3c00cbe9 "b" (mybook), already in destination as 3:be1832deae9a "b"
-Check that working directory was not updated to rev 3 because rev 2 was skipped
-during the rebase operation
+Check that working directory and bookmark was updated to rev 3 although rev 2
+was skipped
   $ hg log -r .
-  2:1e9a3c00cbe9 b (no-eol)
-
-Check that bookmark was not moved to rev 3 if rev 2 was skipped during the
-rebase operation. This makes sense because if rev 2 has a successor, the
-operation generating that successor (ex. rebase) should be responsible for
-moving bookmarks. If the bookmark is on a precursor, like rev 2, that means the
-user manually moved it back. In that case we should not move it again.
+  3:be1832deae9a b (no-eol)
   $ hg bookmarks
-     mybook                    2:1e9a3c00cbe9
+     mybook                    3:be1832deae9a
   $ hg debugobsolete --rev tip
   1e9a3c00cbe90d236ac05ef61efcc5e40b7412bc be1832deae9ac531caa7438b8dcf6055a122cd8e 0 (*) {'user': 'test'} (glob)
+
+Obsoleted working parent and bookmark could be moved if an ancestor of working
+parent gets moved:
+
+  $ hg init $TESTTMP/ancestor-wd-move
+  $ cd $TESTTMP/ancestor-wd-move
+  $ hg debugdrawdag <<'EOS'
+  >  E D1  # rebase: D1 -> D2
+  >  | |
+  >  | C
+  > D2 |
+  >  | B
+  >  |/
+  >  A
+  > EOS
+  $ hg update D1 -q
+  $ hg bookmark book -i
+  $ hg rebase -r B+D1 -d E
+  rebasing 1:112478962961 "B" (B)
+  note: not rebasing 5:15ecf15e0114 "D1" (D1 tip book), already in destination as 2:0807738e0be9 "D2"
+  $ hg log -G -T '{desc} {bookmarks}'
+  @  B book
+  |
+  | x  D1
+  | |
+  o |  E
+  | |
+  | o  C
+  | |
+  o |  D2
+  | |
+  | x  B
+  |/
+  o  A
+