[PATCH 2 of 2 V4] manifest: change manifestctx to not inherit from manifestdict

Martin von Zweigbergk martinvonz at google.com
Mon Sep 12 19:40:07 EDT 2016


Pushed to committed, thanks.

On Mon, Sep 12, 2016 at 2:33 PM, Durham Goode <durham at fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1473702943 25200
> #      Mon Sep 12 10:55:43 2016 -0700
> # Node ID 0291745860223e276e4be73a84eb7c097640388e
> # Parent  3a45359ff38aee2b4da284186024c3a83cc2b3ae
> 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
> @@ -962,12 +962,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
>
> @@ -978,21 +979,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
>
> @@ -1003,19 +1009,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
> @@ -1130,7 +1143,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()
> +            return cached
>          if self._treeondisk:
>              def gettext():
>                  return self.revision(node)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


More information about the Mercurial-devel mailing list