# HG changeset patch # User Jun Wu # Date 1505757240 25200 # Node ID 2f427b57bf9019c6dc3750baa539dc22c1be50f6 # Parent 2dbd6d259cd2f873b34cf88df4f1ec408a0343c9 rebase: move bookmarks with --keep (issue5682) This is a regression caused by 3b7cb3d17137. We have documented the behavior in rebase help: Rebase will destroy original commits unless you use "--keep". It will also move your bookmarks (even if you do). So let's restore the old behavior. It is done by changing `scmutil.cleanupnodes` to accept more information so a node could have different "movement destination" from "successors". It also helps simplifying the callsite as a side effect - the special bookmark movement logic in rebase is removed. Differential Revision: https://phab.mercurial-scm.org/D727 diff -r 2dbd6d259cd2 -r 2f427b57bf90 hgext/rebase.py --- a/hgext/rebase.py Wed Sep 20 09:32:26 2017 -0700 +++ b/hgext/rebase.py Mon Sep 18 10:54:00 2017 -0700 @@ -508,12 +508,12 @@ ui.note(_("update back to initial working directory parent\n")) hg.updaterepo(repo, newwd, False) + collapsedas = None if not self.keepf: - collapsedas = None if self.collapsef: collapsedas = newnode - clearrebased(ui, repo, self.dest, self.state, self.skipped, - collapsedas) + clearrebased(ui, repo, self.dest, self.state, self.skipped, + collapsedas, self.keepf) clearstatus(repo) clearcollapsemsg(repo) @@ -1354,32 +1354,30 @@ state[r] = revprecursor return originalwd, dest.rev(), state -def clearrebased(ui, repo, dest, state, skipped, collapsedas=None): +def clearrebased(ui, repo, dest, state, skipped, collapsedas=None, keepf=False): """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.""" + `collapsedas` node. + + If `keepf` is not True, the rebase has --keep set and no nodes should be + removed (but bookmarks still need to be moved). + """ 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 = {} + replacements = {} + moves = {} for rev, newrev in sorted(state.items()): if newrev >= 0 and newrev != rev: - if rev in skipped: - succs = () - elif collapsedas is not None: - succs = (collapsedas,) - else: - succs = (tonode(newrev),) - mapping[tonode(rev)] = succs - scmutil.cleanupnodes(repo, mapping, 'rebase') + oldnode = tonode(rev) + newnode = collapsedas or tonode(newrev) + moves[oldnode] = newnode + if not keepf: + if rev in skipped: + succs = () + else: + succs = (newnode,) + replacements[oldnode] = succs + scmutil.cleanupnodes(repo, replacements, 'rebase', moves) def pullrebase(orig, ui, repo, *args, **opts): 'Call rebase after pull if the latter has been invoked with --rebase' diff -r 2dbd6d259cd2 -r 2f427b57bf90 mercurial/scmutil.py --- a/mercurial/scmutil.py Wed Sep 20 09:32:26 2017 -0700 +++ b/mercurial/scmutil.py Mon Sep 18 10:54:00 2017 -0700 @@ -576,23 +576,34 @@ def __contains__(self, node): return self._revcontains(self._torev(node)) -def cleanupnodes(repo, replacements, operation): +def cleanupnodes(repo, replacements, operation, moves=None): """do common cleanups when old nodes are replaced by new nodes That includes writing obsmarkers or stripping nodes, and moving bookmarks. (we might also want to move working directory parent in the future) + By default, bookmark moves are calculated automatically from 'replacements', + but 'moves' can be used to override that. Also, 'moves' may include + additional bookmark moves that should not have associated obsmarkers. + replacements is {oldnode: [newnode]} or a iterable of nodes if they do not have replacements. operation is a string, like "rebase". """ + if not replacements and not moves: + return + + # translate mapping's other forms if not util.safehasattr(replacements, 'items'): replacements = {n: () for n in replacements} # Calculate bookmark movements - moves = {} + if moves is None: + moves = {} # Unfiltered repo is needed since nodes in replacements might be hidden. unfi = repo.unfiltered() for oldnode, newnodes in replacements.items(): + if oldnode in moves: + continue if len(newnodes) > 1: # usually a split, take the one with biggest rev number newnode = next(unfi.set('max(%ln)', newnodes)).node() @@ -646,10 +657,13 @@ rels = [(unfi[n], tuple(unfi[m] for m in s)) for n, s in sorted(replacements.items(), key=sortfunc) if s or not isobs(n)] - obsolete.createmarkers(repo, rels, operation=operation) + if rels: + obsolete.createmarkers(repo, rels, operation=operation) else: from . import repair # avoid import cycle - repair.delayedstrip(repo.ui, repo, list(replacements), operation) + tostrip = list(replacements) + if tostrip: + repair.delayedstrip(repo.ui, repo, tostrip, operation) def addremove(repo, matcher, prefix, opts=None, dry_run=None, similarity=None): if opts is None: diff -r 2dbd6d259cd2 -r 2f427b57bf90 tests/test-rebase-bookmarks.t --- a/tests/test-rebase-bookmarks.t Wed Sep 20 09:32:26 2017 -0700 +++ b/tests/test-rebase-bookmarks.t Mon Sep 18 10:54:00 2017 -0700 @@ -1,6 +1,7 @@ $ cat >> $HGRCPATH < [extensions] > rebase= + > drawdag=$TESTDIR/drawdag.py > > [phases] > publish=False @@ -210,3 +211,35 @@ rebasing 6:f677a2907404 "bisect2" rebasing 7:325c16001345 "bisect3" (tip bisect) saved backup bundle to $TESTTMP/a3/.hg/strip-backup/345c90f326a4-b4840586-rebase.hg (glob) + +Bookmark and working parent get moved even if --keep is set (issue5682) + + $ hg init $TESTTMP/book-keep + $ cd $TESTTMP/book-keep + $ hg debugdrawdag <<'EOS' + > B C + > |/ + > A + > EOS + $ eval `hg tags -T 'hg bookmark -ir {node} {tag};\n' | grep -v tip` + $ rm .hg/localtags + $ hg up -q B + $ hg tglog + o 2: 'C' bookmarks: C + | + | @ 1: 'B' bookmarks: B + |/ + o 0: 'A' bookmarks: A + + $ hg rebase -r B -d C --keep + rebasing 1:112478962961 "B" (B) + $ hg tglog + @ 3: 'B' bookmarks: B + | + o 2: 'C' bookmarks: C + | + | o 1: 'B' bookmarks: + |/ + o 0: 'A' bookmarks: A + + diff -r 2dbd6d259cd2 -r 2f427b57bf90 tests/test-rebase-emptycommit.t --- a/tests/test-rebase-emptycommit.t Wed Sep 20 09:32:26 2017 -0700 +++ b/tests/test-rebase-emptycommit.t Mon Sep 18 10:54:00 2017 -0700 @@ -47,7 +47,7 @@ |/ o 0 A -With --keep, bookmark should not move +With --keep, bookmark should move $ hg rebase -r 3+4 -d E --keep rebasing 3:e7b3f00ed42e "D" (BOOK-D) @@ -55,15 +55,15 @@ 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 7 E BOOK-D BOOK-E | o 6 D | | o 5 F BOOK-F | | - | o 4 E BOOK-E + | o 4 E | | - | o 3 D BOOK-D + | o 3 D | | | o 2 C BOOK-C | | @@ -71,6 +71,11 @@ |/ o 0 A +Move D and E back for the next test + + $ hg bookmark BOOK-D -fqir 3 + $ hg bookmark BOOK-E -fqir 4 + 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,