[PATCH 3 of 3 V2] manifest: change manifestctx to not inherit from manifestdict

[Martin von Zweigbergk]

Queued, thanks!

I've said it before, but I really like the direction this is going.
I'm a little embarrassed I didn't think of creating the context
objects myself. It seems so obvious now :-)

Now I just need to figure out how to make narrowhg deal with these
changes. Shouldn't be too hard, but will probably involve some
duplication during the transition, just like manifest.py itself has a
lot of duplication during the transition.

On Wed, Aug 31, 2016 at 1:30 PM, Durham Goode <durham at fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1472672813 25200
> #      Wed Aug 31 12:46:53 2016 -0700
> # Node ID e65cbaceb14f4b522ab59cdccf3f4b2761b2e3bd
> # Parent  59bc68e1d78538bb83f60c0d4f9342ec0a8893bf
> manifest: change manifestctx to not inherit from manifestdict
>
> If manifestctx inherits from manifestdict, it requires some weird logic to
> lazily load the dict if a piece of information is asked for. This ended up being
> complicated and unintuitive to use.
>
> Let's move the dict creation to .read(). This will make even more sense once we
> start adding readdelta() and other similar methods to manifestctx.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -528,7 +528,7 @@ class changectx(basectx):
>
>      @propertycache
>      def _manifest(self):
> -        return self._repo.manifestlog[self._changeset.manifest]
> +        return self._repo.manifestlog[self._changeset.manifest].read()
>
>      @propertycache
>      def _manifestdelta(self):
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -960,12 +960,13 @@ class manifestlog(object):
>              self._mancache[node] = m
>          return m
>
> -class manifestctx(manifestdict):
> +class manifestctx(object):
>      """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._data = None
>
>          self._node = node
>
> @@ -976,21 +977,26 @@ class manifestctx(manifestdict):
>          #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)
> -
>      def node(self):
>          return self._node
>
> -class treemanifestctx(treemanifest):
> +    def read(self):
> +        if not self._data:
> +            if self._node == revlog.nullid:
> +                self._data = manifestdict()
> +            else:
> +                text = self._revlog.revision(self._node)
> +                arraytext = array.array('c', text)
> +                self._revlog._fulltextcache[self._node] = arraytext
> +                self._data = manifestdict(text)
> +        return self._data
> +
> +class treemanifestctx(object):
>      def __init__(self, revlog, dir, node):
>          revlog = revlog.dirlog(dir)
>          self._revlog = revlog
>          self._dir = dir
> +        self._data = None
>
>          self._node = node
>
> @@ -1001,19 +1007,26 @@ class treemanifestctx(treemanifest):
>          #rev = revlog.rev(node)
>          #self.linkrev = revlog.linkrev(rev)
>
> -        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:
> -            text = revlog.revision(node)
> -            arraytext = array.array('c', text)
> -            revlog.fulltextcache[node] = arraytext
> -            super(treemanifestctx, self).__init__(dir=dir, text=text)
> +    def read(self):
> +        if not self._data:
> +            if self._node == revlog.nullid:
> +                self._data = treemanifest()
> +            elif self._revlog._treeondisk:
> +                m = treemanifest(dir=self._dir)
> +                def gettext():
> +                    return self._revlog.revision(self._node)
> +                def readsubtree(dir, subm):
> +                    return treemanifestctx(self._revlog, dir, subm).read()
> +                m.read(gettext, readsubtree)
> +                m.setnode(self._node)
> +                self._data = m
> +            else:
> +                text = self._revlog.revision(self._node)
> +                arraytext = array.array('c', text)
> +                self._revlog.fulltextcache[self._node] = arraytext
> +                self._data = treemanifest(dir=self._dir, text=text)
> +
> +        return self._data
>
>      def node(self):
>          return self._node
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel