From 8cf29bd288cb67008a42a5c9503042f975c04a43 Mon Sep 17 00:00:00 2001 From: Nirbheek Chauhan Date: Sun, 7 May 2017 11:15:47 +0530 Subject: [PATCH] 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. --- mesonbuild/coredata.py | 2 +- mesonbuild/dependencies.py | 18 +++- mesonbuild/interpreter.py | 92 ++++++++++++++----- mesonbuild/mesonlib.py | 2 +- .../5 dependency versions/meson.build | 23 ++++- 5 files changed, 103 insertions(+), 34 deletions(-) diff --git a/mesonbuild/coredata.py b/mesonbuild/coredata.py index 2dd57a9e8..6c091c870 100644 --- a/mesonbuild/coredata.py +++ b/mesonbuild/coredata.py @@ -160,7 +160,7 @@ class CoreData: self.wrap_mode = options.wrap_mode self.compilers = OrderedDict() self.cross_compilers = OrderedDict() - self.deps = {} + self.deps = OrderedDict() self.modules = {} # Only to print a warning if it changes between Meson invocations. self.pkgconf_envvar = os.environ.get('PKG_CONFIG_PATH', '') diff --git a/mesonbuild/dependencies.py b/mesonbuild/dependencies.py index a06c06912..ae04544fb 100644 --- a/mesonbuild/dependencies.py +++ b/mesonbuild/dependencies.py @@ -27,9 +27,10 @@ import subprocess import sysconfig from enum import Enum from collections import OrderedDict -from . mesonlib import MesonException, version_compare, version_compare_many, Popen_safe from . import mlog from . import mesonlib +from .mesonlib import Popen_safe, flatten +from .mesonlib import MesonException, version_compare, version_compare_many from .environment import detect_cpu_family, for_windows class DependencyException(MesonException): @@ -103,6 +104,7 @@ class InternalDependency(Dependency): def __init__(self, version, incdirs, compile_args, link_args, libraries, sources, ext_deps): super().__init__('internal', {}) self.version = version + self.is_found = True self.include_directories = incdirs self.compile_args = compile_args self.link_args = link_args @@ -1744,14 +1746,20 @@ class LLVMDependency(Dependency): def get_dep_identifier(name, kwargs, want_cross): # Need immutable objects since the identifier will be used as a dict key - identifier = (name, want_cross) + version_reqs = flatten(kwargs.get('version', [])) + if isinstance(version_reqs, list): + version_reqs = frozenset(version_reqs) + identifier = (name, version_reqs, want_cross) for key, value in kwargs.items(): - # Ignore versions, they will be handled by the caller - if key == 'version': + # 'version' is embedded above as the second element for easy access + # 'native' is handled above with `want_cross` + # 'required' is irrelevant for caching; the caller handles it separately + # 'fallback' subprojects cannot be cached -- they must be initialized + if key in ('version', 'native', 'required', 'fallback',): continue # All keyword arguments are strings, ints, or lists (or lists of lists) if isinstance(value, list): - value = frozenset(mesonlib.flatten(value)) + value = frozenset(flatten(value)) identifier += (key, value) return identifier diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py index 46ec3f38b..c1926d08e 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 diff --git a/mesonbuild/mesonlib.py b/mesonbuild/mesonlib.py index fbd732a5d..16315d8db 100644 --- a/mesonbuild/mesonlib.py +++ b/mesonbuild/mesonlib.py @@ -305,7 +305,7 @@ def version_compare(vstr1, vstr2, strict=False): return cmpop(varr1, varr2) def version_compare_many(vstr1, conditions): - if not isinstance(conditions, (list, tuple)): + if not isinstance(conditions, (list, tuple, frozenset)): conditions = [conditions] found = [] not_found = [] diff --git a/test cases/linuxlike/5 dependency versions/meson.build b/test cases/linuxlike/5 dependency versions/meson.build index cc33b2ffa..dabbe9843 100644 --- a/test cases/linuxlike/5 dependency versions/meson.build +++ b/test cases/linuxlike/5 dependency versions/meson.build @@ -21,10 +21,18 @@ if dependency('zlib', version : ['<=1.0', '>=9999', '=' + zlib.version()], requi error('zlib <=1.0 >=9999 should not have been found') endif +# Test that a versionless zlib is found after not finding an optional zlib dep with version reqs +zlibopt = dependency('zlib', required : false) +assert(zlibopt.found() == true, 'zlib not found') + # Test https://github.com/mesonbuild/meson/pull/610 dependency('somebrokenlib', version : '>=2.0', required : false) dependency('somebrokenlib', version : '>=1.0', required : false) +# Search for an external dependency that won't be found, but must later be +# found via fallbacks +somelibnotfound = dependency('somelib', required : false) +assert(somelibnotfound.found() == false, 'somelibnotfound was found?') # Find internal dependency without version somelibver = dependency('somelib', fallback : ['somelibnover', 'some_dep']) @@ -37,14 +45,25 @@ somelib = dependency('somelib', somelibver = dependency('somelib', version : '>= 0.3', fallback : ['somelibver', 'some_dep']) -# Find somelib again, but with a fallback that will fail +# Find somelib again, but with a fallback that will fail because subproject does not exist somelibfail = dependency('somelib', version : '>= 0.2', required : false, fallback : ['somelibfail', 'some_dep']) assert(somelibfail.found() == false, 'somelibfail found via wrong fallback') +# Find somelib again, but with a fallback that will fail because dependency does not exist +somefail_dep = dependency('somelib', + version : '>= 0.2', + required : false, + fallback : ['somelib', 'somefail_dep']) +assert(somefail_dep.found() == false, 'somefail_dep found via wrong fallback') -fakezlib_dep = dependency('zlib', +# Fallback should only be used if the primary was not found +fallbackzlib_dep = dependency('zlib', + fallback : ['somelib', 'fakezlib_dep']) +assert(fallbackzlib_dep.type_name() == 'pkgconfig', 'fallbackzlib_dep should be of type "pkgconfig", not ' + fallbackzlib_dep.type_name()) +# Check that the above dependency was not found because it wasn't checked, not because the fallback didn't work +fakezlib_dep = dependency('fakezlib', fallback : ['somelib', 'fakezlib_dep']) assert(fakezlib_dep.type_name() == 'internal', 'fakezlib_dep should be of type "internal", not ' + fakezlib_dep.type_name())