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

[FUJIWARA Katsunori]

At Mon, 8 Aug 2016 18:17:13 -0700,
Durham Goode wrote:
> 
> # HG changeset patch
> # User Durham Goode <durham at fb.com>
> # Date 1470697899 25200
> #      Mon Aug 08 16:11:39 2016 -0700
> # Node ID 6a4c09571793d56c8dad1a6760e3fc1293b9a0b6
> # Parent  f73abdf84e8e2eb9e2029cb28e2246a55b2d2f49
> 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.

All of main tasks of ExactCacheValidationPlan were included into main
repository before release of 3.9.

    https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan

Therefore, change of file should be detected at validation of file
cache, even if writing changed file out (without extra handling in
this plan) causes same ctime/mtime/size/i-node and so on.

Would you tell me the case to reproduce the issue around caching
manifest, if it still occurs on recent Mercurial ? It helps to find
out code paths overlooked by ExactCacheValidationPlan.


> 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
> @@ -921,17 +921,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
>  
>          # 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

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp