Patchwork [6,of,6,V2] fsmonitor: execute setup procedures only if dirstate is already instantiated

login
register
mail settings
Submitter Katsunori FUJIWARA
Date July 10, 2017, 2:18 p.m.
Message ID <f0f0d10b0692825a8d99.1499696299@speaknoevil>
Download mbox | patch
Permalink /patch/22205/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - July 10, 2017, 2:18 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1499695792 -32400
#      Mon Jul 10 23:09:52 2017 +0900
# Node ID f0f0d10b0692825a8d996d73180d58843391da63
# Parent  e7c8ebd3c341070ec069f0e5cc03cd0641f125c9
fsmonitor: execute setup procedures only if dirstate is already instantiated

Before this patch, reposetup() of fsmonitor executes setup procedures
for dirstate, even if it isn't yet instantiated at that time.

On the other hand, dirstate might be already instantiated before
reposetup() intentionally (prefilling by chg, for example, see
bf3af0eced44 for detail). If so, just discarding already instantiated
one in reposetup() causes issue.

To resolve both issues above, this patch executes setup procedures,
only if dirstate is already instantiated.

BTW, this patch removes "del repo.unfiltered().__dict__['dirstate']",
because it is responsibility of the code path, which causes
instantiation of dirstate before reposetup(). After this patch, using
localrepo.isfilecached() should avoid creating the corresponded entry
in repo.unfiltered().__dict__.
Jun Wu - July 11, 2017, 11:14 p.m.
I just double checked. The cleanup and fix is neat. Thanks!

Excerpts from FUJIWARA Katsunori's message of 2017-07-10 23:18:19 +0900:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1499695792 -32400
> #      Mon Jul 10 23:09:52 2017 +0900
> # Node ID f0f0d10b0692825a8d996d73180d58843391da63
> # Parent  e7c8ebd3c341070ec069f0e5cc03cd0641f125c9
> fsmonitor: execute setup procedures only if dirstate is already instantiated
> 
> Before this patch, reposetup() of fsmonitor executes setup procedures
> for dirstate, even if it isn't yet instantiated at that time.
> 
> On the other hand, dirstate might be already instantiated before
> reposetup() intentionally (prefilling by chg, for example, see
> bf3af0eced44 for detail). If so, just discarding already instantiated
> one in reposetup() causes issue.
> 
> To resolve both issues above, this patch executes setup procedures,
> only if dirstate is already instantiated.
> 
> BTW, this patch removes "del repo.unfiltered().__dict__['dirstate']",
> because it is responsibility of the code path, which causes
> instantiation of dirstate before reposetup(). After this patch, using
> localrepo.isfilecached() should avoid creating the corresponded entry
> in repo.unfiltered().__dict__.
> 
> diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py
> --- a/hgext/fsmonitor/__init__.py
> +++ b/hgext/fsmonitor/__init__.py
> @@ -698,15 +698,11 @@ def reposetup(ui, repo):
>          repo._fsmonitorstate = fsmonitorstate
>          repo._watchmanclient = client
>  
> -        # at this point since fsmonitorstate wasn't present, repo.dirstate is
> -        # not a fsmonitordirstate
> -        dirstate = repo.dirstate
> -        makedirstate(repo, dirstate)
> -
> -        # invalidate property cache, but keep filecache which contains the
> -        # wrapped dirstate object
> -        del repo.unfiltered().__dict__['dirstate']
> -        assert dirstate is repo._filecache['dirstate'].obj
> +        dirstate, cached = localrepo.isfilecached(repo, 'dirstate')
> +        if cached:
> +            # at this point since fsmonitorstate wasn't present,
> +            # repo.dirstate is not a fsmonitordirstate
> +            makedirstate(repo, dirstate)
>  
>          class fsmonitorrepo(repo.__class__):
>              def status(self, *args, **kwargs):

Patch

diff --git a/hgext/fsmonitor/__init__.py b/hgext/fsmonitor/__init__.py
--- a/hgext/fsmonitor/__init__.py
+++ b/hgext/fsmonitor/__init__.py
@@ -698,15 +698,11 @@  def reposetup(ui, repo):
         repo._fsmonitorstate = fsmonitorstate
         repo._watchmanclient = client
 
-        # at this point since fsmonitorstate wasn't present, repo.dirstate is
-        # not a fsmonitordirstate
-        dirstate = repo.dirstate
-        makedirstate(repo, dirstate)
-
-        # invalidate property cache, but keep filecache which contains the
-        # wrapped dirstate object
-        del repo.unfiltered().__dict__['dirstate']
-        assert dirstate is repo._filecache['dirstate'].obj
+        dirstate, cached = localrepo.isfilecached(repo, 'dirstate')
+        if cached:
+            # at this point since fsmonitorstate wasn't present,
+            # repo.dirstate is not a fsmonitordirstate
+            makedirstate(repo, dirstate)
 
         class fsmonitorrepo(repo.__class__):
             def status(self, *args, **kwargs):