[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 17:07:28 UTC 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