This is an archive of the discontinued Mercurial Phabricator instance.

Differential D750

rebase: move bookmarks with --keep (issue5682)
AbandonedPublic

Authored by martinvonz on Sep 20 2017, 12:55 PM.

Details

Reviewers
quark
Group Reviewers
hg-reviewers
Summary

This is my version of Jun's D727. Not meant to be accepted, but
instead possibly incorporated into Jun's patch.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Sep 20 2017, 12:55 PM
martinvonz updated this revision to Diff 1940.Sep 20 2017, 1:04 PM
martinvonz updated this revision to Diff 1987.Sep 21 2017, 6:14 PM
quark accepted this revision.Sep 21 2017, 7:01 PM
quark added a subscriber: quark.
quark added inline comments.
hgext/rebase.py
1539

In the future we might want to get the 'rebase' name from current transaction (and enforce an active transaction in cleanupnodes). In that case the name parameter becomes less necessary and moves could be the 3rd argument.

I'll update the code if I'm expected to do that.

martinvonz added inline comments.Sep 21 2017, 7:07 PM
hgext/rebase.py
1539

I'll update the code if I'm expected to do that.

Yes, I was planning to let you take the contents of this patch and put it in yours.

mercurial/scmutil.py
588–589

As I said on IRC, please also update this to be more accurate, because it 'moves' it not only about overrides, it can also have items that are not in 'replacements'

594–595

I'd just drop this actually. Nothing expensive happens if they're both empty anyway

martinvonz updated this revision to Diff 2198.Sep 30 2017, 10:16 AM
martinvonz abandoned this revision.Sep 30 2017, 10:49 AM

Dropping this as planned. Contents of it went in as D727 (because it was Jun who wrote it).

Revision Contents
Changeset List

Diff 1940

hgext/rebase.py

newwd = self.state.get(self.originalwd, self.originalwd) newwd = self.state.get(self.originalwd, self.originalwd)
if newwd < 0: if newwd < 0:
# original directory is a parent of rebase set root or ignored # original directory is a parent of rebase set root or ignored
newwd = self.originalwd newwd = self.originalwd
if newwd not in [c.rev() for c in repo[None].parents()]: if newwd not in [c.rev() for c in repo[None].parents()]:
ui.note(_("update back to initial working directory parent\n")) ui.note(_("update back to initial working directory parent\n"))
hg.updaterepo(repo, newwd, False) hg.updaterepo(repo, newwd, False)
if not self.keepf:
collapsedas = None collapsedas = None
if not self.keepf:
if self.collapsef: if self.collapsef:
collapsedas = newnode collapsedas = newnode
clearrebased(ui, repo, self.destmap, self.state, self.skipped, clearrebased(ui, repo, self.destmap, self.state, self.skipped,
collapsedas) collapsedas, self.keepf)
clearstatus(repo) clearstatus(repo)
clearcollapsemsg(repo) clearcollapsemsg(repo)
ui.note(_("rebase completed\n")) ui.note(_("rebase completed\n"))
util.unlinkpath(repo.sjoin('undo'), ignoremissing=True) util.unlinkpath(repo.sjoin('undo'), ignoremissing=True)
if self.skipped: if self.skipped:
skippedlen = len(self.skipped) skippedlen = len(self.skipped)
return None return None
for rev in sorted(state): for rev in sorted(state):
parents = [p for p in repo.changelog.parentrevs(rev) if p != nullrev] parents = [p for p in repo.changelog.parentrevs(rev) if p != nullrev]
# if all parents of this revision are done, then so is this revision # 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)): if parents and all((state.get(p) == p for p in parents)):
state[rev] = rev state[rev] = rev
return originalwd, destmap, state 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 """dispose of rebased revision at the end of the rebase
If `collapsedas` is not None, the rebase was a collapse whose result if the 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 tonode = repo.changelog.node
# Move bookmark of skipped nodes to destination. This cannot be handled replacements = {}
# by scmutil.cleanupnodes since it will treat rev as removed (no successor) moves = {}
# 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()): for rev, newrev in sorted(state.items()):
if newrev >= 0 and newrev != rev: if newrev >= 0 and newrev != rev:
oldnode = tonode(rev)
newnode = collapsedas or tonode(newrev)
moves[oldnode] = newnode
if not keepf:
if rev in skipped: if rev in skipped:
succs = () succs = ()
elif collapsedas is not None:
succs = (collapsedas,)
else: else:
succs = (tonode(newrev),) succs = (newnode,)
mapping[tonode(rev)] = succs replacements[oldnode] = succs
scmutil.cleanupnodes(repo, mapping, 'rebase') scmutil.cleanupnodes(repo, replacements, 'rebase', moves)
quarkUnsubmitted
Not Done

In the future we might want to get the 'rebase' name from current transaction (and enforce an active transaction in cleanupnodes). In that case the name parameter becomes less necessary and moves could be the 3rd argument.

I'll update the code if I'm expected to do that.

quark: In the future we might want to get the `'rebase'` name from current transaction (and enforce an…
martinvonzAuthorUnsubmitted
Not Done

I'll update the code if I'm expected to do that.

Yes, I was planning to let you take the contents of this patch and put it in yours.

martinvonz: > I'll update the code if I'm expected to do that. Yes, I was planning to let you take the…
def pullrebase(orig, ui, repo, *args, **opts): def pullrebase(orig, ui, repo, *args, **opts):
'Call rebase after pull if the latter has been invoked with --rebase' 'Call rebase after pull if the latter has been invoked with --rebase'
ret = None ret = None
if opts.get('rebase'): if opts.get('rebase'):
if ui.configbool('commands', 'rebase.requiredest'): if ui.configbool('commands', 'rebase.requiredest'):
msg = _('rebase destination required by configuration') msg = _('rebase destination required by configuration')
hint = _('use hg pull followed by hg rebase -d DEST') hint = _('use hg pull followed by hg rebase -d DEST')

mercurial/scmutil.py

def __init__(self, repo, revcontainer): def __init__(self, repo, revcontainer):
self._torev = repo.changelog.rev self._torev = repo.changelog.rev
self._revcontains = revcontainer.__contains__ self._revcontains = revcontainer.__contains__
def __contains__(self, node): def __contains__(self, node):
return self._revcontains(self._torev(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 """do common cleanups when old nodes are replaced by new nodes
That includes writing obsmarkers or stripping nodes, and moving bookmarks. That includes writing obsmarkers or stripping nodes, and moving bookmarks.
(we might also want to move working directory parent in the future) (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.
martinvonzAuthorUnsubmitted
Not Done

As I said on IRC, please also update this to be more accurate, because it 'moves' it not only about overrides, it can also have items that are not in 'replacements'

martinvonz: As I said on IRC, please also update this to be more accurate, because it 'moves' it not only…
replacements is {oldnode: [newnode]} or a iterable of nodes if they do not have replacements is {oldnode: [newnode]} or a iterable of nodes if they do not have
replacements. operation is a string, like "rebase". replacements. operation is a string, like "rebase".
""" """
if not replacements and not moves:
return
martinvonzAuthorUnsubmitted
Not Done

I'd just drop this actually. Nothing expensive happens if they're both empty anyway

martinvonz: I'd just drop this actually. Nothing expensive happens if they're both empty anyway
# translate mapping's other forms
if not util.safehasattr(replacements, 'items'): if not util.safehasattr(replacements, 'items'):
replacements = {n: () for n in replacements} replacements = {n: () for n in replacements}
# Calculate bookmark movements # Calculate bookmark movements
if moves is None:
moves = {} moves = {}
unfi = repo.unfiltered() unfi = repo.unfiltered()
for oldnode, newnodes in replacements.items(): for oldnode, newnodes in replacements.items():
if oldnode in moves:
continue
if len(newnodes) > 1: if len(newnodes) > 1:
# usually a split, take the one with biggest rev number # usually a split, take the one with biggest rev number
newnode = next(unfi.set('max(%ln)', newnodes)).node() newnode = next(unfi.set('max(%ln)', newnodes)).node()
elif len(newnodes) == 0: elif len(newnodes) == 0:
# move bookmark backwards # move bookmark backwards
roots = list(unfi.set('max((::%n) - %ln)', oldnode, roots = list(unfi.set('max((::%n) - %ln)', oldnode,
list(replacements))) list(replacements)))
if roots: if roots:
# hidden. # hidden.
unfi = repo.unfiltered() unfi = repo.unfiltered()
isobs = unfi.obsstore.successors.__contains__ isobs = unfi.obsstore.successors.__contains__
torev = unfi.changelog.rev torev = unfi.changelog.rev
sortfunc = lambda ns: torev(ns[0]) sortfunc = lambda ns: torev(ns[0])
rels = [(unfi[n], tuple(unfi[m] for m in s)) rels = [(unfi[n], tuple(unfi[m] for m in s))
for n, s in sorted(replacements.items(), key=sortfunc) for n, s in sorted(replacements.items(), key=sortfunc)
if s or not isobs(n)] if s or not isobs(n)]
if rels:
obsolete.createmarkers(repo, rels, operation=operation) obsolete.createmarkers(repo, rels, operation=operation)
else: else:
from . import repair # avoid import cycle 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): def addremove(repo, matcher, prefix, opts=None, dry_run=None, similarity=None):
if opts is None: if opts is None:
opts = {} opts = {}
m = matcher m = matcher
if dry_run is None: if dry_run is None:
dry_run = opts.get('dry_run') dry_run = opts.get('dry_run')
if similarity is None: if similarity is None:

tests/test-rebase-bookmarks.t

$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [extensions] > [extensions]
> rebase= > rebase=
> drawdag=$TESTDIR/drawdag.py
> >
> [phases] > [phases]
> publish=False > publish=False
> >
> [alias] > [alias]
> tglog = log -G --template "{rev}: '{desc}' bookmarks: {bookmarks}\n" > tglog = log -G --template "{rev}: '{desc}' bookmarks: {bookmarks}\n"
> EOF > EOF
$ hg ci -m bisect3 $ hg ci -m bisect3
$ hg book bisect $ hg book bisect
$ hg update -q Y $ hg update -q Y
$ hg rebase -r '"bisect"^^::"bisect"^' -r bisect -d Z $ hg rebase -r '"bisect"^^::"bisect"^' -r bisect -d Z
rebasing 5:345c90f326a4 "bisect" rebasing 5:345c90f326a4 "bisect"
rebasing 6:f677a2907404 "bisect2" rebasing 6:f677a2907404 "bisect2"
rebasing 7:325c16001345 "bisect3" (tip bisect) rebasing 7:325c16001345 "bisect3" (tip bisect)
saved backup bundle to $TESTTMP/a3/.hg/strip-backup/345c90f326a4-b4840586-rebase.hg (glob) 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

tests/test-rebase-emptycommit.t

| o 3 D BOOK-D | o 3 D BOOK-D
| | | |
| o 2 C BOOK-C | o 2 C BOOK-C
| | | |
o | 1 B o | 1 B
|/ |/
o 0 A o 0 A
With --keep, bookmark should not move With --keep, bookmark should move
$ hg rebase -r 3+4 -d E --keep $ hg rebase -r 3+4 -d E --keep
rebasing 3:e7b3f00ed42e "D" (BOOK-D) rebasing 3:e7b3f00ed42e "D" (BOOK-D)
note: rebase of 3:e7b3f00ed42e created no changes to commit note: rebase of 3:e7b3f00ed42e created no changes to commit
rebasing 4:69a34c08022a "E" (BOOK-E) rebasing 4:69a34c08022a "E" (BOOK-E)
note: rebase of 4:69a34c08022a created no changes to commit note: rebase of 4:69a34c08022a created no changes to commit
$ hg log -G -T '{rev} {desc} {bookmarks}' $ hg log -G -T '{rev} {desc} {bookmarks}'
o 7 E o 7 E BOOK-D BOOK-E
| |
o 6 D o 6 D
| |
| o 5 F BOOK-F | 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 2 C BOOK-C
| | | |
o | 1 B o | 1 B
|/ |/
o 0 A 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 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 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, still introduced by an ancestor of changeset on B-NEW. In the below case,
"BOOK-D", and "BOOK-E" include changes introduced by "C". "BOOK-D", and "BOOK-E" include changes introduced by "C".
$ hg rebase -s 2 -d E $ hg rebase -s 2 -d E
rebasing 2:dc0947a82db8 "C" (C BOOK-C) rebasing 2:dc0947a82db8 "C" (C BOOK-C)
rebasing 3:e7b3f00ed42e "D" (BOOK-D) rebasing 3:e7b3f00ed42e "D" (BOOK-D)