[PATCH 1 of 2] manifest: make revlog verification optional

Durham Goode durham at fb.com
Mon Nov 14 19:26:53 EST 2016


On 11/14/16 4:23 PM, Martin von Zweigbergk wrote:
> On Mon, Nov 14, 2016 at 3:27 PM, Durham Goode <durham at fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1479165447 28800
>> #      Mon Nov 14 15:17:27 2016 -0800
>> # Node ID 27209d52a5865422c5ef4ba05cedb28ce32919ed
>> # Parent  046a7e828ea63ec940ffae1089a33fae7954da2e
>> manifest: make revlog verification optional
>>
>> This patches adds an parameter to manifestlog.get() to disable hash checking.
>> This will be used in an upcoming patch to support treemanifestctx reading
>> sub-trees without loading them from the revlog. (This is already supported but
>> does not go through the manifestlog.get() code path)
>>
>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>> --- a/mercurial/manifest.py
>> +++ b/mercurial/manifest.py
>> @@ -1278,9 +1278,12 @@ class manifestlog(object):
>>           """
>>           return self.get('', node)
>>
>> -    def get(self, dir, node):
>> +    def get(self, dir, node, verify=True):
>>           """Retrieves the manifest instance for the given node. Throws a
>>           LookupError if not found.
>> +
>> +        `verify` - if True an exception will be thrown if the node is not in
>> +                   the revlog
> Patch 2/2 says "Set verify to False since we need to be able to create
> subtrees for trees that don't exist on disk yet.", which makes it
> sounds like it's only about reading empty revlogs, which should be
> safe anyway. What am I missing?
"yet" was probably the wrong word there.  This logic is to support the 
test case where the test moves some of the revlogs out of the way and 
expects the treemanifest to still work because those revlogs are outside 
the narrow spec. So this verification needs to be turned off so the ctx 
can be created (and it just so happens that the contents of the ctx are 
never accessed).

I think it's a bit weird that readsubtree() is called for trees outside 
the narrow spec (which is why we need to handle this case), but that 
would require a deeper refactoring.


More information about the Mercurial-devel mailing list