This instance will be upgraded to Heptapod 0.26.0rc1 on 2021-10-26 at 14:00 UTC+2 (a few minutes of down time)

Commit 7114532e authored by Martin von Zweigbergk's avatar Martin von Zweigbergk
Browse files

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
parent bd56597b2254
Pipeline #8919 failed with stage
in 25 minutes and 30 seconds
......@@ -635,12 +635,11 @@ def commitfuncfor(repo, src):
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:
......
......@@ -874,7 +874,7 @@ def overridecopy(orig, ui, repo, pats, opts, rename=False):
# 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 @@ def overriderevert(orig, ui, repo, ctx, parents, *pats, **opts):
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)
......
......@@ -1717,11 +1717,7 @@ class queue(object):
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
......
......@@ -3493,9 +3493,9 @@ def postcommitstatus(repo, pats, opts):
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 @@ def revert(ui, repo, ctx, parents, *pats, **opts):
match = scmutil.match(repo[None], pats)
_performrevert(
repo,
parents,
ctx,
names,
uipathfn,
......@@ -3807,7 +3806,6 @@ def revert(ui, repo, ctx, parents, *pats, **opts):
def _performrevert(
repo,
parents,
ctx,
names,
uipathfn,
......@@ -3823,7 +3821,7 @@ def _performrevert(
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 = []
......
......@@ -837,7 +837,7 @@ def _dobackout(ui, repo, node=None, rev=None, **opts):
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 @@ def revert(ui, repo, *pats, **opts):
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(
......
......@@ -772,7 +772,7 @@ def mergefiles(ui, repo, wctx, shelvectx):
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()
......
......@@ -986,12 +986,11 @@ class hgsubrepo(abstractsubrepo):
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]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment