Mercurial > public > mercurial-scm > hg-stable
comparison mercurial/cmdutil.py @ 45395:8c466bcb0879
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
| author | Martin von Zweigbergk <martinvonz@google.com> |
|---|---|
| date | Mon, 10 Aug 2020 21:46:47 -0700 |
| parents | 77b8588dd84e |
| children | 6ba7190ff863 |
comparison
equal
deleted
inserted
replaced
| 45394:bd56597b2254 | 45395:8c466bcb0879 |
|---|---|
| 3491 | 3491 |
| 3492 def postcommitstatus(repo, pats, opts): | 3492 def postcommitstatus(repo, pats, opts): |
| 3493 return repo.status(match=scmutil.match(repo[None], pats, opts)) | 3493 return repo.status(match=scmutil.match(repo[None], pats, opts)) |
| 3494 | 3494 |
| 3495 | 3495 |
| 3496 def revert(ui, repo, ctx, parents, *pats, **opts): | 3496 def revert(ui, repo, ctx, *pats, **opts): |
| 3497 opts = pycompat.byteskwargs(opts) | 3497 opts = pycompat.byteskwargs(opts) |
| 3498 parent, p2 = parents | 3498 parent, p2 = repo.dirstate.parents() |
| 3499 node = ctx.node() | 3499 node = ctx.node() |
| 3500 | 3500 |
| 3501 mf = ctx.manifest() | 3501 mf = ctx.manifest() |
| 3502 if node == p2: | 3502 if node == p2: |
| 3503 parent = p2 | 3503 parent = p2 |
| 3779 repo, [(ctx.rev(), matchfiles)], | 3779 repo, [(ctx.rev(), matchfiles)], |
| 3780 ) | 3780 ) |
| 3781 match = scmutil.match(repo[None], pats) | 3781 match = scmutil.match(repo[None], pats) |
| 3782 _performrevert( | 3782 _performrevert( |
| 3783 repo, | 3783 repo, |
| 3784 parents, | |
| 3785 ctx, | 3784 ctx, |
| 3786 names, | 3785 names, |
| 3787 uipathfn, | 3786 uipathfn, |
| 3788 actions, | 3787 actions, |
| 3789 match, | 3788 match, |
| 3805 ) | 3804 ) |
| 3806 | 3805 |
| 3807 | 3806 |
| 3808 def _performrevert( | 3807 def _performrevert( |
| 3809 repo, | 3808 repo, |
| 3810 parents, | |
| 3811 ctx, | 3809 ctx, |
| 3812 names, | 3810 names, |
| 3813 uipathfn, | 3811 uipathfn, |
| 3814 actions, | 3812 actions, |
| 3815 match, | 3813 match, |
| 3821 This is an independent function to let extension to plug in and react to | 3819 This is an independent function to let extension to plug in and react to |
| 3822 the imminent revert. | 3820 the imminent revert. |
| 3823 | 3821 |
| 3824 Make sure you have the working directory locked when calling this function. | 3822 Make sure you have the working directory locked when calling this function. |
| 3825 """ | 3823 """ |
| 3826 parent, p2 = parents | 3824 parent, p2 = repo.dirstate.parents() |
| 3827 node = ctx.node() | 3825 node = ctx.node() |
| 3828 excluded_files = [] | 3826 excluded_files = [] |
| 3829 | 3827 |
| 3830 def checkout(f): | 3828 def checkout(f): |
| 3831 fc = ctx[f] | 3829 fc = ctx[f] |