From 398c4b22874c2f67eb2eca6371e5a94d21ab8eed Mon Sep 17 00:00:00 2001 From: Eli Schwartz Date: Mon, 13 Nov 2023 18:45:54 -0500 Subject: [PATCH] dependencies: allow get_variable to define multiple pkgconfig defines It was previously impossible to do this: ``` dep.get_pkgconfig_variable( 'foo', define_variable: ['prefix', '/usr', 'datadir', '/usr/share'], ) ``` since get_pkgconfig_variable mandated exactly two (if any) arguments. However, you could do this: ``` dep.get_variable( 'foo', pkgconfig_define: ['prefix', '/usr', 'datadir', '/usr/share'], ) ``` It would silently do the wrong thing, by defining "prefix" as `/usr=datadir=/usr/share`, which might not "matter" if only datadir was used in the "foo" variable as the unmodified value might be adequate. The actual intention of anyone writing such a meson.build is that they aren't sure whether the .pc file uses ${prefix} or ${datadir} (or which one gets used, might have changed between versions of that .pc file, even). A recent refactor made this into a hard error, which broke some projects that were doing this and inadvertently depending on some .pc file that only used the second variable. (This was "fine" since the result was essentially meaningful, and even resulted in behavior identical to the intended behavior if both projects were installed into the same prefix -- in which case there's nothing to remap.) Re-allow this. There are two ways we could re-allow this: - ignore it with a warning - add a new feature to allow actually doing this Since the use case which triggered this bug actually has a pretty good reason to want to do this, it makes sense to add the new feature. Fixes https://bugs.gentoo.org/916576 Fixes https://github.com/containers/bubblewrap/issues/609 --- .../snippets/pkgconfig_define_variable.md | 10 ++++++++++ docs/yaml/objects/dep.yaml | 8 ++++++++ mesonbuild/dependencies/pkgconfig.py | 6 ++++-- mesonbuild/dependencies/scalapack.py | 2 +- mesonbuild/interpreter/interpreterobjects.py | 7 +++++++ mesonbuild/interpreter/type_checking.py | 15 ++++++++++----- mesonbuild/mdevenv.py | 2 +- test cases/linuxlike/1 pkg-config/meson.build | 7 +++++++ 8 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 docs/markdown/snippets/pkgconfig_define_variable.md diff --git a/docs/markdown/snippets/pkgconfig_define_variable.md b/docs/markdown/snippets/pkgconfig_define_variable.md new file mode 100644 index 000000000000..0f7082b0d094 --- /dev/null +++ b/docs/markdown/snippets/pkgconfig_define_variable.md @@ -0,0 +1,10 @@ +## pkg-config dependencies can now get a variable with multiple replacements + +When using [[dep.get_variable]] and defining a `pkgconfig_define`, it is +sometimes useful to remap multiple dependency variables. For example, if the +upstream project changed the variable name that is interpolated and it is +desirable to support both versions. + +It is now possible to pass multiple pairs of variable/value. + +The same applies to the compatibility [[dep.get_pkgconfig_variable]] method. diff --git a/docs/yaml/objects/dep.yaml b/docs/yaml/objects/dep.yaml index d847690f0dd5..52e28faca2d2 100644 --- a/docs/yaml/objects/dep.yaml +++ b/docs/yaml/objects/dep.yaml @@ -38,6 +38,8 @@ methods: variable by passing a list to this kwarg that can affect the retrieved variable: `['prefix', '/'])`. + *(Since 1.3.0)* Multiple variables can be specified in pairs. + default: type: str since: 0.45.0 @@ -174,6 +176,12 @@ methods: from the object then `default_value` is returned, if it is not set then an error is raised. + warnings: + - Before 1.3.0, specifying multiple pkgconfig_define pairs would silently + malform the results. Only the first variable would be redefined, but + its value would contain both the second variable name, as well as its + value. + optargs: varname: type: str diff --git a/mesonbuild/dependencies/pkgconfig.py b/mesonbuild/dependencies/pkgconfig.py index 93422defd67f..e86206b88c2e 100644 --- a/mesonbuild/dependencies/pkgconfig.py +++ b/mesonbuild/dependencies/pkgconfig.py @@ -139,9 +139,11 @@ def version(self, name: str) -> T.Optional[str]: @staticmethod def _define_variable_args(define_variable: PkgConfigDefineType) -> T.List[str]: + ret = [] if define_variable: - return ['--define-variable=' + '='.join(define_variable)] - return [] + for pair in define_variable: + ret.append('--define-variable=' + '='.join(pair)) + return ret @lru_cache(maxsize=None) def cflags(self, name: str, allow_system: bool = False, diff --git a/mesonbuild/dependencies/scalapack.py b/mesonbuild/dependencies/scalapack.py index 158056fa0311..a8e20f405e92 100644 --- a/mesonbuild/dependencies/scalapack.py +++ b/mesonbuild/dependencies/scalapack.py @@ -148,5 +148,5 @@ def _set_cargs(self) -> None: # gfortran doesn't appear to look in system paths for INCLUDE files, # so don't allow pkg-config to suppress -I flags for system paths allow_system = True - cflags = self.pkgconfig.cflags(self.name, allow_system, define_variable=('prefix', self.__mklroot.as_posix())) + cflags = self.pkgconfig.cflags(self.name, allow_system, define_variable=(('prefix', self.__mklroot.as_posix()),)) self.compile_args = self._convert_mingw_paths(cflags) diff --git a/mesonbuild/interpreter/interpreterobjects.py b/mesonbuild/interpreter/interpreterobjects.py index effeebb2019e..f13e3ff4ad17 100644 --- a/mesonbuild/interpreter/interpreterobjects.py +++ b/mesonbuild/interpreter/interpreterobjects.py @@ -494,6 +494,9 @@ def pkgconfig_method(self, args: T.Tuple[str], kwargs: 'kwargs.DependencyPkgConf from ..dependencies.pkgconfig import PkgConfigDependency if not isinstance(self.held_object, PkgConfigDependency): raise InvalidArguments(f'{self.held_object.get_name()!r} is not a pkgconfig dependency') + if kwargs['define_variable'] and len(kwargs['define_variable']) > 1: + FeatureNew.single_use('dependency.get_pkgconfig_variable keyword argument "define_variable" with more than one pair', + '1.3.0', self.subproject, location=self.current_node) return self.held_object.get_variable( pkgconfig=args[0], default_value=kwargs['default'], @@ -536,6 +539,10 @@ def variable_method(self, args: T.Tuple[T.Optional[str]], kwargs: 'kwargs.Depend 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) + if kwargs['pkgconfig_define'] and len(kwargs['pkgconfig_define']) > 1: + FeatureNew.single_use('dependency.get_variable keyword argument "pkgconfig_define" with more than one pair', + '1.3.0', self.subproject, 'In previous versions, this silently returned a malformed value.', + self.current_node) return self.held_object.get_variable( cmake=kwargs['cmake'] or default_varname, pkgconfig=kwargs['pkgconfig'] or default_varname, diff --git a/mesonbuild/interpreter/type_checking.py b/mesonbuild/interpreter/type_checking.py index 5212a85ba754..616f4efbb58e 100644 --- a/mesonbuild/interpreter/type_checking.py +++ b/mesonbuild/interpreter/type_checking.py @@ -4,8 +4,7 @@ """Helpers for strict type checking.""" from __future__ import annotations -import os -import re +import itertools, os, re import typing as T from .. import compilers @@ -30,7 +29,7 @@ from ..mesonlib import EnvInitValueType _FullEnvInitValueType = T.Union[EnvironmentVariables, T.List[str], T.List[T.List[str]], EnvInitValueType, str, None] - PkgConfigDefineType = T.Optional[T.Tuple[str, str]] + PkgConfigDefineType = T.Optional[T.Tuple[T.Tuple[str, str], ...]] SourcesVarargsType = T.List[T.Union[str, File, CustomTarget, CustomTargetIndex, GeneratedList, StructuredSources, ExtractedObjects, BuildTarget]] @@ -841,10 +840,16 @@ def _convert_darwin_versions(val: T.List[T.Union[str, int]]) -> T.Optional[T.Tup ) ] +def _pkgconfig_define_convertor(x: T.List[str]) -> PkgConfigDefineType: + if x: + keys = itertools.islice(x, 0, None, 2) + vals = itertools.islice(x, 1, None, 2) + return tuple(zip(keys, vals)) + return None + PKGCONFIG_DEFINE_KW: KwargInfo = KwargInfo( 'pkgconfig_define', ContainerTypeInfo(list, str, pairs=True), default=[], - validator=lambda x: 'must be of length 2 or empty' if len(x) not in {0, 2} else None, - convertor=lambda x: tuple(x) if x else None + convertor=_pkgconfig_define_convertor, ) diff --git a/mesonbuild/mdevenv.py b/mesonbuild/mdevenv.py index b60d7a5729d8..9f3d1b973d28 100644 --- a/mesonbuild/mdevenv.py +++ b/mesonbuild/mdevenv.py @@ -87,7 +87,7 @@ def bash_completion_files(b: build.Build, install_data: 'InstallData') -> T.List datadir = b.environment.coredata.get_option(OptionKey('datadir')) assert isinstance(datadir, str), 'for mypy' datadir_abs = os.path.join(prefix, datadir) - completionsdir = dep.get_variable(pkgconfig='completionsdir', pkgconfig_define=('datadir', datadir_abs)) + completionsdir = dep.get_variable(pkgconfig='completionsdir', pkgconfig_define=(('datadir', datadir_abs),)) assert isinstance(completionsdir, str), 'for mypy' completionsdir_path = Path(completionsdir) for f in install_data.data: diff --git a/test cases/linuxlike/1 pkg-config/meson.build b/test cases/linuxlike/1 pkg-config/meson.build index ca48e9bd098f..b09630a16db4 100644 --- a/test cases/linuxlike/1 pkg-config/meson.build +++ b/test cases/linuxlike/1 pkg-config/meson.build @@ -22,6 +22,13 @@ assert(dep.get_pkgconfig_variable('nonexisting') == '', 'Value of unknown variab assert(dep.get_pkgconfig_variable('nonexisting', default: 'foo') == 'foo', 'Value of unknown variable is not defaulted.') # pkg-config is able to replace variables assert(dep.get_pkgconfig_variable('prefix', define_variable: ['prefix', '/tmp']) == '/tmp', 'prefix variable has not been replaced.') +assert(dep.get_variable('prefix', pkgconfig_define: ['prefix', '/tmp']) == '/tmp', 'prefix variable has not been replaced.') +# pkg-config can replace multiple variables at once +assert(dep.get_variable('prefix', pkgconfig_define: ['prefix', '/tmp', 'libdir', '/bad/libdir']) == '/tmp', 'first variable has not been replaced.') +assert(dep.get_variable('prefix', pkgconfig_define: ['libdir', '/bad/libdir', 'prefix', '/tmp']) == '/tmp', 'second variable has not been replaced.') +assert(dep.get_pkgconfig_variable('prefix', define_variable: ['prefix', '/tmp', 'libdir', '/bad/libdir']) == '/tmp', 'first variable has not been replaced.') +assert(dep.get_pkgconfig_variable('prefix', define_variable: ['libdir', '/bad/libdir', 'prefix', '/tmp']) == '/tmp', 'second variable has not been replaced.') + # Test that dependencies of dependencies work. dep2 = declare_dependency(dependencies : dep)