D727: rebase: move bookmarks with --keep (issue5682)
quark (Jun Wu)
phabricator at mercurial-scm.org
Mon Sep 18 18:58:36 UTC 2017
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
This is a regression caused by https://phab.mercurial-scm.org/rHG3b7cb3d171375f7b1a84fd8997814f14a0f29da5. 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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D727
AFFECTED FILES
hgext/rebase.py
mercurial/scmutil.py
tests/test-rebase-bookmarks.t
tests/test-rebase-emptycommit.t
CHANGE DETAILS
diff --git a/tests/test-rebase-emptycommit.t b/tests/test-rebase-emptycommit.t
--- a/tests/test-rebase-emptycommit.t
+++ b/tests/test-rebase-emptycommit.t
@@ -47,30 +47,35 @@
|/
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)
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 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
| |
o | 1 B
|/
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,
diff --git a/tests/test-rebase-bookmarks.t b/tests/test-rebase-bookmarks.t
--- a/tests/test-rebase-bookmarks.t
+++ b/tests/test-rebase-bookmarks.t
@@ -1,6 +1,7 @@
$ cat >> $HGRCPATH <<EOF
> [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 --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -585,35 +585,53 @@
That includes writing obsmarkers or stripping nodes, and moving bookmarks.
(we might also want to move working directory parent in the future)
- mapping is {oldnode: [newnode]} or a iterable of nodes if they do not have
- replacements. operation is a string, like "rebase".
+ mapping is {oldnode: {'new': [newnode], 'move': newnode}}, where 'new'
+ decide obsolete markers, and 'move' decide bookmark move, which could be
+ different from new. If 'move' is not provided, it will be calculated
+ automatically from 'new'. If 'new' is not set, the oldnode will not be
+ removed or obsoleted.
+
+ To make the API easier to use, mapping could also be in the following
+ forms:
+
+ - {oldnode: [newnode]}, equal to {oldnode: {'new': [newnode]}}
+ - {oldnode}, equal to {oldnode: []}
"""
+ if not mapping:
+ return
+
+ # translate {oldnode} form
if not util.safehasattr(mapping, 'items'):
- mapping = {n: () for n in mapping}
+ mapping = {n: {'new': ()} for n in mapping}
+ elif all('new' not in v and 'move' not in v for v in mapping.values()):
+ mapping = {n: {'new': newnodes} for n, newnodes in mapping.items()}
with repo.transaction('cleanup') as tr:
# Move bookmarks
bmarks = repo._bookmarks
bmarkchanges = []
- allnewnodes = [n for ns in mapping.values() for n in ns]
- for oldnode, newnodes in mapping.items():
+ allnewnodes = [n for v in mapping.values() for n in v.get('new', ())]
+ for oldnode, value in mapping.items():
oldbmarks = repo.nodebookmarks(oldnode)
if not oldbmarks:
continue
+ newnode = value.get('move')
from . import bookmarks # avoid import cycle
- if len(newnodes) > 1:
- # usually a split, take the one with biggest rev number
- newnode = next(repo.set('max(%ln)', newnodes)).node()
- elif len(newnodes) == 0:
- # move bookmark backwards
- roots = list(repo.set('max((::%n) - %ln)', oldnode,
- list(mapping)))
- if roots:
- newnode = roots[0].node()
+ if not newnode:
+ newnodes = value['new']
+ if len(newnodes) > 1:
+ # usually a split, take the one with biggest rev number
+ newnode = next(repo.set('max(%ln)', newnodes)).node()
+ elif len(newnodes) == 0:
+ # move bookmark backwards
+ roots = list(repo.set('max((::%n) - %ln)', oldnode,
+ list(mapping)))
+ if roots:
+ newnode = roots[0].node()
+ else:
+ newnode = nullid
else:
- newnode = nullid
- else:
- newnode = newnodes[0]
+ newnode = newnodes[0]
repo.ui.debug('moving bookmarks %r from %s to %s\n' %
(oldbmarks, hex(oldnode), hex(newnode)))
# Delete divergent bookmarks being parents of related newnodes
@@ -641,13 +659,16 @@
isobs = unfi.obsstore.successors.__contains__
torev = unfi.changelog.rev
sortfunc = lambda ns: torev(ns[0])
- rels = [(unfi[n], tuple(unfi[m] for m in s))
- for n, s in sorted(mapping.items(), key=sortfunc)
- if s or not isobs(n)]
- obsolete.createmarkers(repo, rels, operation=operation)
+ rels = [(unfi[n], tuple(unfi[m] for m in v['new']))
+ for n, v in sorted(mapping.items(), key=sortfunc)
+ if 'new' in v and (v['new'] or not isobs(n))]
+ if rels:
+ obsolete.createmarkers(repo, rels, operation=operation)
else:
from . import repair # avoid import cycle
- repair.delayedstrip(repo.ui, repo, list(mapping), operation)
+ tostrip = [n for n, v in mapping.items() if 'new' in v]
+ 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 --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -543,12 +543,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.destmap, self.state, self.skipped,
- collapsedas)
+ clearrebased(ui, repo, self.destmap, self.state, self.skipped,
+ collapsedas, self.keepf)
clearstatus(repo)
clearcollapsemsg(repo)
@@ -1512,31 +1512,29 @@
state[rev] = rev
return originalwd, destmap, state
-def clearrebased(ui, repo, destmap, state, skipped, collapsedas=None):
+def clearrebased(ui, repo, destmap, 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(state[rev]))
- 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:
- if rev in skipped:
- succs = ()
- elif collapsedas is not None:
- succs = (collapsedas,)
- else:
- succs = (tonode(newrev),)
- mapping[tonode(rev)] = succs
+ newnode = collapsedas or tonode(newrev)
+ value = {'move': newnode}
+ if not keepf:
+ if rev in skipped:
+ succs = ()
+ else:
+ succs = (newnode,)
+ value['new'] = succs
+ mapping[tonode(rev)] = value
scmutil.cleanupnodes(repo, mapping, 'rebase')
def pullrebase(orig, ui, repo, *args, **opts):
To: quark, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list