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

[Pierre-Yves David]

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.
I'm speaking at the whole durin42/google effort to have manifest related 
operation lazy.


-- 
Pierre-Yves David