intrp: Consolidate subproject dep checking and loggingnirbheek/consolidate-subproject-dep-checking

If a dep is not found on the system and a fallback is specified, we
have two cases:

1. Look for the dependency in a pre-initialized subproject
2. Initialize the subproject and look for the dependency

Both these require version comparing, ensuring the fetched variable
is a dependency, and printing a success message, erroring out, etc.

Now we share the relevant code instead of duplicating it. It already
diverged, so this is a good thing.

As a side-effect, we now log fallback dependencies in the same format
as system dependencies:

    Dependency libva found: YES
    Dependency libva found: YES (cached)

    Dependency glib-2.0 from subproject subprojects/glib found: YES
    Dependency glib-2.0 from subproject subprojects/glib found: YES (cached)

5 files changed, 102 insertions, 41 deletions

diff --git a/mesonbuild/interpreter.py b/mesonbuild/interpreter.py
index e5238a7ea..b30ba36b6 100644
--- a/mesonbuild/interpreter.py
+++ b/mesonbuild/interpreter.py
@@ -2140,8 +2140,8 @@ to directly access options of other subprojects.''')
         # Check if we've already searched for and found this dep
         if identifier in self.coredata.deps:
             cached_dep = self.coredata.deps[identifier]
-            mlog.log('Cached dependency', mlog.bold(name),
-                     'found:', mlog.green('YES'))
+            mlog.log('Dependency', mlog.bold(name),
+                     'found:', mlog.green('YES'), '(cached)')
         else:
             # Check if exactly the same dep with different version requirements
             # was found already.
@@ -2158,13 +2158,59 @@ to directly access options of other subprojects.''')
                     break
         return identifier, cached_dep
 
+    @staticmethod
+    def check_subproject_version(wanted, found):
+        if wanted == 'undefined':
+            return True
+        if found == 'undefined' or not mesonlib.version_compare(found, wanted):
+            return False
+        return True
+
+    def get_subproject_dep(self, name, dirname, varname, required):
+        try:
+            dep = self.subprojects[dirname].get_variable_method([varname], {})
+        except KeyError:
+            if required:
+                raise DependencyException('Could not find dependency {} in subproject {}'
+                                          ''.format(varname, dirname))
+            # If the dependency is not required, don't raise an exception
+            subproj_path = os.path.join(self.subproject_dir, dirname)
+            mlog.log('Dependency', mlog.bold(name), 'from subproject',
+                     mlog.bold(subproj_path), 'found:', mlog.red('NO'))
+            return None
+        if not isinstance(dep, DependencyHolder):
+            raise InvalidCode('Fetched variable {!r} in the subproject {!r} is '
+                              'not a dependency object.'.format(varname, dirname))
+        return dep
+
+    def _find_cached_fallback_dep(self, name, dirname, varname, wanted, required):
+        if dirname not in self.subprojects:
+            return False
+        dep = self.get_subproject_dep(name, dirname, varname, required)
+        if not dep:
+            return False
+        found = dep.version_method([], {})
+        if self.check_subproject_version(wanted, found):
+            subproj_path = os.path.join(self.subproject_dir, dirname)
+            mlog.log('Dependency', mlog.bold(name), 'from subproject',
+                     mlog.bold(subproj_path), 'found:', mlog.green('YES'), '(cached)')
+            return dep
+        if required:
+            raise DependencyException('Version {} of subproject dependency {} already '
+                                      'cached, requested incompatible version {} for '
+                                      'dep {}'.format(found, dirname, wanted, name))
+        return None
+
     @permittedKwargs(permitted_kwargs['dependency'])
     def func_dependency(self, node, args, kwargs):
         self.validate_arguments(args, 1, [str])
+        required = kwargs.get('required', True)
+        if not isinstance(required, bool):
+            raise DependencyException('Keyword "required" must be a boolean.')
         name = args[0]
 
         if name == '':
-            if kwargs.get('required', True):
+            if required:
                 raise InvalidArguments('Dependency is both required and not-found')
             return DependencyHolder(Dependency('not-found', {}))
 
@@ -2174,7 +2220,7 @@ to directly access options of other subprojects.''')
         identifier, cached_dep = self._find_cached_dep(name, kwargs)
 
         if cached_dep:
-            if kwargs.get('required', True) and not cached_dep.found():
+            if required and not cached_dep.found():
                 m = 'Dependency {!r} was already checked and was not found'
                 raise DependencyException(m.format(name))
             dep = cached_dep
@@ -2183,26 +2229,10 @@ to directly access options of other subprojects.''')
             # a higher level project, try to use it first.
             if 'fallback' in kwargs:
                 dirname, varname = self.get_subproject_infos(kwargs)
-                required = kwargs.get('required', True)
                 wanted = kwargs.get('version', 'undefined')
-                if not isinstance(required, bool):
-                    raise DependencyException('Keyword "required" must be a boolean.')
-                if dirname in self.subprojects:
-                    found = self.subprojects[dirname].held_object.project_version
-                    valid_version = wanted == 'undefined' or mesonlib.version_compare(found, wanted)
-                    if required and not valid_version:
-                        m = 'Version {} of {} already loaded, requested incompatible version {}'
-                        raise DependencyException(m.format(found, dirname, wanted))
-                    elif valid_version:
-                        mlog.log('Found a', mlog.green('(cached)'), 'subproject',
-                                 mlog.bold(os.path.join(self.subproject_dir, dirname)), 'for',
-                                 mlog.bold(name))
-                        subproject = self.subprojects[dirname]
-                        try:
-                            # Never add fallback deps to self.coredata.deps
-                            return subproject.get_variable_method([varname], {})
-                        except KeyError:
-                            pass
+                dep = self._find_cached_fallback_dep(name, dirname, varname, wanted, required)
+                if dep:
+                    return dep
 
             # We need to actually search for this dep
             exception = None
