Skip to content
Snippets Groups Projects
Commit 32fba6fe authored by Martin von Zweigbergk's avatar Martin von Zweigbergk
Browse files

scmutil: make cleanupnodes optionally also fix the phase

We have had multiple bugs where the phase wasn't correctly carried
forward to a rewritten changeset (for example: phabricator, split,
evolve, fix). Handling the phase update in cleanupnodes() makes it
less likely to happen again, especially once we have made it fix the
phase by default (perhaps in the next release cycle).

This patch also updates all applicable callers so we get some testing
of it.

Note that rebase and histedit can't be fixed yet because they call
cleanupnodes() only at the end and the phase may have been changed by
the user when the rebase/histedit was interrupted (due to merge
conflicts). I think we should make them write one commit at a time (as
it already does), along with associated obsmarkers, bookmark moves,
etc. When that's done, we can switch them over to cleanupnodes().

Differential Revision: https://phab.mercurial-scm.org/D3818
parent a6addfd6
No related branches found
No related tags found
No related merge requests found
......@@ -580,9 +580,7 @@
repo, old, parents=parents, text=newdesc,
user=old.user(), date=old.date(), extra=old.extra())
overrides = {(b'phases', b'new-commit'): old.phase()}
with ui.configoverride(overrides, b'phabsend'):
newnode = new.commit()
newnode = new.commit()
mapping[old.node()] = [newnode]
# Update diff property
......@@ -592,7 +590,7 @@
if tagname in repo.tags():
tags.tag(repo, tagname, nullid, message=None, user=None,
date=None, local=True)
scmutil.cleanupnodes(repo, mapping, b'phabsend')
scmutil.cleanupnodes(repo, mapping, b'phabsend', fixphase=True)
if wnode in mapping:
unfi.setparents(mapping[wnode][0])
......
......@@ -158,7 +158,7 @@
del filedata[rev]
replacements = {prec: [succ] for prec, succ in replacements.iteritems()}
scmutil.cleanupnodes(repo, replacements, 'fix')
scmutil.cleanupnodes(repo, replacements, 'fix', fixphase=True)
def getworkqueue(ui, repo, pats, opts, revstofix, basectxs):
""""Constructs the list of files to be fixed at specific revisions
......@@ -484,25 +484,23 @@
isexec=fctx.isexec(),
copied=copied)
overrides = {('phases', 'new-commit'): ctx.phase()}
with ui.configoverride(overrides, source='fix'):
memctx = context.memctx(
repo,
parents=(newp1node, newp2node),
text=ctx.description(),
files=set(ctx.files()) | set(filedata.keys()),
filectxfn=filectxfn,
user=ctx.user(),
date=ctx.date(),
extra=ctx.extra(),
branch=ctx.branch(),
editor=None)
sucnode = memctx.commit()
prenode = ctx.node()
if prenode == sucnode:
ui.debug('node %s already existed\n' % (ctx.hex()))
else:
replacements[ctx.node()] = sucnode
memctx = context.memctx(
repo,
parents=(newp1node, newp2node),
text=ctx.description(),
files=set(ctx.files()) | set(filedata.keys()),
filectxfn=filectxfn,
user=ctx.user(),
date=ctx.date(),
extra=ctx.extra(),
branch=ctx.branch(),
editor=None)
sucnode = memctx.commit()
prenode = ctx.node()
if prenode == sucnode:
ui.debug('node %s already existed\n' % (ctx.hex()))
else:
replacements[ctx.node()] = sucnode
def getfixers(ui):
"""Returns a map of configured fixer tools indexed by their names
......
......@@ -91,12 +91,7 @@
user=ctx.user(),
date=ctx.date(),
extra=ctx.extra())
# phase handling
commitphase = ctx.phase()
overrides = {('phases', 'new-commit'): commitphase}
with repo.ui.configoverride(overrides, 'uncommit'):
newid = repo.commitctx(new)
return newid
return repo.commitctx(new)
def _fixdirstate(repo, oldctx, newctx, status):
""" fix the dirstate after switching the working directory from oldctx to
......@@ -183,7 +178,7 @@
# Fully removed the old commit
mapping[old.node()] = ()
scmutil.cleanupnodes(repo, mapping, 'uncommit')
scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True)
with repo.dirstate.parentchange():
repo.dirstate.setparents(newid, node.nullid)
......@@ -242,12 +237,7 @@
user=predctx.user(),
date=predctx.date(),
extra=extras)
# phase handling
commitphase = curctx.phase()
overrides = {('phases', 'new-commit'): commitphase}
with repo.ui.configoverride(overrides, 'uncommit'):
newprednode = repo.commitctx(newctx)
newprednode = repo.commitctx(newctx)
newpredctx = repo[newprednode]
dirstate = repo.dirstate
......@@ -257,4 +247,4 @@
_fixdirstate(repo, curctx, newpredctx, s)
mapping = {curctx.node(): (newprednode,)}
scmutil.cleanupnodes(repo, mapping, 'unamend')
scmutil.cleanupnodes(repo, mapping, 'unamend', fixphase=True)
......@@ -35,6 +35,7 @@
obsolete,
patch,
pathutil,
phases,
pycompat,
revlog,
rewriteutil,
......@@ -784,12 +785,8 @@
extra=extra,
branch=label)
commitphase = ctx.phase()
overrides = {('phases', 'new-commit'): commitphase}
with repo.ui.configoverride(overrides, 'branch-change'):
newnode = repo.commitctx(mc)
newnode = repo.commitctx(mc)
replacements[ctx.node()] = (newnode,)
ui.debug('new node id is %s\n' % hex(newnode))
# create obsmarkers and move bookmarks
......@@ -792,8 +789,8 @@
replacements[ctx.node()] = (newnode,)
ui.debug('new node id is %s\n' % hex(newnode))
# create obsmarkers and move bookmarks
scmutil.cleanupnodes(repo, replacements, 'branch-change')
scmutil.cleanupnodes(repo, replacements, 'branch-change', fixphase=True)
# move the working copy too
wctx = repo[None]
......@@ -2536,4 +2533,5 @@
# This not what we expect from amend.
return old.node()
commitphase = None
if opts.get('secret'):
......@@ -2539,10 +2537,6 @@
if opts.get('secret'):
commitphase = 'secret'
else:
commitphase = old.phase()
overrides = {('phases', 'new-commit'): commitphase}
with ui.configoverride(overrides, 'amend'):
newid = repo.commitctx(new)
commitphase = phases.secret
newid = repo.commitctx(new)
# Reroute the working copy parent to the new changeset
repo.setparents(newid, nullid)
......@@ -2550,7 +2544,8 @@
obsmetadata = None
if opts.get('note'):
obsmetadata = {'note': opts['note']}
scmutil.cleanupnodes(repo, mapping, 'amend', metadata=obsmetadata)
scmutil.cleanupnodes(repo, mapping, 'amend', metadata=obsmetadata,
fixphase=True, targetphase=commitphase)
# Fixing the dirstate because localrepo.commitctx does not update
# it. This is rather convenient because we did not need to update
......
......@@ -779,7 +779,8 @@
def __contains__(self, node):
return self._revcontains(self._torev(node))
def cleanupnodes(repo, replacements, operation, moves=None, metadata=None):
def cleanupnodes(repo, replacements, operation, moves=None, metadata=None,
fixphase=False, targetphase=None):
"""do common cleanups when old nodes are replaced by new nodes
That includes writing obsmarkers or stripping nodes, and moving bookmarks.
......@@ -795,6 +796,7 @@
metadata is dictionary containing metadata to be stored in obsmarker if
obsolescence is enabled.
"""
assert fixphase or targetphase is None
if not replacements and not moves:
return
......@@ -825,7 +827,35 @@
newnode = newnodes[0]
moves[oldnode] = newnode
allnewnodes = [n for ns in replacements.values() for n in ns]
toretract = {}
toadvance = {}
if fixphase:
precursors = {}
for oldnode, newnodes in replacements.items():
for newnode in newnodes:
precursors.setdefault(newnode, []).append(oldnode)
allnewnodes.sort(key=lambda n: unfi[n].rev())
newphases = {}
def phase(ctx):
return newphases.get(ctx.node(), ctx.phase())
for newnode in allnewnodes:
ctx = unfi[newnode]
if targetphase is None:
oldphase = max(unfi[oldnode].phase()
for oldnode in precursors[newnode])
parentphase = max(phase(p) for p in ctx.parents())
newphase = max(oldphase, parentphase)
else:
newphase = targetphase
newphases[newnode] = newphase
if newphase > ctx.phase():
toretract.setdefault(newphase, []).append(newnode)
elif newphase < ctx.phase():
toadvance.setdefault(newphase, []).append(newnode)
with repo.transaction('cleanup') as tr:
# Move bookmarks
bmarks = repo._bookmarks
bmarkchanges = []
......@@ -828,8 +858,7 @@
with repo.transaction('cleanup') as tr:
# Move bookmarks
bmarks = repo._bookmarks
bmarkchanges = []
allnewnodes = [n for ns in replacements.values() for n in ns]
for oldnode, newnode in moves.items():
oldbmarks = repo.nodebookmarks(oldnode)
if not oldbmarks:
......@@ -850,6 +879,11 @@
if bmarkchanges:
bmarks.applychanges(repo, tr, bmarkchanges)
for phase, nodes in toretract.items():
phases.retractboundary(repo, tr, phase, nodes)
for phase, nodes in toadvance.items():
phases.advanceboundary(repo, tr, phase, nodes)
# Obsolete or strip nodes
if obsolete.isenabled(repo, obsolete.createmarkersopt):
# If a node is already obsoleted, and we want to obsolete it
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment