Merge pull request #10039 from eli-schwartz/wayland-protocols-subproject-files

dependencies: allow get_variable to expose files from subprojects

docs/markdown/Dependencies.md

@@ -90,6 +90,28 @@ following will happen: If 'default_value' was provided that value will
 be returned, if 'default_value' was not provided then an error will be
 raised.
 
+## Dependencies that provide resource files
+
+Sometimes a dependency provides installable files which other projects then
+need to use. For example, wayland-protocols XML files.
+
+```meson
+foo_dep = dependency('foo')
+foo_datadir = foo_dep.get_variable('pkgdatadir')
+custom_target(
+    'foo-generated.c',
+    input: foo_datadir / 'prototype.xml',
+    output: 'foo-generated.c',
+    command: [generator, '@INPUT@', '@OUTPUT@']
+)
+```
+
+*Since 0.63.0* these actually work as expected, even when they come from a
+(well-formed) internal dependency. This only works when treating the files to
+be obtained as interchangeable with a system dependency -- e.g. only public
+files may be used, and leaving the directory pointed to by the dependency is
+not allowed.
+
 # Declaring your own
 
 You can declare your own dependency objects that can be used

mesonbuild/dependencies/base.py

@@ -216,7 +216,7 @@ class Dependency(HoldableObject):
     def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
                      configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
                      default_value: T.Optional[str] = None,
-                     pkgconfig_define: T.Optional[T.List[str]] = None) -> T.Union[str, T.List[str]]:
+                     pkgconfig_define: T.Optional[T.List[str]] = None) -> str:
         if default_value is not None:
             return default_value
         raise DependencyException(f'No default provided for dependency {self!r}, which is not pkg-config, cmake, or config-tool based.')
@@ -232,7 +232,7 @@ class InternalDependency(Dependency):
                  libraries: T.List[T.Union['BuildTarget', 'CustomTarget']],
                  whole_libraries: T.List[T.Union['BuildTarget', 'CustomTarget']],
                  sources: T.Sequence[T.Union['FileOrString', 'CustomTarget', StructuredSources]],
-                 ext_deps: T.List[Dependency], variables: T.Dict[str, T.Any],
+                 ext_deps: T.List[Dependency], variables: T.Dict[str, str],
                  d_module_versions: T.List[str], d_import_dirs: T.List['IncludeDirs']):
         super().__init__(DependencyTypeName('internal'), {})
         self.version = version
@@ -301,16 +301,10 @@ class InternalDependency(Dependency):
     def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
                      configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
                      default_value: T.Optional[str] = None,
-                     pkgconfig_define: T.Optional[T.List[str]] = None) -> T.Union[str, T.List[str]]:
+                     pkgconfig_define: T.Optional[T.List[str]] = None) -> str:
         val = self.variables.get(internal, default_value)
         if val is not None:
-            # TODO: Try removing this assert by better typing self.variables
-            if isinstance(val, str):
-                return val
-            if isinstance(val, list):
-                for i in val:
-                    assert isinstance(i, str)
-                return val
+            return val
         raise DependencyException(f'Could not get an internal variable and no default provided for {self!r}')
 
     def generate_link_whole_dependency(self) -> Dependency:

mesonbuild/dependencies/cmake.py

@@ -627,17 +627,23 @@ class CMakeDependency(ExternalDependency):
     def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
                      configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
                      default_value: T.Optional[str] = None,
-                     pkgconfig_define: T.Optional[T.List[str]] = None) -> T.Union[str, T.List[str]]:
+                     pkgconfig_define: T.Optional[T.List[str]] = None) -> str:
         if cmake and self.traceparser is not None:
             try:
                 v = self.traceparser.vars[cmake]
             except KeyError:
                 pass
             else:
-                if len(v) == 1:
-                    return v[0]
-                elif v:
-                    return v
+                # CMake does NOT have a list datatype. We have no idea whether
+                # anything is a string or a string-separated-by-; Internally,
+                # we treat them as the latter and represent everything as a
+                # list, because it is convenient when we are mostly handling
+                # imported targets, which have various properties that are
+                # actually lists.
+                #
+                # As a result we need to convert them back to strings when grabbing
+                # raw variables the user requested.
+                return ';'.join(v)
         if default_value is not None:
             return default_value
         raise DependencyException(f'Could not get cmake variable and no default provided for {self!r}')

mesonbuild/dependencies/configtool.py

@@ -155,7 +155,7 @@ class ConfigToolDependency(ExternalDependency):
     def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
                      configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
                      default_value: T.Optional[str] = None,
