Completely overhaul caching of external dependencies

The old caching was a mess of spaghetti code layered over pasta code.

The new code is well-commented, is clear about what it's trying to do,
and uses a blacklist of keyword arguments instead of a whitelist while
generating identifiers for dep caching which makes it much more robust
for future changes.

The only side-effect of forgetting about a new keyword argument would
be that the dependency would not be cached unless the values of that
keyword arguments were the same in the cached and new dependency.

There are also more tests which identify scenarios that were broken
earlier.

1 files changed, 67 insertions, 25 deletions

diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py
index 46ec3f3..c1926d0 100644
--- a/mesonbuild/interpreter.py
+++ b/mesonbuild/interpreter.py
@@ -23,7 +23,8 @@ from . import compilers
 from .wrap import wrap, WrapMode
 from . import mesonlib
 from .mesonlib import FileMode, Popen_safe, get_meson_script
-from .dependencies import InternalDependency, Dependency, ExternalProgram
+from .dependencies import ExternalProgram
+from .dependencies import InternalDependency, Dependency, DependencyException
 from .interpreterbase import InterpreterBase
 from .interpreterbase import check_stringlist, noPosargs, noKwargs, stringArgs
 from .interpreterbase import InterpreterException, InvalidArguments, InvalidCode
@@ -1852,12 +1853,16 @@ class Interpreter(InterpreterBase):
     def func_find_library(self, node, args, kwargs):
         mlog.log(mlog.red('DEPRECATION:'), 'find_library() is removed, use the corresponding method in compiler object instead.')
 
-    def func_dependency(self, node, args, kwargs):
-        self.validate_arguments(args, 1, [str])
-        name = args[0]
-        if '<' in name or '>' in name or '=' in name:
-            raise InvalidArguments('Characters <, > and = are forbidden in dependency names. To specify'
-                                   'version\n requirements use the \'version\' keyword argument instead.')
+    def _find_cached_dep(self, name, kwargs):
+        '''
+        Check that there aren't any mismatches between the cached dep and the
+        wanted dep in terms of version and whether to use a fallback or not.
+        For instance, the cached dep and the wanted dep could have mismatching
+        version requirements. The cached dep did not search for a fallback, but
+        the wanted dep specifies a fallback. There are many more edge-cases.
+        Most cases are (or should be) documented in:
+        `test cases/linuxlike/5 dependency versions/meson.build`
+        '''
         # Check if we want this as a cross-dep or a native-dep
         # FIXME: Not all dependencies support such a distinction right now,
         # and we repeat this check inside dependencies that do. We need to
@@ -1868,52 +1873,89 @@ class Interpreter(InterpreterBase):
         else:
             want_cross = is_cross
         identifier = dependencies.get_dep_identifier(name, kwargs, want_cross)
-        # Check if we've already searched for and found this dep
         cached_dep = None
+        # Check if we've already searched for and found this dep
         if identifier in self.coredata.deps:
             cached_dep = self.coredata.deps[identifier]
-            # All other kwargs are handled in get_dep_identifier(). We have
-            # this here as a tiny optimization to avoid searching for
-            # dependencies that we already have.
-            if 'version' in kwargs:
-                wanted = kwargs['version']
-                found = cached_dep.get_version()
-                if not cached_dep.found() or \
-                   not mesonlib.version_compare_many(found, wanted)[0]:
-                    # Cached dep has the wrong version. Check if an external
-                    # dependency or a fallback dependency provides it.
-                    cached_dep = None
+        else:
+            # Check if exactly the same dep with different version requirements
+            # was found already.
+            # We only return early if we find a usable cached dependency since
+            # there might be multiple cached dependencies like this.
+            w = identifier[1]
+            for trial, trial_dep in self.coredata.deps.items():
+                # trial[1], identifier[1] are the version requirements
+                if trial[0] != identifier[0] or trial[2:] != identifier[2:]:
+                    continue
+                # The version requirements are the only thing that's different.
+                if trial_dep.found():
+                    # Cached dependency was found. We're close.
+                    f = trial_dep.get_version()
+                    if not w or mesonlib.version_compare_many(f, w)[0]:
+                        # We either don't care about the version, or the
+                        # cached dep version matches our requirements! Yay!
+                        return identifier, trial_dep
+                elif 'fallback' not in kwargs:
+                    # this cached dependency matched everything but was
+                    # not-found. Tentatively set this as the dep to use.
+                    # If no other cached dep matches, we will use this as the
+                    # not-found dep.
+                    cached_dep = trial_dep
+        # There's a subproject fallback specified for this not-found dependency
+        # which might provide it, so we must check it.
+        if cached_dep and not cached_dep.found() and 'fallback' in kwargs:
+            return identifier, None
+        # Either no cached deps matched the dep we're looking for, or some
+        # not-found cached dep matched and there is no fallback specified.
+        # Either way, we're done.
+        return identifier, cached_dep
+
+    def func_dependency(self, node, args, kwargs):
+        self.validate_arguments(args, 1, [str])
+        name = args[0]
+        if '<' in name or '>' in name or '=' in name:
+            raise InvalidArguments('Characters <, > and = are forbidden in dependency names. To specify'
+                                   'version\n requirements use the \'version\' keyword argument instead.')
+        identifier, cached_dep = self._find_cached_dep(name, kwargs)
 
         if cached_dep:
+            if kwargs.get('required', True) and not cached_dep.found():
+                m = 'Dependency {!r} was already checked and was not found'
+                raise DependencyException(m.format(name))
             dep = cached_dep
         else:
             # We need to actually search for this dep
             exception = None
             dep = None
-            # If the fallback has already been configured (possibly by a higher level project)
-            # try to use it before using the native version
+            # If the dependency has already been configured, possibly by
+            # a higher level project, try to use it first.
             if 'fallback' in kwargs:
                 dirname, varname = self.get_subproject_infos(kwargs)
                 if dirname in self.subprojects:
+                    subproject = self.subprojects[dirname]
                     try:
-                        dep = self.subprojects[dirname].get_variable_method([varname], {})
-                        dep = dep.held_object
+                        # Never add fallback deps to self.coredata.deps
+                        return subproject.get_variable_method([varname], {})
                     except KeyError:
                         pass
 
+            # Search for it outside the project
             if not dep:
                 try:
                     dep = dependencies.find_external_dependency(name, self.environment, kwargs)
-                except dependencies.DependencyException as e:
+                except DependencyException as e:
                     exception = e
                     pass
 
+            # Search inside the projects list
             if not dep or not dep.found():
                 if 'fallback' in kwargs:
                     fallback_dep = self.dependency_fallback(name, kwargs)
                     if fallback_dep:
+                        # Never add fallback deps to self.coredata.deps since we
+                        # cannot cache them. They must always be evaluated else
+                        # we won't actually read all the build files.
                         return fallback_dep
-
                 if not dep:
                     raise exception