[PATCH 1 of 4] manifest: add treemanifestctx class
Durham Goode
durham at fb.com
Wed Aug 31 15:43:43 EDT 2016
On 8/31/16 10:33 AM, Martin von Zweigbergk wrote:
> On Tue, Aug 30, 2016 at 4:40 PM, Durham Goode <durham at fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1472518307 25200
>> # Mon Aug 29 17:51:47 2016 -0700
>> # Node ID a6f968072eb72212bf8e16fb74114654aca98330
>> # Parent 318e2b600b80e4ed3c6f37df46ec7544f60d4c0b
>> manifest: add treemanifestctx class
>>
>> Before we start using repo.manifestlog in the rest of the code base, we need to
>> make sure that it's capable of returning treemanifests. As we add new
>> functionality to manifestctx, we'll add it to treemanifestctx at the same time.
>>
>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>> --- a/mercurial/manifest.py
>> +++ b/mercurial/manifest.py
>> @@ -944,21 +944,24 @@ class manifestlog(object):
>> """Retrieves the manifest instance for the given node. Throws a KeyError
>> if not found.
>> """
>> - if (self._oldmanifest._treeondisk
>> - or self._oldmanifest._treeinmem):
>> - # TODO: come back and support tree manifests directly
>> - return self._oldmanifest.read(node)
>> + if node == revlog.nullid:
>> + if self._oldmanifest._treeinmem:
>> + return treemanifest()
>> + else:
>> + return manifestdict()
> At this point of the stack, his method (manifestlog.__getitem__()) can
> return either a manifest (treemanifest/manifestdict) or a context
> (treemanifestctx/manifestctx). Please fix :-) Perhaps just make "
> self._mancache[node] = m" conditional (on node != nullid)? Unless
> nullid is requested very frequently, I suspect the extra "if node in
> self._mancache" (which will be False) won't be an issue.
Will do
>
>> - if node == revlog.nullid:
>> - return manifestdict()
>> if node in self._mancache:
>> cachemf = self._mancache[node]
>> # The old manifest may put non-ctx manifests in the cache, so skip
>> # those since they don't implement the full api.
>> - if isinstance(cachemf, manifestctx):
>> + if (isinstance(cachemf, manifestctx) or
>> + isinstance(cachemf, treemanifestctx)):
> Maybe this suggests that a super class for manifestctx and
> treemanifestctx would make sense? Maybe manifestctx, flatmanifestctx
> and treemanifestctx? If that makes sense, it could of course be added
> after this part of the series (or as a fifth patch in the series).
Perhaps. But this code will be disappearing once the mancache is only
populated with ctx's.
>
>> return cachemf
>>
>> - m = manifestctx(self._revlog, node)
>> + if self._oldmanifest._treeinmem:
>> + m = treemanifestctx(self._revlog, node, dir='')
>> + else:
>> + m = manifestctx(self._revlog, node)
>> self._mancache[node] = m
>> return m
>>
>> @@ -984,6 +987,35 @@ class manifestctx(manifestdict):
>> def node(self):
>> return self._node
>>
>> +class treemanifestctx(treemanifest):
> manifestctx also has a node() method, so you should probably add that
> to treemanifestctx for consistency.
Will do
>
>> + def __init__(self, revlog, node, dir=''):
> You always pass a value for dir, so make it not optional? And I'd like
> to see it before the node argument, since it defines the scope for the
> node (it's the node for a directory, not the directory for a node).
Will do
>
>> + revlog = revlog.dirlog(dir)
>> + self._revlog = revlog
>> + self._dir = dir
>> +
>> + self._node = node
>> +
>> + # TODO: Load p1/p2/linkrev lazily. They need to be lazily loaded so that
>> + # we can instantiate treemanifestctx objects for directories we don't
>> + # have on disk.
>> + #self.p1, self.p2 = revlog.parents(node)
>> + #rev = revlog.rev(node)
>> + #self.linkrev = revlog.linkrev(rev)
> Do you mind commenting out the ones in manifestctx too for now? I'm
> worried we otherwise might break treemanifests when we start using
> p1/p2/rev/linkrev and not notice because it's not covered as well by
> tests.
Will do
>
>> +
>> + if revlog._treeondisk:
>> + super(treemanifestctx, self).__init__(dir=dir)
>> + def gettext():
>> + return revlog.revision(node)
>> + def readsubtree(dir, subm):
>> + return revlog.dirlog(dir).read(subm)
>> + self.read(gettext, readsubtree)
>> + self.setnode(node)
>> + else:
>> + data = revlog.revision(node)
>> + arraytext = array.array('c', data)
>> + revlog.fulltextcache[node] = arraytext
>> + super(treemanifestctx, self).__init__(dir=dir, text=data)
>> +
>> class manifest(manifestrevlog):
>> def __init__(self, opener, dir='', dirlogcache=None):
>> '''The 'dir' and 'dirlogcache' arguments are for internal use by
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel at mercurial-scm.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=7SFuBPXwDS16G42wgq9qhYwEXsl7NwAi_Bgfq1u3ekQ&s=xUWhiuVGCJz58KtvVIygw7XmK2Nhq8LSCVhK72rZjHE&e=
More information about the Mercurial-devel
mailing list