[PATCH 3 of 5 V2] manifest: introduce manifestlog and manifestctx classes

[Martin von Zweigbergk]

On Wed, Aug 17, 2016 at 2:07 PM, Durham Goode <durham at fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1471465513 25200
> #      Wed Aug 17 13:25:13 2016 -0700
> # Node ID 00f8d3832aad368660c69eff4be3e03a5568aebe
> # Parent  8ddbe86953023c40beb7be9dcb8025d1055813c5
> manifest: introduce manifestlog and manifestctx classes
>
> This is the start of a large refactoring of the manifest class. It introduces
> the new manifestlog and manifestctx classes which will represent the collection
> of all manifests and individual instances, respectively.
>
> Future patches will begin to convert usages of repo.manifest to
> repo.manifestlog, adding the necessary functionality to manifestlog and instance
> as they are needed.

I'd appreciate seeing patch 5/5 squashed into this one. I don't like
introducing unused code like this without even tests (IIUC).

And are we dropping 4/5? I'm not sure what the status of your
discussion with foozy is.

>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -504,6 +504,10 @@ class localrepository(object):
>      def manifest(self):
>          return manifest.manifest(self.svfs)
>
> +    @storecache('00manifest.i')
> +    def manifestlog(self):
> +        return manifest.manifestlog(self.svfs, self.manifest)
> +
>      @repofilecache('dirstate')
>      def dirstate(self):
>          return dirstate.dirstate(self.vfs, self.ui, self.root,
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -914,6 +914,70 @@ class manifestrevlog(revlog.revlog):
>          super(manifestrevlog, self).clearcaches()
>          self._fulltextcache.clear()
>
> +class manifestlog(object):
> +    """A collection class representing the collection of manifest snapshots
> +    referenced by commits in the repository.
> +
> +    In this situation, 'manifest' refers to the abstract concept of a snapshot
> +    of the list of files in the given commit. Consumers of the output of this
> +    class do not care about the implementation details of the actual manifests
> +    they receive (i.e. tree or flat or lazily loaded, etc)."""
> +    def __init__(self, opener, oldmanifest):
> +        self._revlog = oldmanifest
> +
> +        # We'll separate this into it's own cache once oldmanifest is no longer
> +        # used
> +        self._mancache = oldmanifest._mancache
> +
> +        # _revlog is the same as _oldmanifest right now, but we eventually want
> +        # to delete _oldmanifest while still allowing manifestlog to access the
> +        # revlog specific apis.
> +        self._oldmanifest = oldmanifest
> +
> +    def __getitem__(self, node):
> +        """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:
> +            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):
> +                return cachemf
> +
> +        m = manifestctx(self._revlog, node)
> +        self._mancache[node] = m
> +        return m
> +
> +class manifestctx(manifestdict):
> +    """A class representing a single revision of a manifest, including its
> +    contents, its parent revs, and its linkrev.
> +    """
> +    def __init__(self, revlog, node):
> +        self._revlog = revlog
> +
> +        self._node = node
> +        self.p1, self.p2 = revlog.parents(node)
> +        rev = revlog.rev(node)
> +        self.linkrev = revlog.linkrev(rev)
> +
> +        # This should eventually be made lazy loaded, so consumers can access
> +        # the node/p1/linkrev data without having to parse the whole manifest.
> +        data = revlog.revision(node)
> +        arraytext = array.array('c', data)
> +        revlog._fulltextcache[node] = arraytext
> +        super(manifestctx, self).__init__(data)

I think much of this is not yet needed. The pedantic part of me wishes
it was left out until it is used, but I won't insist. I suppose it
serves as documentation of what future patches will add.

> +
> +    def node(self):
> +        return self._node
> +
>  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://www.mercurial-scm.org/mailman/listinfo/mercurial-devel