changeset 33590:52f82e7d6a7e stable

rebase: move bookmark to destination for commits becoming empty (issue5627) When rebasing a changeset X and that changeset becomes empty, we should move the bookmark on X to rebase destination. This is a regression caused by the scmutil.cleanupnodes refactoring for rebase. The `adjustdest` function calculates the destination of bookmark movement. It was back-ported from https://phab.mercurial-scm.org/D21. It might be slightly more powerful than the minimal requirement to solve this issue. For example, it's impossible for a merge changeset to become empty while any of its ancestors does not become empty, but the code could handle that case. Since the code is reasonably short and clean, and helps the upcoming D21 series, I'd like to check-in `adjustdest` now. Thanks Martin von Zweigbergk for spotting corner cases (-k and descendant with bookmarks) in this area!
author Jun Wu <quark@fb.com>
date Mon, 24 Jul 2017 23:52:56 -0700
parents a0bfcd08f5fe
children ee11d18fcd3c
files hgext/rebase.py tests/test-rebase-emptycommit.t
diffstat 2 files changed, 266 insertions(+), 2 deletions(-) [+]
line wrap: on
line diff
--- a/hgext/rebase.py	Wed Jul 26 23:39:42 2017 +0900
+++ b/hgext/rebase.py	Mon Jul 24 23:52:56 2017 -0700
@@ -512,7 +512,8 @@
             collapsedas = None
             if self.collapsef:
                 collapsedas = newnode
-            clearrebased(ui, repo, self.state, self.skipped, collapsedas)
+            clearrebased(ui, repo, self.dest, self.state, self.skipped,
+                         collapsedas)
 
         clearstatus(repo)
         clearcollapsemsg(repo)
@@ -897,6 +898,58 @@
         copies.duplicatecopies(repo, rev, p1rev, skiprev=dest)
     return stats
 
+def adjustdest(repo, rev, dest, state):
+    """adjust rebase destination given the current rebase state
+
+    rev is what is being rebased. Return a list of two revs, which are the
+    adjusted destinations for rev's p1 and p2, respectively. If a parent is
+    nullrev, return dest without adjustment for it.
+
+    For example, when doing rebase -r B+E -d F, rebase will first move B to B1,
+    and E's destination will be adjusted from F to B1.
+
+        B1 <- written during rebasing B
+        |
+        F <- original destination of B, E
+        |
+        | E <- rev, which is being rebased
+        | |
+        | D <- prev, one parent of rev being checked
+        | |
+        | x <- skipped, ex. no successor or successor in (::dest)
+        | |
+        | C
+        | |
+        | B <- rebased as B1
+        |/
+        A
+
+    Another example about merge changeset, rebase -r C+G+H -d K, rebase will
+    first move C to C1, G to G1, and when it's checking H, the adjusted
+    destinations will be [C1, G1].
+
+            H       C1 G1
+           /|       | /
+          F G       |/
+        K | |  ->   K
+        | C D       |
+        | |/        |
+        | B         | ...
+        |/          |/
+        A           A
+    """
+    result = []
+    for prev in repo.changelog.parentrevs(rev):
+        adjusted = dest
+        if prev != nullrev:
+            # pick already rebased revs from state
+            source = [s for s, d in state.items() if d > 0]
+            candidate = repo.revs('max(%ld and (::%d))', source, prev).first()
+            if candidate is not None:
+                adjusted = state[candidate]
+        result.append(adjusted)
+    return result
+
 def nearestrebased(repo, rev, state):
     """return the nearest ancestors of rev in the rebase result"""
     rebased = [r for r in state if state[r] > nullmerge]
@@ -1301,12 +1354,21 @@
             state[r] = revprecursor
     return originalwd, dest.rev(), state
 
-def clearrebased(ui, repo, state, skipped, collapsedas=None):
+def clearrebased(ui, repo, dest, state, skipped, collapsedas=None):
     """dispose of rebased revision at the end of the rebase
 
     If `collapsedas` is not None, the rebase was a collapse whose result if the
     `collapsedas` node."""
     tonode = repo.changelog.node
+    # Move bookmark of skipped nodes to destination. This cannot be handled
+    # by scmutil.cleanupnodes since it will treat rev as removed (no successor)
+    # and move bookmark backwards.
+    bmchanges = [(name, tonode(max(adjustdest(repo, rev, dest, state))))
+                 for rev in skipped
+                 for name in repo.nodebookmarks(tonode(rev))]
+    if bmchanges:
+        with repo.transaction('rebase') as tr:
+            repo._bookmarks.applychanges(repo, tr, bmchanges)
     mapping = {}
     for rev, newrev in sorted(state.items()):
         if newrev >= 0 and newrev != rev:
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/tests/test-rebase-emptycommit.t	Mon Jul 24 23:52:56 2017 -0700
@@ -0,0 +1,202 @@
+  $ cat >> $HGRCPATH<<EOF
+  > [extensions]
+  > rebase=
+  > drawdag=$TESTDIR/drawdag.py
+  > EOF
+
+  $ hg init non-merge
+  $ cd non-merge
+  $ hg debugdrawdag<<'EOS'
+  >   F
+  >   |
+  >   E
+  >   |
+  >   D
+  >   |
+  > B C
+  > |/
+  > A
+  > EOS
+
+  $ for i in C D E F; do
+  >   hg bookmark -r $i -i BOOK-$i
+  > done
+
+  $ hg debugdrawdag<<'EOS'
+  > E
+  > |
+  > D
+  > |
+  > B
+  > EOS
+
+  $ hg log -G -T '{rev} {desc} {bookmarks}'
+  o  7 E
+  |
+  o  6 D
+  |
+  | o  5 F BOOK-F
+  | |
+  | o  4 E BOOK-E
+  | |
+  | o  3 D BOOK-D
+  | |
+  | o  2 C BOOK-C
+  | |
+  o |  1 B
+  |/
+  o  0 A
+  
+With --keep, bookmark should not move
+
+  $ hg rebase -r 3+4 -d E --keep
+  rebasing 3:e7b3f00ed42e "D" (BOOK-D)
+  note: rebase of 3:e7b3f00ed42e created no changes to commit
+  rebasing 4:69a34c08022a "E" (BOOK-E)
+  note: rebase of 4:69a34c08022a created no changes to commit
+  $ hg log -G -T '{rev} {desc} {bookmarks}'
+  o  7 E
+  |
+  o  6 D
+  |
+  | o  5 F BOOK-F
+  | |
+  | o  4 E BOOK-E
+  | |
+  | o  3 D BOOK-D
+  | |
+  | o  2 C BOOK-C
+  | |
+  o |  1 B
+  |/
+  o  0 A
+  
+Bookmark is usually an indication of a head. For changes that are introduced by
+an ancestor of bookmark B, after moving B to B-NEW, the changes are ideally
+still introduced by an ancestor of changeset on B-NEW. In the below case,
+"BOOK-D", and "BOOK-E" include changes introduced by "C".
+
+  $ hg rebase -s 2 -d E
+  rebasing 2:dc0947a82db8 "C" (C BOOK-C)
+  rebasing 3:e7b3f00ed42e "D" (BOOK-D)
+  note: rebase of 3:e7b3f00ed42e created no changes to commit
+  rebasing 4:69a34c08022a "E" (BOOK-E)
+  note: rebase of 4:69a34c08022a created no changes to commit
+  rebasing 5:6b2aeab91270 "F" (F BOOK-F)
+  saved backup bundle to $TESTTMP/non-merge/.hg/strip-backup/dc0947a82db8-52bb4973-rebase.hg (glob)
+  $ hg log -G -T '{rev} {desc} {bookmarks}'
+  o  5 F BOOK-F
+  |
+  o  4 C BOOK-C BOOK-D BOOK-E
+  |
+  o  3 E
+  |
+  o  2 D
+  |
+  o  1 B
+  |
+  o  0 A
+  
+Merge and its ancestors all become empty
+
+  $ hg init $TESTTMP/merge1
+  $ cd $TESTTMP/merge1
+
+  $ hg debugdrawdag<<'EOS'
+  >     E
+  >    /|
+  > B C D
+  >  \|/
+  >   A
+  > EOS
+
+  $ for i in C D E; do
+  >   hg bookmark -r $i -i BOOK-$i
+  > done
+
+  $ hg debugdrawdag<<'EOS'
+  > H
+  > |
+  > D
+  > |
+  > C
+  > |
+  > B
+  > EOS
+
+  $ hg rebase -r '(A::)-(B::)-A' -d H
+  rebasing 2:dc0947a82db8 "C" (BOOK-C)
+  note: rebase of 2:dc0947a82db8 created no changes to commit
+  rebasing 3:b18e25de2cf5 "D" (BOOK-D)
+  note: rebase of 3:b18e25de2cf5 created no changes to commit
+  rebasing 4:86a1f6686812 "E" (E BOOK-E)
+  note: rebase of 4:86a1f6686812 created no changes to commit
+  saved backup bundle to $TESTTMP/merge1/.hg/strip-backup/b18e25de2cf5-1fd0a4ba-rebase.hg (glob)
+
+  $ hg log -G -T '{rev} {desc} {bookmarks}'
+  o  4 H BOOK-C BOOK-D BOOK-E
+  |
+  o  3 D
+  |
+  o  2 C
+  |
+  o  1 B
+  |
+  o  0 A
+  
+Part of ancestors of a merge become empty
+
+  $ hg init $TESTTMP/merge2
+  $ cd $TESTTMP/merge2
+
+  $ hg debugdrawdag<<'EOS'
+  >     G
+  >    /|
+  >   E F
+  >   | |
+  > B C D
+  >  \|/
+  >   A
+  > EOS
+
+  $ for i in C D E F G; do
+  >   hg bookmark -r $i -i BOOK-$i
+  > done
+
+  $ hg debugdrawdag<<'EOS'
+  > H
+  > |
+  > F
+  > |
+  > C
+  > |
+  > B
+  > EOS
+
+  $ hg rebase -r '(A::)-(B::)-A' -d H
+  rebasing 2:dc0947a82db8 "C" (BOOK-C)
+  note: rebase of 2:dc0947a82db8 created no changes to commit
+  rebasing 3:b18e25de2cf5 "D" (D BOOK-D)
+  rebasing 4:03ca77807e91 "E" (E BOOK-E)
+  rebasing 5:ad6717a6a58e "F" (BOOK-F)
+  note: rebase of 5:ad6717a6a58e created no changes to commit
+  rebasing 6:c58e8bdac1f4 "G" (G BOOK-G)
+  saved backup bundle to $TESTTMP/merge2/.hg/strip-backup/b18e25de2cf5-2d487005-rebase.hg (glob)
+
+  $ hg log -G -T '{rev} {desc} {bookmarks}'
+  o    7 G BOOK-G
+  |\
+  | o  6 E BOOK-E
+  | |
+  o |  5 D BOOK-D BOOK-F
+  |/
+  o  4 H BOOK-C
+  |
+  o  3 F
+  |
+  o  2 C
+  |
+  o  1 B
+  |
+  o  0 A
+