summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2018-08-08 22:52:19 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2018-08-09 08:00:22 -0700
commited79d6a11b4c5baf811a126406bed1c7f623a26a (patch)
treee1f5f7f9be7871d9010db381ac6b917b68376353
parent393d3c64fc6275e6e6d20269cbc155e4fdea97f8 (diff)
downloadspack-ed79d6a11b4c5baf811a126406bed1c7f623a26a.tar.gz
spack-ed79d6a11b4c5baf811a126406bed1c7f623a26a.tar.bz2
spack-ed79d6a11b4c5baf811a126406bed1c7f623a26a.tar.xz
spack-ed79d6a11b4c5baf811a126406bed1c7f623a26a.zip
bugfix: cc handles spaces in flag variables properly
- cc cleanup caused a parsing regression in flag handling - We added proper quoting to array expansions, but flag variables were never actually converted to arrays. Old code relied on this. This commit: - Adds reads to convert flags to arrays. - Makes the cc test check for improper space handling to prevent future regressions.
-rwxr-xr-xlib/spack/env/cc14
-rw-r--r--lib/spack/spack/test/cc.py158
2 files changed, 82 insertions, 90 deletions
diff --git a/lib/spack/env/cc b/lib/spack/env/cc
index 2dd82d7d73..826a0fe457 100755
--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -74,9 +74,18 @@ function die {
exit 1
}
-# read system directories into an array (delimited by :)
+# read input parameters into proper bash arrays.
+# SYSTEM_DIRS is delimited by :
IFS=':' read -ra SPACK_SYSTEM_DIRS <<< "${SPACK_SYSTEM_DIRS}"
+# SPACK_<LANG>FLAGS and SPACK_LDLIBS are split by ' '
+IFS=' ' read -ra SPACK_FFLAGS <<< "$SPACK_FFLAGS"
+IFS=' ' read -ra SPACK_CPPFLAGS <<< "$SPACK_CPPFLAGS"
+IFS=' ' read -ra SPACK_CFLAGS <<< "$SPACK_CFLAGS"
+IFS=' ' read -ra SPACK_CXXFLAGS <<< "$SPACK_CXXFLAGS"
+IFS=' ' read -ra SPACK_LDFLAGS <<< "$SPACK_LDFLAGS"
+IFS=' ' read -ra SPACK_LDLIBS <<< "$SPACK_LDLIBS"
+
# test whether a path is a system directory
function system_dir {
path="$1"
@@ -524,7 +533,8 @@ fi
# dump the full command if the caller supplies SPACK_TEST_COMMAND=dump-args
if [[ $SPACK_TEST_COMMAND == dump-args ]]; then
- echo "${full_command[@]}"
+ IFS="
+" && echo "${full_command[*]}"
exit
elif [[ -n $SPACK_TEST_COMMAND ]]; then
die "ERROR: Unknown test command"
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
index 1e0ef31070..c0a4cd1eed 100644
--- a/lib/spack/spack/test/cc.py
+++ b/lib/spack/spack/test/cc.py
@@ -178,77 +178,59 @@ def dep4(tmpdir_factory):
pytestmark = pytest.mark.usefixtures('wrapper_environment')
-def check_cc(command, args, expected):
- with set_env(SPACK_TEST_COMMAND=command):
- assert expected == cc(*args, output=str).strip().split()
+def check_args(cc, args, expected):
+ """Check output arguments that cc produces when called with args.
-
-def check_cxx(command, args, expected):
- with set_env(SPACK_TEST_COMMAND=command):
- assert expected == cxx(*args, output=str).strip().split()
-
-
-def check_fc(command, args, expected):
- with set_env(SPACK_TEST_COMMAND=command):
- assert expected == fc(*args, output=str).strip().split()
-
-
-def check_ld(command, args, expected):
- with set_env(SPACK_TEST_COMMAND=command):
- assert expected == ld(*args, output=str).strip().split()
+ This assumes that cc will print debug command output with one element
+ per line, so that we see whether arguments that should (or shouldn't)
+ contain spaces are parsed correctly.
+ """
+ with set_env(SPACK_TEST_COMMAND='dump-args'):
+ assert expected == cc(*args, output=str).strip().split('\n')
-def check_cpp(command, args, expected):
- with set_env(SPACK_TEST_COMMAND=command):
- assert expected == cpp(*args, output=str).strip().split()
+def dump_mode(cc, args):
+ """Make cc dump the mode it detects, and return it."""
+ with set_env(SPACK_TEST_COMMAND='dump-mode'):
+ return cc(*args, output=str).strip()
def test_vcheck_mode():
- check_cc(
- 'dump-mode', ['-I/include', '--version'], ['vcheck'])
- check_cc(
- 'dump-mode', ['-I/include', '-V'], ['vcheck'])
- check_cc(
- 'dump-mode', ['-I/include', '-v'], ['vcheck'])
- check_cc(
- 'dump-mode', ['-I/include', '-dumpversion'], ['vcheck'])
- check_cc(
- 'dump-mode', ['-I/include', '--version', '-c'], ['vcheck'])
- check_cc(
- 'dump-mode', ['-I/include', '-V', '-o', 'output'], ['vcheck'])
+ assert dump_mode(cc, ['-I/include', '--version']) == 'vcheck'
+ assert dump_mode(cc, ['-I/include', '-V']) == 'vcheck'
+ assert dump_mode(cc, ['-I/include', '-v']) == 'vcheck'
+ assert dump_mode(cc, ['-I/include', '-dumpversion']) == 'vcheck'
+ assert dump_mode(cc, ['-I/include', '--version', '-c']) == 'vcheck'
+ assert dump_mode(cc, ['-I/include', '-V', '-o', 'output']) == 'vcheck'
def test_cpp_mode():
- check_cc('dump-mode', ['-E'], ['cpp'])
- check_cpp('dump-mode', [], ['cpp'])
+ assert dump_mode(cc, ['-E']) == 'cpp'
+ assert dump_mode(cxx, ['-E']) == 'cpp'
+ assert dump_mode(cpp, []) == 'cpp'
def test_as_mode():
- check_cc('dump-mode', ['-S'], ['as'])
+ assert dump_mode(cc, ['-S']) == 'as'
def test_ccld_mode():
- check_cc(
- 'dump-mode', [], ['ccld'])
- check_cc(
- 'dump-mode', ['foo.c', '-o', 'foo'], ['ccld'])
- check_cc(
- 'dump-mode', ['foo.c', '-o', 'foo', '-Wl,-rpath,foo'], ['ccld'])
- check_cc(
- 'dump-mode',
- ['foo.o', 'bar.o', 'baz.o', '-o', 'foo', '-Wl,-rpath,foo'], ['ccld'])
+ assert dump_mode(cc, []) == 'ccld'
+ assert dump_mode(cc, ['foo.c', '-o', 'foo']) == 'ccld'
+ assert dump_mode(cc, ['foo.c', '-o', 'foo', '-Wl,-rpath,foo']) == 'ccld'
+ assert dump_mode(cc, [
+ 'foo.o', 'bar.o', 'baz.o', '-o', 'foo', '-Wl,-rpath,foo']) == 'ccld'
def test_ld_mode():
- check_ld('dump-mode', [], ['ld'])
- check_ld(
- 'dump-mode',
- ['foo.o', 'bar.o', 'baz.o', '-o', 'foo', '-Wl,-rpath,foo'], ['ld'])
+ assert dump_mode(ld, []) == 'ld'
+ assert dump_mode(ld, [
+ 'foo.o', 'bar.o', 'baz.o', '-o', 'foo', '-Wl,-rpath,foo']) == 'ld'
def test_ld_flags(wrapper_flags):
- check_ld(
- 'dump-args', test_args,
+ check_args(
+ ld, test_args,
['ld'] +
spack_ldflags +
test_include_paths +
@@ -260,8 +242,8 @@ def test_ld_flags(wrapper_flags):
def test_cpp_flags(wrapper_flags):
- check_cpp(
- 'dump-args', test_args,
+ check_args(
+ cpp, test_args,
['cpp'] +
spack_cppflags +
test_include_paths +
@@ -270,8 +252,8 @@ def test_cpp_flags(wrapper_flags):
def test_cc_flags(wrapper_flags):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
spack_cppflags +
spack_cflags +
@@ -285,8 +267,8 @@ def test_cc_flags(wrapper_flags):
def test_cxx_flags(wrapper_flags):
- check_cxx(
- 'dump-args', test_args,
+ check_args(
+ cxx, test_args,
[real_cc] +
spack_cppflags +
spack_cxxflags +
@@ -300,8 +282,8 @@ def test_cxx_flags(wrapper_flags):
def test_fc_flags(wrapper_flags):
- check_fc(
- 'dump-args', test_args,
+ check_args(
+ fc, test_args,
[real_cc] +
spack_fflags +
spack_cppflags +
@@ -316,8 +298,8 @@ def test_fc_flags(wrapper_flags):
def test_dep_rpath():
"""Ensure RPATHs for root package are added."""
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
test_include_paths +
test_library_paths +
@@ -331,8 +313,8 @@ def test_dep_include(dep4):
with set_env(SPACK_DEPENDENCIES=dep4,
SPACK_RPATH_DEPS=dep4,
SPACK_LINK_DEPS=dep4):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
test_include_paths +
['-I' + dep4 + '/include'] +
@@ -347,8 +329,8 @@ def test_dep_lib(dep2):
with set_env(SPACK_DEPENDENCIES=dep2,
SPACK_RPATH_DEPS=dep2,
SPACK_LINK_DEPS=dep2):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
test_include_paths +
test_library_paths +
@@ -363,8 +345,8 @@ def test_dep_lib_no_rpath(dep2):
"""Ensure a single dependency link flag is added with no dep RPATH."""
with set_env(SPACK_DEPENDENCIES=dep2,
SPACK_LINK_DEPS=dep2):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
test_include_paths +
test_library_paths +
@@ -378,8 +360,8 @@ def test_dep_lib_no_lib(dep2):
"""Ensure a single dependency RPATH is added with no -L."""
with set_env(SPACK_DEPENDENCIES=dep2,
SPACK_RPATH_DEPS=dep2):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
test_include_paths +
test_library_paths +
@@ -395,8 +377,8 @@ def test_ccld_deps(dep1, dep2, dep3, dep4):
with set_env(SPACK_DEPENDENCIES=deps,
SPACK_RPATH_DEPS=deps,
SPACK_LINK_DEPS=deps):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
[real_cc] +
test_include_paths +
['-I' + dep1 + '/include',
@@ -420,8 +402,8 @@ def test_cc_deps(dep1, dep2, dep3, dep4):
with set_env(SPACK_DEPENDENCIES=deps,
SPACK_RPATH_DEPS=deps,
SPACK_LINK_DEPS=deps):
- check_cc(
- 'dump-args', ['-c'] + test_args,
+ check_args(
+ cc, ['-c'] + test_args,
[real_cc] +
test_include_paths +
['-I' + dep1 + '/include',
@@ -444,8 +426,8 @@ def test_ccld_with_system_dirs(dep1, dep2, dep3, dep4):
'-Wl,-rpath,/usr/lib64',
'-I/usr/local/include',
'-L/lib64/']
- check_cc(
- 'dump-args', sys_path_args + test_args,
+ check_args(
+ cc, sys_path_args + test_args,
[real_cc] +
test_include_paths +
['-I' + dep1 + '/include',
@@ -474,8 +456,8 @@ def test_ld_deps(dep1, dep2, dep3, dep4):
with set_env(SPACK_DEPENDENCIES=deps,
SPACK_RPATH_DEPS=deps,
SPACK_LINK_DEPS=deps):
- check_ld(
- 'dump-args', test_args,
+ check_args(
+ ld, test_args,
['ld'] +
test_include_paths +
test_library_paths +
@@ -495,8 +477,8 @@ def test_ld_deps_no_rpath(dep1, dep2, dep3, dep4):
deps = ':'.join((dep1, dep2, dep3, dep4))
with set_env(SPACK_DEPENDENCIES=deps,
SPACK_LINK_DEPS=deps):
- check_ld(
- 'dump-args', test_args,
+ check_args(
+ ld, test_args,
['ld'] +
test_include_paths +
test_library_paths +
@@ -513,8 +495,8 @@ def test_ld_deps_no_link(dep1, dep2, dep3, dep4):
deps = ':'.join((dep1, dep2, dep3, dep4))
with set_env(SPACK_DEPENDENCIES=deps,
SPACK_RPATH_DEPS=deps):
- check_ld(
- 'dump-args', test_args,
+ check_args(
+ ld, test_args,
['ld'] +
test_include_paths +
test_library_paths +
@@ -536,8 +518,8 @@ def test_ld_deps_partial(dep1):
# TODO: do we need to add RPATHs on other platforms like Linux?
# TODO: Can't we treat them the same?
os.environ['SPACK_SHORT_SPEC'] = "foo@1.2=linux-x86_64"
- check_ld(
- 'dump-args', ['-r'] + test_args,
+ check_args(
+ ld, ['-r'] + test_args,
['ld'] +
test_include_paths +
test_library_paths +
@@ -551,8 +533,8 @@ def test_ld_deps_partial(dep1):
# rpaths from the underlying command will still appear
# Spack will not add its own rpaths.
os.environ['SPACK_SHORT_SPEC'] = "foo@1.2=darwin-x86_64"
- check_ld(
- 'dump-args', ['-r'] + test_args,
+ check_args(
+ ld, ['-r'] + test_args,
['ld'] +
test_include_paths +
test_library_paths +
@@ -564,8 +546,8 @@ def test_ld_deps_partial(dep1):
def test_ccache_prepend_for_cc():
with set_env(SPACK_CCACHE_BINARY='ccache'):
- check_cc(
- 'dump-args', test_args,
+ check_args(
+ cc, test_args,
['ccache'] + # ccache prepended in cc mode
[real_cc] +
test_include_paths +
@@ -576,8 +558,8 @@ def test_ccache_prepend_for_cc():
def test_no_ccache_prepend_for_fc():
- check_fc(
- 'dump-args', test_args,
+ check_args(
+ fc, test_args,
# no ccache for Fortran
[real_cc] +
test_include_paths +