[PATCH 4 of 5 V2] manifest: use property instead of field for manifest revlog storage

Martin von Zweigbergk martinvonz at google.com
Wed Aug 31 13:07:28 EDT 2016


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 d8456d11c582e4b073ed9a7bf8098ee4393df5ca
> # Parent  00f8d3832aad368660c69eff4be3e03a5568aebe
> manifest: use property instead of field for manifest revlog storage
>
> The file caches we're using to avoid reloading the manifest from disk everytime
> has an annoying bug that causes the in memory structure to not be reloaded if
> the mtime and the size haven't changed. This causes a breakage in the tests
> because the manifestlog is not being reloaded after a commit+strip operation in
> mq (the mtime is the same because it all happens in the same second, and the
> resulting size is the same because we add 1 and remove 1). The only reason this
> doesn't affect the manifest itself is because we touch it so often that we
> had already reloaded it after the commit, but before the strip.
>
> Once the entire manifest has migrated to manifestlog, we can get rid of these
> properties, since then the manifestlog will be touched after the commit, but
> before the strip, as well.
>
> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
> --- a/mercurial/localrepo.py
> +++ b/mercurial/localrepo.py
> @@ -504,9 +504,9 @@ class localrepository(object):
>      def manifest(self):
>          return manifest.manifest(self.svfs)
>
> -    @storecache('00manifest.i')
> +    @property
>      def manifestlog(self):
> -        return manifest.manifestlog(self.svfs, self.manifest)
> +        return manifest.manifestlog(self.svfs, self)
>
>      @repofilecache('dirstate')
>      def dirstate(self):
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -922,17 +922,23 @@ class manifestlog(object):
>      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
> +    def __init__(self, opener, repo):
> +        self._repo = repo

I didn't notice until now, but I'm pretty sure this is a layering
violation: the revlogs are not supposed to know about the repo. Will
this have gone away by the end of your refactoring?

>
>          # We'll separate this into it's own cache once oldmanifest is no longer
>          # used
> -        self._mancache = oldmanifest._mancache
> +        self._mancache = repo.manifest._mancache
>
> +    @property
> +    def _revlog(self):
> +        return self._repo.manifest
> +
> +    @property
> +    def _oldmanifest(self):
>          # _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
> +        return self._repo.manifest
>
>      def __getitem__(self, node):
>          """Retrieves the manifest instance for the given node. Throws a KeyError
> _______________________________________________
> 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