dependencies: tighten type checking and fix cmake API violation for get_variable

dep.get_variable() only supports string values for pkg-config and
config-tool, because those interfaces use text communication, and
internal variables (from declare_dependency) operate the same way.

CMake had an oddity, where get_variable doesn't document that it allows
list values but apparently it miiiiiight work? Actually getting that
kind of result would be dangerously inconsistent though. Also, CMake
does not support lists so it's a lie. Strings that are *treated* as
lists with `;` splitting don't count...

We could do two things here:

- raise an error
- treat it as a string and return a string

It's not clear what the use case of get_variable() on a maybe-list is,
and should probably be a hard error. But that's controversial, so
instead we just return the original `;`-delimited string. It is probably
the wrong thing, but users are welcome to cope with that somehow on
their own.
pull/10039/head
Eli Schwartz 3 years ago
parent c649a2b8c5
commit b55349c2e9
No known key found for this signature in database
GPG Key ID: CEB167EFB5722BD6
  1. 14
      mesonbuild/dependencies/base.py
  2. 16
      mesonbuild/dependencies/cmake.py
  3. 2
      mesonbuild/dependencies/configtool.py
  4. 2
      mesonbuild/dependencies/pkgconfig.py
  5. 2
      mesonbuild/interpreter/interpreterobjects.py
  6. 6
      test cases/unit/63 cmake parser/meson.build

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

@ -627,17 +627,23 @@ class CMakeDependency(ExternalDependency):
def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None, def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
configtool: T.Optional[str] = None, internal: T.Optional[str] = None, configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
default_value: 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: if cmake and self.traceparser is not None:
try: try:
v = self.traceparser.vars[cmake] v = self.traceparser.vars[cmake]
except KeyError: except KeyError:
pass pass
else: else:
if len(v) == 1: # CMake does NOT have a list datatype. We have no idea whether
return v[0] # anything is a string or a string-separated-by-; Internally,
elif v: # we treat them as the latter and represent everything as a
return v # 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: if default_value is not None:
return default_value return default_value
raise DependencyException(f'Could not get cmake variable and no default provided for {self!r}') raise DependencyException(f'Could not get cmake variable and no default provided for {self!r}')

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

@ -478,7 +478,7 @@ class PkgConfigDependency(ExternalDependency):
def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None, def get_variable(self, *, cmake: T.Optional[str] = None, pkgconfig: T.Optional[str] = None,
configtool: T.Optional[str] = None, internal: T.Optional[str] = None, configtool: T.Optional[str] = None, internal: T.Optional[str] = None,
default_value: 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: if pkgconfig:
try: try:
return self.get_pkgconfig_variable(pkgconfig, pkgconfig_define or [], default_value) return self.get_pkgconfig_variable(pkgconfig, pkgconfig_define or [], default_value)

@ -479,7 +479,7 @@ class DependencyHolder(ObjectHolder[Dependency]):
KwargInfo('default_value', (str, NoneType)), KwargInfo('default_value', (str, NoneType)),
KwargInfo('pkgconfig_define', ContainerTypeInfo(list, str, pairs=True), default=[], listify=True), 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] default_varname = args[0]
if default_varname is not None: if default_varname is not None:
FeatureNew('Positional argument to dependency.get_variable()', '0.58.0').use(self.subproject, self.current_node) FeatureNew('Positional argument to dependency.get_variable()', '0.58.0').use(self.subproject, self.current_node)

@ -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 : '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_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_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_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_WS') == 'foo;foo bar;bar', 'set(CACHED STRING[]) with spaces is incorrect')
# We don't support this, so it should be unset. # 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')

Loading…
Cancel
Save