# HG changeset patch # User Martin von Zweigbergk # Date 1597121207 25200 # Node ID 8c466bcb087960e126ca699b59337e6cf5c30fe6 # Parent bd56597b225455b1d34501c043e36e62c1aa4748 revert: remove dangerous `parents` argument from `cmdutil.revert()` As we found out the hard way (thanks to spectral@ for figuring it out!), `cmdutil.revert()`'s `parents` argument must be `repo.dirstate.parents()` or things may go wrong. We had an extension that passed in the target commit as the first parent. The `hg split` command from the evolve extension seems to have made the same mistake, but I haven't looked carefully. The problem is that `cmdutil._performrevert()` calls `dirstate.normal()` on reverted files if the commit to revert to equals the first parent. So if you pass in `ctx=foo` and `parents=(foo.node(), nullid)`, then `dirstate.normal()` will be called for the revert files, even though they might not be clean in the working copy. There doesn't seem to be any reason, other than a tiny performance benefit, to passing the `parents` around instead of looking them up again in `cmdutil._performrevert()`, so that's what this patch does. Differential Revision: https://phab.mercurial-scm.org/D8925 diff -r bd56597b2254 -r 8c466bcb0879 hgext/histedit.py --- a/hgext/histedit.py Mon Aug 10 18:08:15 2020 -0700 +++ b/hgext/histedit.py Mon Aug 10 21:46:47 2020 -0700 @@ -635,12 +635,11 @@ def applychanges(ui, repo, ctx, opts): """Merge changeset from ctx (only) in the current working directory""" - wcpar = repo.dirstate.p1() - if ctx.p1().node() == wcpar: + if ctx.p1().node() == repo.dirstate.p1(): # edits are "in place" we do not need to make any merge, # just applies changes on parent for editing ui.pushbuffer() - cmdutil.revert(ui, repo, ctx, (wcpar, node.nullid), all=True) + cmdutil.revert(ui, repo, ctx, all=True) stats = mergemod.updateresult(0, 0, 0, 0) ui.popbuffer() else: diff -r bd56597b2254 -r 8c466bcb0879 hgext/largefiles/overrides.py --- a/hgext/largefiles/overrides.py Mon Aug 10 18:08:15 2020 -0700 +++ b/hgext/largefiles/overrides.py Mon Aug 10 21:46:47 2020 -0700 @@ -874,7 +874,7 @@ # the matcher to hit standins instead of largefiles. Based on the # resulting standins update the largefiles. @eh.wrapfunction(cmdutil, b'revert') -def overriderevert(orig, ui, repo, ctx, parents, *pats, **opts): +def overriderevert(orig, ui, repo, ctx, *pats, **opts): # Because we put the standins in a bad state (by updating them) # and then return them to a correct state we need to lock to # prevent others from changing them in their incorrect state. @@ -937,7 +937,7 @@ return m with extensions.wrappedfunction(scmutil, b'match', overridematch): - orig(ui, repo, ctx, parents, *pats, **opts) + orig(ui, repo, ctx, *pats, **opts) newstandins = lfutil.getstandinsstate(repo) filelist = lfutil.getlfilestoupdate(oldstandins, newstandins) diff -r bd56597b2254 -r 8c466bcb0879 hgext/mq.py --- a/hgext/mq.py Mon Aug 10 18:08:15 2020 -0700 +++ b/hgext/mq.py Mon Aug 10 21:46:47 2020 -0700 @@ -1717,11 +1717,7 @@ except: # re-raises self.ui.warn(_(b'cleaning up working directory...\n')) cmdutil.revert( - self.ui, - repo, - repo[b'.'], - repo.dirstate.parents(), - no_backup=True, + self.ui, repo, repo[b'.'], no_backup=True, ) # only remove unknown files that we know we touched or # created while patching diff -r bd56597b2254 -r 8c466bcb0879 mercurial/cmdutil.py --- a/mercurial/cmdutil.py Mon Aug 10 18:08:15 2020 -0700 +++ b/mercurial/cmdutil.py Mon Aug 10 21:46:47 2020 -0700 @@ -3493,9 +3493,9 @@ return repo.status(match=scmutil.match(repo[None], pats, opts)) -def revert(ui, repo, ctx, parents, *pats, **opts): +def revert(ui, repo, ctx, *pats, **opts): opts = pycompat.byteskwargs(opts) - parent, p2 = parents + parent, p2 = repo.dirstate.parents() node = ctx.node() mf = ctx.manifest() @@ -3781,7 +3781,6 @@ match = scmutil.match(repo[None], pats) _performrevert( repo, - parents, ctx, names, uipathfn, @@ -3807,7 +3806,6 @@ def _performrevert( repo, - parents, ctx, names, uipathfn, @@ -3823,7 +3821,7 @@ Make sure you have the working directory locked when calling this function. """ - parent, p2 = parents + parent, p2 = repo.dirstate.parents() node = ctx.node() excluded_files = [] diff -r bd56597b2254 -r 8c466bcb0879 mercurial/commands.py --- a/mercurial/commands.py Mon Aug 10 18:08:15 2020 -0700 +++ b/mercurial/commands.py Mon Aug 10 21:46:47 2020 -0700 @@ -837,7 +837,7 @@ else: hg.clean(repo, node, show_stats=False) repo.dirstate.setbranch(branch) - cmdutil.revert(ui, repo, rctx, repo.dirstate.parents()) + cmdutil.revert(ui, repo, rctx) if opts.get(b'no_commit'): msg = _(b"changeset %s backed out, don't forget to commit.\n") @@ -6301,9 +6301,7 @@ hint = _(b"use --all to revert all files") raise error.Abort(msg, hint=hint) - return cmdutil.revert( - ui, repo, ctx, (parent, p2), *pats, **pycompat.strkwargs(opts) - ) + return cmdutil.revert(ui, repo, ctx, *pats, **pycompat.strkwargs(opts)) @command( diff -r bd56597b2254 -r 8c466bcb0879 mercurial/shelve.py --- a/mercurial/shelve.py Mon Aug 10 18:08:15 2020 -0700 +++ b/mercurial/shelve.py Mon Aug 10 21:46:47 2020 -0700 @@ -772,7 +772,7 @@ with ui.configoverride({(b'ui', b'quiet'): True}): hg.update(repo, wctx.node()) ui.pushbuffer(True) - cmdutil.revert(ui, repo, shelvectx, repo.dirstate.parents()) + cmdutil.revert(ui, repo, shelvectx) ui.popbuffer() diff -r bd56597b2254 -r 8c466bcb0879 mercurial/subrepo.py --- a/mercurial/subrepo.py Mon Aug 10 18:08:15 2020 -0700 +++ b/mercurial/subrepo.py Mon Aug 10 21:46:47 2020 -0700 @@ -986,12 +986,11 @@ def filerevert(self, *pats, **opts): ctx = self._repo[opts['rev']] - parents = self._repo.dirstate.parents() if opts.get('all'): pats = [b'set:modified()'] else: pats = [] - cmdutil.revert(self.ui, self._repo, ctx, parents, *pats, **opts) + cmdutil.revert(self.ui, self._repo, ctx, *pats, **opts) def shortid(self, revid): return revid[:12]