# HG changeset patch # User Martin von Zweigbergk # Date 1529431660 25200 # Node ID 32fba6fe893d2303e107b534e9c121207e64577f # Parent a6addfd64507d67c5cf675ad11ead52de600a8c8 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 diff -r a6addfd64507 -r 32fba6fe893d contrib/phabricator.py --- a/contrib/phabricator.py Tue Jun 19 11:07:23 2018 -0700 +++ b/contrib/phabricator.py Tue Jun 19 11:07:40 2018 -0700 @@ -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]) diff -r a6addfd64507 -r 32fba6fe893d hgext/fix.py --- a/hgext/fix.py Tue Jun 19 11:07:23 2018 -0700 +++ b/hgext/fix.py Tue Jun 19 11:07:40 2018 -0700 @@ -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 diff -r a6addfd64507 -r 32fba6fe893d hgext/uncommit.py --- a/hgext/uncommit.py Tue Jun 19 11:07:23 2018 -0700 +++ b/hgext/uncommit.py Tue Jun 19 11:07:40 2018 -0700 @@ -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) diff -r a6addfd64507 -r 32fba6fe893d mercurial/cmdutil.py --- a/mercurial/cmdutil.py Tue Jun 19 11:07:23 2018 -0700 +++ b/mercurial/cmdutil.py Tue Jun 19 11:07:40 2018 -0700 @@ -35,6 +35,7 @@ obsolete, patch, pathutil, + phases, pycompat, revlog, rewriteutil, @@ -784,16 +785,12 @@ 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 - scmutil.cleanupnodes(repo, replacements, 'branch-change') + scmutil.cleanupnodes(repo, replacements, 'branch-change', fixphase=True) # move the working copy too wctx = repo[None] @@ -2536,13 +2533,10 @@ # This not what we expect from amend. return old.node() + commitphase = None 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 diff -r a6addfd64507 -r 32fba6fe893d mercurial/scmutil.py --- a/mercurial/scmutil.py Tue Jun 19 11:07:23 2018 -0700 +++ b/mercurial/scmutil.py Tue Jun 19 11:07:40 2018 -0700 @@ -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,11 +827,38 @@ 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 = [] - 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