[PATCH STABLE] revert: properly revert to ancestor of p2 during merge

[Martin von Zweigbergk]

On Tue, Feb 23, 2016 at 10:17 AM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
>
>
> On 02/23/2016 07:04 PM, Martin von Zweigbergk wrote:
>>
>> On Tue, Feb 23, 2016 at 4:02 AM, Pierre-Yves David
>> <pierre-yves.david at ens-lyon.org> wrote:
>>>
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david at fb.com>
>>> # Date 1456224107 -3600
>>> #      Tue Feb 23 11:41:47 2016 +0100
>>> # Branch stable
>>> # Node ID aa94f42b742e958bd9c3f2f7a82722a7058aee76
>>> # Parent  1bcb4f34b9f91a2e330966182f691664fbada1bc
>>> # Available At http://hg.netv6.net/marmoute-wip/mercurial/
>>> #              hg pull http://hg.netv6.net/marmoute-wip/mercurial/ -r
>>> aa94f42b742e
>>> revert: properly revert to ancestor of p2 during merge
>>>
>>> During merge, added (from one perspective) file can be reported as
>>> "modified".
>>> To work around that, revert was testing if modified file where present in
>>> the
>>> parent manifest and marking them as "added" in this case. However, we
>>> should be
>>> checking against the target revision manifest instead. Otherwise see file
>>> as
>>> "newly added" even if they exist in the target revision.
>>>
>>> That revert behavior regressed in 06fbd9518bc5.
>>>
>>> Note: in this patch we can notice that we are building a set out of a
>>> manifest,
>>> breaking effort for manifest laziness introduced since this code was
>>> written.
>>> Fixing it is another adventure.
>>>
>>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>>> --- a/mercurial/cmdutil.py
>>> +++ b/mercurial/cmdutil.py
>>> @@ -2844,14 +2844,10 @@ def revert(ui, repo, ctx, parents, *pats
>>>       node = ctx.node()
>>>
>>>       mf = ctx.manifest()
>>>       if node == p2:
>>>           parent = p2
>>> -    if node == parent:
>>> -        pmf = mf
>>> -    else:
>>> -        pmf = None
>>>
>>>       # need all matching names in dirstate and manifest of target rev,
>>>       # so have to walk both. do not print errors if files exist in one
>>>       # but not other. in both cases, filesets should be evaluated
>>> against
>>>       # workingctx to get consistent result (issue4497). this means
>>> 'set:**'
>>> @@ -2962,15 +2958,11 @@ def revert(ui, repo, ctx, parents, *pats
>>>               dsadded = added
>>>
>>>           # in case of merge, files that are actually added can be
>>> reported as
>>>           # modified, we need to post process the result
>>>           if p2 != nullid:
>>> -            if pmf is None:
>>> -                # only need parent manifest in the merge case,
>>> -                # so do not read by default
>>> -                pmf = repo[parent].manifest()
>>> -            mergeadd = dsmodified - set(pmf)
>>> +            mergeadd = dsmodified - set(mf)
>>
>>
>>
>> I have replaced "set(mf)" by "smf", which was already defined.
>
>
> Ha good point, thanks
>
>> Does
>> that mean I should also drop the part about "breaking effort for
>> manifest laziness" in the commit message? What was that effort? Could
>> you point to some commits?
>
>
> You can drop it, we are not worse that what smf is doing.

Done. Pushed to the clowncopter.

> I'm speaking at the whole durin42/google effort to have manifest related
> operation lazy.

I thought you might be referring to my treemanifest work, but it
turned out I had not made any relevant changes to revert. However, I
found some old patches in my repo that I should take another look at
:-)