Skip to content

Commit

Permalink
dependencies: allow get_variable to define multiple pkgconfig defines
Browse files Browse the repository at this point in the history
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 containers/bubblewrap#609
  • Loading branch information
eli-schwartz committed Nov 14, 2023
1 parent dac3f26 commit 398c4b2
Show file tree
Hide file tree
Showing 8 changed files with 48 additions and 9 deletions.
10 changes: 10 additions & 0 deletions docs/markdown/snippets/pkgconfig_define_variable.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 8 additions & 0 deletions docs/yaml/objects/dep.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions mesonbuild/dependencies/pkgconfig.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion mesonbuild/dependencies/scalapack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
7 changes: 7 additions & 0 deletions mesonbuild/interpreter/interpreterobjects.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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,
Expand Down
15 changes: 10 additions & 5 deletions mesonbuild/interpreter/type_checking.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]]


Expand Down Expand Up @@ -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,
)
2 changes: 1 addition & 1 deletion mesonbuild/mdevenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions test cases/linuxlike/1 pkg-config/meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 398c4b2

Please sign in to comment.