[Reviewers] [PATCH 2 of 3 V2] manifest: make uses of _mancache aware of contexts

Pierre-Yves David pierre-yves.david at ens-lyon.org
Sat Sep 10 00:10:01 UTC 2016


On 09/08/2016 09:16 PM, Martin von Zweigbergk via Mercurial-devel wrote:
> Dropped.

Okay, apparently they made it back into the repository and got 
published. After chatting with Martin and Durham I've pushed a backout 
of these two. If some other reviewers could give them a second 
acceptance stamp (Martin did one) that would be great.

>
> On Thu, Sep 8, 2016 at 11:19 AM, Durham Goode <durham at fb.com> wrote:
>>
>>
>> On 9/7/16 9:10 AM, Pierre-Yves David wrote:
>>>
>>>
>>>
>>> On 09/07/2016 05:35 PM, Pierre-Yves David wrote:
>>>>
>>>>
>>>>
>>>> On 09/03/2016 05:51 PM, Yuya Nishihara wrote:
>>>>>
>>>>> On Wed, 31 Aug 2016 13:30:20 -0700, Durham Goode wrote:
>>>>>>
>>>>>> # HG changeset patch
>>>>>> # User Durham Goode <durham at fb.com>
>>>>>> # Date 1472518929 25200
>>>>>> #      Mon Aug 29 18:02:09 2016 -0700
>>>>>> # Node ID 59bc68e1d78538bb83f60c0d4f9342ec0a8893bf
>>>>>> # Parent  abce9af35512d8589683d94f34f6d8aa21163568
>>>>>> manifest: make uses of _mancache aware of contexts
>>>>>
>>>>>
>>>>>> --- a/mercurial/manifest.py
>>>>>> +++ b/mercurial/manifest.py
>>>>>> @@ -1128,7 +1128,11 @@ class manifest(manifestrevlog):
>>>>>>          if node == revlog.nullid:
>>>>>>              return self._newmanifest() # don't upset local cache
>>>>>>          if node in self._mancache:
>>>>>> -            return self._mancache[node]
>>>>>> +            cached = self._mancache[node]
>>>>>> +            if (isinstance(cached, manifestctx) or
>>>>>> +                isinstance(cached, treemanifestctx)):
>>>>>> +                cached = cached.read()
>>>>>
>>>>>
>>>>> manifestctx.read() is added by the next patch, so tests fail at this
>>>>> revision.
>>>>> But that wouldn't be a good reason enough to rework this series.
>>>>
>>>>
>>>> This changeset or the next also introduce a strange failure in evolve
>>>> tests where a manifest hash changes (eg: test-evolve.t). please hold on
>>>> during investigation.
>>>
>>>
>>> So, this is strange, this changeset or the next affect various test in
>>> evolve. I've digged in details for test-evolve.t where evolve create a
>>> changeset with a different hash.
>>>
>>> I've added various debug data to the test to track how they change and:
>>>
>>> * the operation is a bump fix
>>>
>>> * the changeset have a different hash because manifest hash change (kinda
>>> expected),
>>>
>>> * the manifest have a different hash because one of the file hash change,
>>>
>>> * file content and commit diff are similar
>>>
>>> * the file hash change because the revlog gains a p1 (from a previous null
>>> ID),
>>>
>>> * The previous value (no parent for the filerev) is most probably wrong as
>>> the file appear "Modified" in the commit.
>>>
>>> So, something in this changes seems to be magically "fix" evolve.
>>>
>>> That's suspicious and I would be much more comfortable if we understood
>>> what is going on here. I'm about to switch out of work mode for the evening,
>>> can someone else look into this (ideal Durham as he is familliar with his
>>> change.
>>
>> I'll look into this when I get a moment.  Feel free to drop the series until
>> then.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David



More information about the Reviewers mailing list