dirstate: use a single closure for get_flags

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.

Differential Revision: https://phab.mercurial-scm.org/D11728

mercurial/dirstate.py

@@ -242,68 +242,59 b' class dirstate(object):'
         return self._rootdir + f
 
     def flagfunc(self, buildfallback):
-        if not (self._checklink and self._checkexec):
-            fallback = buildfallback()
+        """build a callable that returns flags associated with a filename
 
-        def check_both(x):
-            """This platform supports symlinks and exec permissions"""
+        The information is extracted from three possible layers:
+        1. the file system if it supports the information
+        2. the "fallback" information stored in the dirstate if any
+        3. a more expensive mechanism inferring the flags from the parents.
+        """
+
+        # small hack to cache the result of buildfallback()
+        fallback_func = []
+
+        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):

tests/test-share.t

@@ -47,8 +47,8 b" share shouldn't have a full cache dir, o"
   [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

tests/test-subrepo.t

@@ -1275,8 +1275,8 b' Check that share works with subrepo'
   ../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