[Request] [+-- ] D11728: dirstate: use a single closure for get_flags

[marmoute (Pierre-Yves David)]

marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The previous code was overlooking fallback when neither symlink not exec was
  supported.
  
  The number of "variants" is getting too high, so I am consolidating this in a
  single closure that should be easier to maintains.
  
  This also ensure that fallback flags are always taken into account.
  
  (they are not user code yet, but small experimentation shown that the feature
  was working as intended.)
  
  A a small side effect we need to check for symlink support more lazily and this
  show up in the test in a couple of places.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11728

AFFECTED FILES
  mercurial/dirstate.py
  tests/test-share.t
  tests/test-subrepo.t

CHANGE DETAILS

diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -1275,8 +1275,8 @@
   ../shared/subrepo-2/.hg/sharedpath
   ../shared/subrepo-2/.hg/wcache
   ../shared/subrepo-2/.hg/wcache/checkisexec (execbit !)
-  ../shared/subrepo-2/.hg/wcache/checklink (symlink !)
-  ../shared/subrepo-2/.hg/wcache/checklink-target (symlink !)
+  ../shared/subrepo-2/.hg/wcache/checklink (symlink no-rust !)
+  ../shared/subrepo-2/.hg/wcache/checklink-target (symlink no-rust !)
   ../shared/subrepo-2/.hg/wcache/manifestfulltextcache (reporevlogstore !)
   ../shared/subrepo-2/file
   $ hg -R ../shared in
diff --git a/tests/test-share.t b/tests/test-share.t
--- a/tests/test-share.t
+++ b/tests/test-share.t
@@ -47,8 +47,8 @@
   [1]
   $ ls -1 .hg/wcache || true
   checkisexec (execbit !)
-  checklink (symlink !)
-  checklink-target (symlink !)
+  checklink (symlink no-rust !)
+  checklink-target (symlink no-rust !)
   manifestfulltextcache (reporevlogstore !)
   $ ls -1 ../repo1/.hg/cache
   branch2-served
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -242,68 +242,52 @@
         return self._rootdir + f
 
     def flagfunc(self, buildfallback):
-        if not (self._checklink and self._checkexec):
-            fallback = buildfallback()
+
+        # small hack to cache the result of buildfallback()
+        fallback_func = []
 
-        def check_both(x):
-            """This platform supports symlinks and exec permissions"""
+        def get_flags(x):
+            entry = None
+            fallback_value = None
             try:
                 st = os.lstat(self._join(x))
+            except OSError:
+                return b''
+
+            if self._checklink:
                 if util.statislink(st):
                     return b'l'
+            else:
+                entry = self.get_entry(x)
+                if entry.has_fallback_symlink:
+                    if entry.fallback_symlink:
+                        return b'l'
+                else:
+                    if not fallback_func:
+                        fallback_func.append(buildfallback())
+                    fallback_value = fallback_func[0](x)
+                    if b'l' in fallback_value:
+                        return b'l'
+
+            if self._checkexec:
                 if util.statisexec(st):
                     return b'x'
-            except OSError:
-                pass
-            return b''
-
-        def check_link(x):
-            """This platform only supports symlinks"""
-            if os.path.islink(self._join(x)):
-                return b'l'
-            entry = self.get_entry(x)
-            if entry.has_fallback_exec:
-                if entry.fallback_exec:
-                    return b'x'
-            elif b'x' in fallback(x):
-                return b'x'
+            else:
+                if entry is None:
+                    entry = self.get_entry(x)
+                if entry.has_fallback_exec:
+                    if entry.fallback_exec:
+                        return b'x'
+                else:
+                    if fallback_value is None:
+                        if not fallback_func:
+                            fallback_func.append(buildfallback())
+                        fallback_value = fallback_func[0](x)
+                    if b'x' in fallback_value:
+                        return b'x'
             return b''
 
-        def check_exec(x):
-            """This platform only supports exec permissions"""
-            if b'l' in fallback(x):
-                return b'l'
-            entry = self.get_entry(x)
-            if entry.has_fallback_symlink:
-                if entry.fallback_symlink:
-                    return b'l'
-            if util.isexec(self._join(x)):
-                return b'x'
-            return b''
-
-        def check_fallback(x):
-            """This platform supports neither symlinks nor exec permissions, so
-            check the fallback in the dirstate if it exists, otherwise figure it
-            out the more expensive way from the parents."""
-            entry = self.get_entry(x)
-            if entry.has_fallback_symlink:
-                if entry.fallback_symlink:
-                    return b'l'
-            if entry.has_fallback_exec:
-                if entry.fallback_exec:
-                    return b'x'
-                elif entry.has_fallback_symlink:
-                    return b''
-            return fallback(x)
-
-        if self._checklink and self._checkexec:
-            return check_both
-        elif self._checklink:
-            return check_link
-        elif self._checkexec:
-            return check_exec
-        else:
-            return check_fallback
+        return get_flags
 
     @propertycache
     def _cwd(self):



To: marmoute, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20211028/db6c5ffd/attachment-0001.html>