-                     pkgconfig_define: T.Optional[T.List[str]] = None) -> T.Union[str, T.List[str]]:
+                     pkgconfig_define: T.Optional[T.List[str]] = None) -> str:
         if configtool:
             # In the not required case '' (empty string) will be returned if the
             # variable is not found. Since '' is a valid value to return we

mesonbuild/dependencies/pkgconfig.py

@@ -485,7 +485,7 @@ class PkgConfigDependency(ExternalDependency):
     def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
                      configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
                      default_value: T.Optional[str] = None,
-                     pkgconfig_define: T.Optional[T.List[str]] = None) -> T.Union[str, T.List[str]]:
+                     pkgconfig_define: T.Optional[T.List[str]] = None) -> str:
         if pkgconfig:
             try:
                 return self.get_pkgconfig_variable(pkgconfig, pkgconfig_define or [], default_value)

mesonbuild/interpreter/interpreter.py

@@ -420,6 +420,7 @@ class Interpreter(InterpreterBase, HoldableObject):
             bool: P_OBJ.BooleanHolder,
             str: P_OBJ.StringHolder,
             P_OBJ.MesonVersionString: P_OBJ.MesonVersionStringHolder,
+            P_OBJ.DependencyVariableString: P_OBJ.DependencyVariableStringHolder,
 
             # Meson types
             mesonlib.File: OBJ.FileHolder,
@@ -669,6 +670,18 @@ class Interpreter(InterpreterBase, HoldableObject):
         d_module_versions = extract_as_list(kwargs, 'd_module_versions')
         d_import_dirs = self.extract_incdirs(kwargs, 'd_import_dirs')
         final_deps = []
+        srcdir = Path(self.environment.source_dir)
+        # convert variables which refer to an -uninstalled.pc style datadir
+        for k, v in variables.items():
+            try:
+                p = Path(v)
+            except ValueError:
+                continue
+            else:
+                if not self.is_subproject() and srcdir / self.subproject_dir in p.parents:
+                    continue
+                if p.is_absolute() and p.is_dir() and srcdir / self.root_subdir in p.resolve().parents:
+                    variables[k] = P_OBJ.DependencyVariableString(v)
         for d in deps:
             if not isinstance(d, (dependencies.Dependency, dependencies.ExternalLibrary, dependencies.InternalDependency)):
                 raise InterpreterException('Dependencies must be external deps')
@@ -2721,7 +2734,13 @@ external dependencies (including libraries) must go to "dependencies".''')
     @typed_pos_args('join_paths', varargs=str, min_varargs=1)
     @noKwargs
     def func_join_paths(self, node: mparser.BaseNode, args: T.Tuple[T.List[str]], kwargs: 'TYPE_kwargs') -> str:
-        return os.path.join(*args[0]).replace('\\', '/')
+        parts = args[0]
+        other = os.path.join('', *parts[1:]).replace('\\', '/')
+        ret = os.path.join(*parts).replace('\\', '/')
+        if isinstance(parts[0], P_OBJ.DependencyVariableString) and '..' not in other:
+            return P_OBJ.DependencyVariableString(ret)
+        else:
+            return ret
 
     def run(self) -> None:
         super().run()
@@ -2763,6 +2782,26 @@ Try setting b_lundef to false instead.'''.format(self.coredata.options[OptionKey
     # declare_dependency).
     def validate_within_subproject(self, subdir, fname):
         srcdir = Path(self.environment.source_dir)
+        builddir = Path(self.environment.build_dir)
+        if isinstance(fname, P_OBJ.DependencyVariableString):
+            def validate_installable_file(fpath: Path) -> bool:
+                installablefiles: T.Set[Path] = set()
+                for d in self.build.data:
+                    for s in d.sources:
+                        installablefiles.add(Path(s.absolute_path(srcdir, builddir)))
+                installabledirs = [str(Path(srcdir, s.source_subdir)) for s in self.build.install_dirs]
+                if fpath in installablefiles:
+                    return True
+                for d in installabledirs:
+                    if str(fpath).startswith(d):
+                        return True
+                return False
+
+            norm = Path(fname)
+            # variables built from a dep.get_variable are allowed to refer to
+            # subproject files, as long as they are scheduled to be installed.
+            if validate_installable_file(norm):
+                return
         norm = Path(srcdir, subdir, fname).resolve()
         if os.path.isdir(norm):
             inputtype = 'directory'

mesonbuild/interpreter/interpreterobjects.py

@@ -479,7 +479,7 @@ class DependencyHolder(ObjectHolder[Dependency]):
         KwargInfo('default_value', (str, NoneType)),
         KwargInfo('pkgconfig_define', ContainerTypeInfo(list, str, pairs=True), default=[], listify=True),
     )
-    def variable_method(self, args: T.Tuple[T.Optional[str]], kwargs: 'kwargs.DependencyGetVariable') -> T.Union[str, T.List[str]]:
+    def variable_method(self, args: T.Tuple[T.Optional[str]], kwargs: 'kwargs.DependencyGetVariable') -> str:
         default_varname = args[0]
         if default_varname is not None:
             FeatureNew('Positional argument to dependency.get_variable()', '0.58.0').use(self.subproject, self.current_node)

mesonbuild/interpreter/primitives/__init__.py

@@ -10,6 +10,8 @@ __all__ = [
     'StringHolder',
     'MesonVersionString',
     'MesonVersionStringHolder',
+    'DependencyVariableString',
+    'DependencyVariableStringHolder',
 ]
 
 from .array import ArrayHolder
@@ -17,4 +19,8 @@ from .boolean import BooleanHolder
 from .dict import DictHolder
 from .integer import IntegerHolder
 from .range import RangeHolder
-from .string import StringHolder, MesonVersionString, MesonVersionStringHolder
+from .string import (
+    StringHolder,
+    MesonVersionString, MesonVersionStringHolder,
+    DependencyVariableString, DependencyVariableStringHolder
+)

mesonbuild/interpreter/primitives/string.py

@@ -179,3 +179,18 @@ class MesonVersionStringHolder(StringHolder):
     def version_compare_method(self, args: T.Tuple[str], kwargs: TYPE_kwargs) -> bool:
         self.interpreter.tmp_meson_version = args[0]
         return version_compare(self.held_object, args[0])
+
+# These special subclasses of string exist to cover the case where a dependency
+# exports a string variable interchangeable with a system dependency. This
+# matters because a dependency can only have string-type get_variable() return
+# values. If at any time dependencies start supporting additional variable
+# types, this class could be deprecated.
+class DependencyVariableString(str):
+    pass
+
+class DependencyVariableStringHolder(StringHolder):
+    def op_div(self, other: str) -> T.Union[str, DependencyVariableString]:
+        ret = super().op_div(other)
+        if '..' in other:
+            return ret
+        return DependencyVariableString(ret)

test cases/common/251 subproject dependency variables/meson.build

@@ -0,0 +1,13 @@
+project('subproject dependency variables', 'c')
+
+subfiles_dep = subproject('subfiles').get_variable('files_dep')
+
+executable(
+    'foo',
+    join_paths(subfiles_dep.get_variable('pkgdatadir'), 'foo.c')
+)
+
+executable(
+    'foo2',
+    subfiles_dep.get_variable('pkgdatadir2') / 'foo.c'
+)

test cases/common/251 subproject dependency variables/subprojects/subfiles/meson.build

@@ -0,0 +1,26 @@
+project('dependency variable resource')
+
+files_dep = declare_dependency(
+    variables: [
+      'pkgdatadir=@0@/subdir'.format(meson.current_source_dir()),
+      'pkgdatadir2=@0@/subdir2'.format(meson.current_source_dir()),
+    ]
+)
+
+install_data('subdir/foo.c', install_dir: get_option('datadir') / 'subdir')
+install_subdir('subdir2', install_dir: get_option('datadir'))
+
+import('pkgconfig').generate(
+    name: 'depvar_resource',
+    description: 'Get a resource file from pkgconfig or a subproject',
+    version: '0.1',
+    variables: [
+      'pkgdatadir=${datadir}/subdir',
+      'pkgdatadir2=${datadir}/subdir2',
+    ],
+    uninstalled_variables: [
+      'pkgdatadir=@0@/subdir'.format(meson.current_source_dir()),
+      'pkgdatadir2=@0@/subdir2'.format(meson.current_source_dir()),
+    ],
+    dataonly: true,
+)

test cases/common/251 subproject dependency variables/subprojects/subfiles/subdir/foo.c

@@ -0,0 +1 @@
+int main(void) { return 0; }

test cases/common/251 subproject dependency variables/subprojects/subfiles/subdir2/foo.c

@@ -0,0 +1 @@
+int main(void) { return 0; }

test cases/common/251 subproject dependency variables/test.json

@@ -0,0 +1,7 @@
+{
+  "installed": [
+    { "type": "file",  "file": "usr/share/pkgconfig/depvar_resource.pc" },
+    { "type": "file",  "file": "usr/share/subdir/foo.c" },
+    { "type": "file",  "file": "usr/share/subdir2/foo.c" }
+  ]
+}

test cases/failing/123 subproject sandbox violation/meson.build

@@ -0,0 +1,34 @@
+project('subproject-sandbox-violation')
+
+sub1_d = subproject('subproj1').get_variable('d')
+sub1_mustfail = sub1_d.get_variable('dir') / '..' / 'file.txt'
+
+sub2_d = subproject('subproj2').get_variable('d')
+sub2_mustfail = sub2_d.get_variable('dir') / 'file.txt'
+
+main_d = declare_dependency(
+    variables: [
+        'dir=@0@'.format(meson.current_source_dir()),
+    ]
+)
+main_mustfail = main_d.get_variable('dir') / 'subprojects/subproj3/file.txt'
+
+if get_option('failmode') == 'parent-dir'
+    mustfail = sub1_mustfail
+elif get_option('failmode') == 'not-installed'
+    mustfail = sub2_mustfail
+elif get_option('failmode') == 'root-subdir'
+    mustfail = main_mustfail
+endif
+
+custom_target(
+    'mustfail',
+    input: mustfail,
+    output: 'file.txt',
+    command: [
+        'python3', '-c',
+        'import os; shutil.copy(sys.argv[1], sys.argv[2])',
+        '@INPUT@',
+        '@OUTPUT@'
+    ],
+)

test cases/failing/123 subproject sandbox violation/meson_options.txt

@@ -0,0 +1 @@
+option('failmode', type: 'combo', choices: ['parent-dir', 'not-installed', 'root-subdir'])

test cases/failing/123 subproject sandbox violation/subprojects/subproj1/meson.build

@@ -0,0 +1,4 @@
+project('subproj1')
+
+install_data('file.txt')
+subdir('nested')

test cases/failing/123 subproject sandbox violation/subprojects/subproj1/nested/meson.build

@@ -0,0 +1,5 @@
+d = declare_dependency(
+    variables: [
+        'dir=@0@'.format(meson.current_source_dir()),
+    ]
+)

test cases/failing/123 subproject sandbox violation/subprojects/subproj2/meson.build

@@ -0,0 +1,7 @@
+project('subproj1')
+
+d = declare_dependency(
+    variables: [
+        'dir=@0@'.format(meson.current_source_dir()),
+    ]
+)

test cases/failing/123 subproject sandbox violation/subprojects/subproj3/meson.build

@@ -0,0 +1,3 @@
+project('subproj2')
+
+install_data('file.txt')

test cases/failing/123 subproject sandbox violation/test.json

@@ -0,0 +1,16 @@
+{
+  "matrix": {
+    "options": {
+      "failmode": [
+        { "val": "not-installed" },
+        { "val": "parent-dir" },
+        { "val": "root-subdir" }
+      ]
+    }
+  },
+  "stdout": [
+    {
+      "line": "test cases/failing/123 subproject sandbox violation/meson.build:24:0: ERROR: Sandbox violation: Tried to grab file file.txt from a nested subproject."
+    }
+  ]
+}

test cases/unit/63 cmake parser/meson.build

@@ -12,8 +12,8 @@ assert(dep.get_variable(cmake : 'VAR_WITH_SPACES_PS') == 'With Spaces', 'set(PAR
 assert(dep.get_variable(cmake : 'VAR_THAT_IS_UNSET', default_value : 'sentinal') == 'sentinal', 'set() to unset is incorrect')
 assert(dep.get_variable(cmake : 'CACHED_STRING_NS') == 'foo', 'set(CACHED) without spaces is incorrect')
 assert(dep.get_variable(cmake : 'CACHED_STRING_WS') == 'foo bar', 'set(CACHED STRING) with spaces is incorrect')
-assert(dep.get_variable(cmake : 'CACHED_STRING_ARRAY_NS') == ['foo', 'bar'], 'set(CACHED STRING) without spaces is incorrect')
-assert(dep.get_variable(cmake : 'CACHED_STRING_ARRAY_WS') == ['foo', 'foo bar', 'bar'], 'set(CACHED STRING[]) with spaces is incorrect')
+assert(dep.get_variable(cmake : 'CACHED_STRING_ARRAY_NS') == 'foo;bar', 'set(CACHED STRING) without spaces is incorrect')
+assert(dep.get_variable(cmake : 'CACHED_STRING_ARRAY_WS') == 'foo;foo bar;bar', 'set(CACHED STRING[]) with spaces is incorrect')
 
 # We don't support this, so it should be unset.
-assert(dep.get_variable(cmake : 'ENV{var}', default_value : 'sentinal') == 'sentinal', 'set(ENV) should be ignored')
+assert(dep.get_variable(cmake : 'ENV{var}', default_value : 'sentinal') == 'sentinal', 'set(ENV) should be ignored')