aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJussi Pakkanen <jpakkane@gmail.com>2019-06-05 22:28:30 +0300
committerGitHub <noreply@github.com>2019-06-05 22:28:30 +0300
commit906aa4cb540ce283e3562df568405ba70d179835 (patch)
treecea906f3f99fcaee11d52618680f97897d35a14a
parent94c6da33a760ecadc28ba4b54a703338038a580d (diff)
parent31ab9627533e1ae0a024c36226f703ac9ceab0a2 (diff)
downloadmeson-906aa4cb540ce283e3562df568405ba70d179835.zip
meson-906aa4cb540ce283e3562df568405ba70d179835.tar.gz
meson-906aa4cb540ce283e3562df568405ba70d179835.tar.bz2
Merge pull request #5436 from dcbaker/cmake-lexer-spaces-in-args
Cmake lexer spaces in args
-rw-r--r--mesonbuild/dependencies/base.py105
-rwxr-xr-xrun_unittests.py5
-rw-r--r--test cases/unit/61 cmake parser/meson.build19
-rw-r--r--test cases/unit/61 cmake parser/prefix/lib/cmake/mesontest/mesontest-config.cmake63
4 files changed, 158 insertions, 34 deletions
diff --git a/mesonbuild/dependencies/base.py b/mesonbuild/dependencies/base.py
index 1ccbf6f..21cd821 100644
--- a/mesonbuild/dependencies/base.py
+++ b/mesonbuild/dependencies/base.py
@@ -1546,13 +1546,27 @@ class CMakeDependency(ExternalDependency):
return True
return False
- def _cmake_set(self, tline: CMakeTraceLine):
+ def _cmake_set(self, tline: CMakeTraceLine) -> None:
+ """Handler for the CMake set() function in all variaties.
+
+ comes in three flavors:
+ set(<var> <value> [PARENT_SCOPE])
+ set(<var> <value> CACHE <type> <docstring> [FORCE])
+ set(ENV{<var>} <value>)
+
+ We don't support the ENV variant, and any uses of it will be ignored
+ silently. the other two variates are supported, with some caveats:
+ - we don't properly handle scoping, so calls to set() inside a
+ function without PARENT_SCOPE set could incorrectly shadow the
+ outer scope.
+ - We don't honor the type of CACHE arguments
+ """
# DOC: https://cmake.org/cmake/help/latest/command/set.html
# 1st remove PARENT_SCOPE and CACHE from args
args = []
for i in tline.args:
- if i == 'PARENT_SCOPE' or len(i) == 0:
+ if not i or i == 'PARENT_SCOPE':
continue
# Discard everything after the CACHE keyword
@@ -1564,13 +1578,19 @@ class CMakeDependency(ExternalDependency):
if len(args) < 1:
raise self._gen_exception('CMake: set() requires at least one argument\n{}'.format(tline))
- if len(args) == 1:
+ # Now that we've removed extra arguments all that should be left is the
+ # variable identifier and the value, join the value back together to
+ # ensure spaces in the value are correctly handled. This assumes that
+ # variable names don't have spaces. Please don't do that...
+ identifier = args.pop(0)
+ value = ' '.join(args)
+
+ if not value:
# Same as unset
- if args[0] in self.vars:
- del self.vars[args[0]]
+ if identifier in self.vars:
+ del self.vars[identifier]
else:
- values = list(itertools.chain(*map(lambda x: x.split(';'), args[1:])))
- self.vars[args[0]] = values
+ self.vars[identifier] = value.split(';')
def _cmake_unset(self, tline: CMakeTraceLine):
# DOC: https://cmake.org/cmake/help/latest/command/unset.html
@@ -1619,7 +1639,7 @@ class CMakeDependency(ExternalDependency):
self.targets[tline.args[0]] = CMakeTarget(tline.args[0], 'CUSTOM', {})
- def _cmake_set_property(self, tline: CMakeTraceLine):
+ def _cmake_set_property(self, tline: CMakeTraceLine) -> None:
# DOC: https://cmake.org/cmake/help/latest/command/set_property.html
args = list(tline.args)
@@ -1629,8 +1649,10 @@ class CMakeDependency(ExternalDependency):
append = False
targets = []
- while len(args) > 0:
+ while args:
curr = args.pop(0)
+ # XXX: APPEND_STRING is specifically *not* supposed to create a
+ # list, is treating them as aliases really okay?
if curr == 'APPEND' or curr == 'APPEND_STRING':
append = True
continue
@@ -1640,60 +1662,75 @@ class CMakeDependency(ExternalDependency):
targets.append(curr)
+ if not args:
+ raise self._gen_exception('CMake: set_property() faild to parse argument list\n{}'.format(tline))
+
if len(args) == 1:
# Tries to set property to nothing so nothing has to be done
return
- if len(args) < 2:
- raise self._gen_exception('CMake: set_property() faild to parse argument list\n{}'.format(tline))
-
- propName = args[0]
- propVal = list(itertools.chain(*map(lambda x: x.split(';'), args[1:])))
- propVal = list(filter(lambda x: len(x) > 0, propVal))
-
- if len(propVal) == 0:
+ identifier = args.pop(0)
+ value = ' '.join(args).split(';')
+ if not value:
return
for i in targets:
if i not in self.targets:
raise self._gen_exception('CMake: set_property() TARGET {} not found\n{}'.format(i, tline))
- if propName not in self.targets[i].properies:
- self.targets[i].properies[propName] = []
+ if identifier not in self.targets[i].properies:
+ self.targets[i].properies[identifier] = []
if append:
- self.targets[i].properies[propName] += propVal
+ self.targets[i].properies[identifier] += value
else:
- self.targets[i].properies[propName] = propVal
+ self.targets[i].properies[identifier] = value
- def _cmake_set_target_properties(self, tline: CMakeTraceLine):
+ def _cmake_set_target_properties(self, tline: CMakeTraceLine) -> None:
# DOC: https://cmake.org/cmake/help/latest/command/set_target_properties.html
args = list(tline.args)
targets = []
- while len(args) > 0:
+ while args:
curr = args.pop(0)
if curr == 'PROPERTIES':
break
targets.append(curr)
- if (len(args) % 2) != 0:
- raise self._gen_exception('CMake: set_target_properties() uneven number of property arguments\n{}'.format(tline))
-
- while len(args) > 0:
- propName = args.pop(0)
- propVal = args.pop(0).split(';')
- propVal = list(filter(lambda x: len(x) > 0, propVal))
-
- if len(propVal) == 0:
- continue
+ # Now we need to try to reconsitute the original quoted format of the
+ # arguments, as a property value could have spaces in it. Unlike
+ # set_property() this is not context free. There are two approaches I
+ # can think of, both have drawbacks:
+ #
+ # 1. Assume that the property will be capitalized, this is convention
+ # but cmake doesn't require it.
+ # 2. Maintain a copy of the list here: https://cmake.org/cmake/help/latest/manual/cmake-properties.7.html#target-properties
+ #
+ # Neither of these is awesome for obvious reasons. I'm going to try
+ # option 1 first and fall back to 2, as 1 requires less code and less
+ # synchroniztion for cmake changes.
+
+ arglist = [] # type: typing.List[typing.Tuple[str, typing.List[str]]]
+ name = args.pop(0)
+ values = []
+ for a in args:
+ if a.isupper():
+ if values:
+ arglist.append((name, ' '.join(values).split(';')))
+ name = a
+ values = []
+ else:
+ values.append(a)
+ if values:
+ arglist.append((name, ' '.join(values).split(';')))
+ for name, value in arglist:
for i in targets:
if i not in self.targets:
raise self._gen_exception('CMake: set_target_properties() TARGET {} not found\n{}'.format(i, tline))
- self.targets[i].properies[propName] = propVal
+ self.targets[i].properies[name] = value
def _lex_trace(self, trace):
# The trace format is: '<file>(<line>): <func>(<args -- can contain \n> )\n'
diff --git a/run_unittests.py b/run_unittests.py
index b477aa3..b8f0bf2 100755
--- a/run_unittests.py
+++ b/run_unittests.py
@@ -3681,6 +3681,11 @@ recommended as it is not supported on some platforms''')
testdir = os.path.join(self.unit_test_dir, '60 cmake_prefix_path')
self.init(testdir, extra_args=['-Dcmake_prefix_path=' + os.path.join(testdir, 'prefix')])
+ @skip_if_no_cmake
+ def test_cmake_parser(self):
+ testdir = os.path.join(self.unit_test_dir, '61 cmake parser')
+ self.init(testdir, extra_args=['-Dcmake_prefix_path=' + os.path.join(testdir, 'prefix')])
+
class FailureTests(BasePlatformTests):
'''
Tests that test failure conditions. Build files here should be dynamically
diff --git a/test cases/unit/61 cmake parser/meson.build b/test cases/unit/61 cmake parser/meson.build
new file mode 100644
index 0000000..62b9124
--- /dev/null
+++ b/test cases/unit/61 cmake parser/meson.build
@@ -0,0 +1,19 @@
+project('61 cmake parser')
+
+dep = dependency('mesontest')
+
+# Test a bunch of variations of the set() command
+assert(dep.get_variable(cmake : 'VAR_WITHOUT_SPACES') == 'NoSpaces', 'set() without spaces incorrect')
+assert(dep.get_variable(cmake : 'VAR_WITH_SPACES') == 'With Spaces', 'set() with spaces incorrect')
+
+assert(dep.get_variable(cmake : 'VAR_WITHOUT_SPACES_PS') == 'NoSpaces', 'set(PARENT_SCOPE) without spaces incorrect')
+assert(dep.get_variable(cmake : 'VAR_WITH_SPACES_PS') == 'With Spaces', 'set(PARENT_SCOPE) with spaces 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_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')
+
+# We don't suppor this, so it should be unset.
+assert(dep.get_variable(cmake : 'ENV{var}', default_value : 'sentinal') == 'sentinal', 'set(ENV) should be ignored') \ No newline at end of file
diff --git a/test cases/unit/61 cmake parser/prefix/lib/cmake/mesontest/mesontest-config.cmake b/test cases/unit/61 cmake parser/prefix/lib/cmake/mesontest/mesontest-config.cmake
new file mode 100644
index 0000000..7474eb7
--- /dev/null
+++ b/test cases/unit/61 cmake parser/prefix/lib/cmake/mesontest/mesontest-config.cmake
@@ -0,0 +1,63 @@
+# This should be enough to satisfy the basic parser
+set(MESONTEST_VERSION "1.2.3")
+set(MESONTEST_LIBRARIES "foo.so")
+set(MESONTEST_INCLUDE_DIR "")
+set(MESONTEST_FOUND "TRUE")
+
+## Tests for set() in its various forms
+
+# Basic usage
+set(VAR_WITHOUT_SPACES "NoSpaces")
+set(VAR_WITH_SPACES "With Spaces")
+
+# test of PARENT_SCOPE, requires a function to have a parent scope obviously...
+function(foo)
+ set(VAR_WITHOUT_SPACES_PS "NoSpaces" PARENT_SCOPE)
+ set(VAR_WITH_SPACES_PS "With Spaces" PARENT_SCOPE)
+endfunction(foo)
+foo()
+
+# Using set() to unset values
+set(VAR_THAT_IS_UNSET "foo")
+set(VAR_THAT_IS_UNSET)
+
+# The more advanced form that uses CACHE
+# XXX: Why don't we read the type and use that instead of always treat things as strings?
+set(CACHED_STRING_NS "foo" CACHE STRING "docstring")
+set(CACHED_STRING_WS "foo bar" CACHE STRING "docstring")
+set(CACHED_STRING_ARRAY_NS "foo;bar" CACHE STRING "doc string")
+set(CACHED_STRING_ARRAY_WS "foo;foo bar;bar" CACHE STRING "stuff" FORCE)
+
+set(CACHED_BOOL ON CACHE BOOL "docstring")
+
+set(CACHED_PATH_NS "foo/bar" CACHE PATH "docstring")
+set(CACHED_PATH_WS "foo bar/fin" CACHE PATH "docstring")
+
+set(CACHED_FILEPATH_NS "foo/bar.txt" CACHE FILEPATH "docstring")
+set(CACHED_FILEPATH_WS "foo bar/fin.txt" CACHE FILEPATH "docstring")
+
+# Set ENV, we don't support this so it shouldn't be showing up
+set(ENV{var}, "foo")
+
+
+## Tests for set_properties()
+# We need something to attach properties too
+add_custom_target(MESONTEST_FOO ALL)
+
+set_property(TARGET MESONTEST_FOO PROPERTY FOLDER "value")
+set_property(TARGET MESONTEST_FOO APPEND PROPERTY FOLDER "name")
+set_property(TARGET MESONTEST_FOO PROPERTY FOLDER "value")
+set_property(TARGET MESONTEST_FOO APPEND_STRING PROPERTY FOLDER "name")
+
+set_property(TARGET MESONTEST_FOO PROPERTY FOLDER "value space")
+set_property(TARGET MESONTEST_FOO PROPERTY FOLDER "value space")
+set_property(TARGET MESONTEST_FOO APPEND PROPERTY FOLDER "name space")
+set_property(TARGET MESONTEST_FOO PROPERTY FOLDER "value space")
+set_property(TARGET MESONTEST_FOO APPEND_STRING PROPERTY FOLDER "name space")
+
+## Tests for set_target_properties()
+set_target_properties(MESONTEST_FOO PROPERTIES FOLDER "value")
+set_target_properties(MESONTEST_FOO PROPERTIES FOLDER "value space")
+set_target_properties(MESONTEST_FOO PROPERTIES FOLDER "value" OUTPUT_NAME "another value")
+set_target_properties(MESONTEST_FOO PROPERTIES FOLDER "value space" OUTPUT_NAME "another value")
+set_target_properties(MESONTEST_FOO PROPERTIES FOLDER "value space" OUTPUT_NAME "value") \ No newline at end of file