@@ -2292,32 +2322,21 @@ root and issuing %s.
                      mlog.bold(os.path.join(self.subproject_dir, dirname)),
                      'for the dependency', mlog.bold(name))
             return None
-        try:
-            dep = self.subprojects[dirname].get_variable_method([varname], {})
-        except KeyError:
-            if kwargs.get('required', True):
-                m = 'Fallback variable {!r} in the subproject {!r} does not exist'
-                raise DependencyException(m.format(varname, dirname))
-            # If the dependency is not required, don't raise an exception
-            mlog.log('Also couldn\'t find the dependency', mlog.bold(name),
-                     'in the fallback subproject',
-                     mlog.bold(os.path.join(self.subproject_dir, dirname)))
+        dep = self.get_subproject_dep(name, dirname, varname, kwargs.get('required', True))
+        if not dep:
             return None
-        if not isinstance(dep, DependencyHolder):
-            raise InvalidCode('Fallback variable {!r} in the subproject {!r} is '
-                              'not a dependency object.'.format(varname, dirname))
+        subproj_path = os.path.join(self.subproject_dir, dirname)
         # Check if the version of the declared dependency matches what we want
         if 'version' in kwargs:
             wanted = kwargs['version']
             found = dep.version_method([], {})
-            if found == 'undefined' or not mesonlib.version_compare(found, wanted):
-                mlog.log('Subproject', mlog.bold(dirname), 'dependency',
+            if not self.check_subproject_version(wanted, found):
+                mlog.log('Subproject', mlog.bold(subproj_path), 'dependency',
                          mlog.bold(varname), 'version is', mlog.bold(found),
                          'but', mlog.bold(wanted), 'is required.')
                 return None
-        mlog.log('Found a', mlog.green('fallback'), 'subproject',
-                 mlog.bold(os.path.join(self.subproject_dir, dirname)), 'for',
-                 mlog.bold(name))
+        mlog.log('Dependency', mlog.bold(name), 'from subproject',
+                 mlog.bold(subproj_path), 'found:', mlog.green('YES'))
         return dep
 
     @permittedKwargs(permitted_kwargs['executable'])
diff --git a/run_unittests.py b/run_unittests.py
index 184386cb7..107306b46 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -1880,6 +1880,29 @@ class FailureTests(BasePlatformTests):
             return
         raise unittest.SkipTest("objc and objcpp found, can't test detection failure")
 
+    def test_subproject_variables(self):
+        '''
+        Test that:
+        1. The correct message is outputted when a not-required dep is not
+           found and the fallback subproject is also not found.
+        2. A not-found not-required dep with a fallback subproject outputs the
+           correct message when the fallback subproject is found but the
+           variable inside it is not.
+        3. A fallback dependency is found from the subproject parsed in (2)
+        4. A not-required fallback dependency is not found because the
+           subproject failed to parse.
+        '''
+        tdir = os.path.join(self.unit_test_dir, '20 subproj dep variables')
+        out = self.init(tdir, inprocess=True)
+        self.assertRegex(out, r"Also couldn't find a fallback subproject "
+                         "in.*subprojects.*nosubproj.*for the dependency.*somedep")
+        self.assertRegex(out, r'Dependency.*somenotfounddep.*from subproject.*'
+                         'subprojects.*somesubproj.*found:.*NO')
+        self.assertRegex(out, r'Dependency.*zlibproxy.*from subproject.*'
+                         'subprojects.*somesubproj.*found:.*YES.*(cached)')
+        self.assertRegex(out, r'Also couldn\'t find a fallback subproject in '
+                         '.*subprojects.*failingsubproj.*for the dependency.*somedep')
+
 
 class WindowsTests(BasePlatformTests):
     '''
diff --git a/test cases/unit/20 subproj dep variables/meson.build b/test cases/unit/20 subproj dep variables/meson.build
new file mode 100644
index 000000000..f1622f945
--- /dev/null
+++ b/test cases/unit/20 subproj dep variables/meson.build
@@ -0,0 +1,13 @@
+project('subproj found dep not found', 'c')
+
+dependency('somedep', required : false,
+           fallback : ['nosubproj', 'dep_name'])
+
+dependency('somedep', required : false,
+           fallback : ['failingsubproj', 'dep_name'])
+
+dependency('somenotfounddep', required : false,
+           fallback : ['somesubproj', 'dep_name'])
+
+dependency('zlibproxy', required : true,
+           fallback : ['somesubproj', 'zlibproxy_dep'])
diff --git a/test cases/unit/20 subproj dep variables/subprojects/failingsubproj/meson.build b/test cases/unit/20 subproj dep variables/subprojects/failingsubproj/meson.build
new file mode 100644
index 000000000..3a84bd20c
--- /dev/null
+++ b/test cases/unit/20 subproj dep variables/subprojects/failingsubproj/meson.build
@@ -0,0 +1,3 @@
+project('failingsubproj', 'c')
+
+dep_name = declare_dependency('arg')
diff --git a/test cases/unit/20 subproj dep variables/subprojects/somesubproj/meson.build b/test cases/unit/20 subproj dep variables/subprojects/somesubproj/meson.build
new file mode 100644
index 000000000..dd65c99f7
--- /dev/null
+++ b/test cases/unit/20 subproj dep variables/subprojects/somesubproj/meson.build
@@ -0,0 +1,3 @@
+project('dep', 'c')
+
+zlibproxy_dep = declare_dependency(dependencies : dependency('zlib', required : false))