From bb5d83890d4f9a95ab7ef410221aaff0f2f76535 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Tue, 7 Aug 2018 22:58:13 -0700 Subject: cc: refactor flag adding so that it's not in reverse order - flags were prepended in reverse order to args, but this makes it hard to see what order they'll be in on the final command line. - add them in the order they'll appear to make cc easier to maintain. - simplify code for assembling the command line - fix separator used in SPACK_SYSTEM_DIRS test --- lib/spack/env/cc | 71 ++++++++++++++++++++++++++++++++-------------- lib/spack/spack/test/cc.py | 14 ++++----- 2 files changed, 56 insertions(+), 29 deletions(-) diff --git a/lib/spack/env/cc b/lib/spack/env/cc index e8a0903d2a..2dd82d7d73 100755 --- a/lib/spack/env/cc +++ b/lib/spack/env/cc @@ -80,7 +80,7 @@ IFS=':' read -ra SPACK_SYSTEM_DIRS <<< "${SPACK_SYSTEM_DIRS}" # test whether a path is a system directory function system_dir { path="$1" - for sd in ${SPACK_SYSTEM_DIRS[@]}; do + for sd in "${SPACK_SYSTEM_DIRS[@]}"; do if [ "${path}" == "${sd}" ] || [ "${path}" == "${sd}/" ]; then # success if path starts with a system prefix return 0 @@ -89,7 +89,7 @@ function system_dir { return 1 # fail if path starts no system prefix } -for param in ${parameters[@]}; do +for param in "${parameters[@]}"; do if [[ -z ${!param} ]]; then die "Spack compiler must be run from Spack! Input '$param' is missing." fi @@ -248,7 +248,6 @@ fi # Save original command for debug logging input_command="$*" -args=() # # Parse the command line arguments. @@ -277,7 +276,9 @@ libs=() other_args=() while [ -n "$1" ]; do + # an RPATH to be added after the case statement. rp="" + case "$1" in -I*) arg="${1#-I}" @@ -362,37 +363,53 @@ while [ -n "$1" ]; do done # -# Prepend flags from Spack's cppflags, cflags, cxxflags, fcflags, fflags, -# and ldflags. +# Add flags from Spack's cppflags, cflags, cxxflags, fcflags, fflags, and +# ldflags. We stick to the order that gmake puts the flags in by default. +# +# See the gmake manual on implicit rules for details: +# https://www.gnu.org/software/make/manual/html_node/Implicit-Variables.html # +flags=() -# Add ldflags +# Fortran flags come before CPPFLAGS case "$mode" in - ld|ccld) - args=(${SPACK_LDFLAGS[@]} "${args[@]}") ;; + cc|ccld) + case $lang_flags in + F) + flags=("${flags[@]}" "${SPACK_FFLAGS[@]}") ;; + esac + ;; +esac + +# C preprocessor flags come before any C/CXX flags +case "$mode" in + cpp|as|cc|ccld) + flags=("${flags[@]}" "${SPACK_CPPFLAGS[@]}") ;; esac -# Add C, C++, and Fortran flags + +# Add C and C++ flags case "$mode" in cc|ccld) case $lang_flags in C) - args=(${SPACK_CFLAGS[@]} "${args[@]}") ;; + flags=("${flags[@]}" "${SPACK_CFLAGS[@]}") ;; CXX) - args=(${SPACK_CXXFLAGS[@]} "${args[@]}") ;; - F) - args=(${SPACK_FFLAGS[@]} "${args[@]}") ;; + flags=("${flags[@]}" "${SPACK_CXXFLAGS[@]}") ;; esac ;; esac -# Add cppflags +# Linker flags case "$mode" in - cpp|as|cc|ccld) - args=(${SPACK_CPPFLAGS[@]} "${args[@]}") ;; + ld|ccld) + flags=("${flags[@]}" "${SPACK_LDFLAGS[@]}") ;; esac -# Include all -L's and prefix/whatever dirs in rpath + +# Include the package's prefix/lib[64] dirs in rpath. We don't know until +# *after* installation which one's correct, so we include both lib and +# lib64, assuming that only one will be present. case "$mode" in ld|ccld) $add_rpaths && rpaths+=("$SPACK_PREFIX/lib") @@ -400,10 +417,10 @@ case "$mode" in ;; esac -# Read spack dependencies from the path environment variable +# Read spack dependencies from the environment. This is a list of prefixes. IFS=':' read -ra deps <<< "$SPACK_DEPENDENCIES" for dep in "${deps[@]}"; do - # Append include directories + # Append include directories in any compilation mode case "$mode" in cpp|cc|as|ccld) if [[ -d $dep/include ]]; then @@ -412,7 +429,7 @@ for dep in "${deps[@]}"; do ;; esac - # Append lib/lib64 and RPATH directories + # Append lib/lib64 and RPATH directories, but only if we're linking case "$mode" in ld|ccld) if [[ -d $dep/lib ]]; then @@ -436,6 +453,7 @@ for dep in "${deps[@]}"; do esac done +# add RPATHs if we're in in any linking mode case "$mode" in ld|ccld) # Set extra RPATHs @@ -446,14 +464,23 @@ case "$mode" in done # Add SPACK_LDLIBS to args - for lib in ${SPACK_LDLIBS[@]}; do + for lib in "${SPACK_LDLIBS[@]}"; do libs+=("${lib#-l}") done ;; esac -# Put the arguments back together in one list +# +# Finally, reassemble the command line. +# + # Includes and system includes first +args=() + +# flags assembled earlier +args+=("${flags[@]}") + +# include directory search paths for dir in "${includes[@]}"; do args+=("-I$dir"); done for dir in "${system_includes[@]}"; do args+=("-I$dir"); done diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py index 517b7f1dd3..1e0ef31070 100644 --- a/lib/spack/spack/test/cc.py +++ b/lib/spack/spack/test/cc.py @@ -124,7 +124,7 @@ def wrapper_environment(): SPACK_DEBUG_LOG_ID='foo-hashabc', SPACK_COMPILER_SPEC='gcc@4.4.7', SPACK_SHORT_SPEC='foo@1.2 arch=linux-rhel6-x86_64 /hashabc', - SPACK_SYSTEM_DIRS=' '.join(system_dirs), + SPACK_SYSTEM_DIRS=':'.join(system_dirs), SPACK_CC_RPATH_ARG='-Wl,-rpath,', SPACK_CXX_RPATH_ARG='-Wl,-rpath,', SPACK_F77_RPATH_ARG='-Wl,-rpath,', @@ -180,27 +180,27 @@ pytestmark = pytest.mark.usefixtures('wrapper_environment') def check_cc(command, args, expected): with set_env(SPACK_TEST_COMMAND=command): - assert cc(*args, output=str).strip().split() == expected + assert expected == cc(*args, output=str).strip().split() def check_cxx(command, args, expected): with set_env(SPACK_TEST_COMMAND=command): - assert cxx(*args, output=str).strip().split() == expected + assert expected == cxx(*args, output=str).strip().split() def check_fc(command, args, expected): with set_env(SPACK_TEST_COMMAND=command): - assert fc(*args, output=str).strip().split() == expected + assert expected == fc(*args, output=str).strip().split() def check_ld(command, args, expected): with set_env(SPACK_TEST_COMMAND=command): - assert ld(*args, output=str).strip().split() == expected + assert expected == ld(*args, output=str).strip().split() def check_cpp(command, args, expected): with set_env(SPACK_TEST_COMMAND=command): - assert cpp(*args, output=str).strip().split() == expected + assert expected == cpp(*args, output=str).strip().split() def test_vcheck_mode(): @@ -303,8 +303,8 @@ def test_fc_flags(wrapper_flags): check_fc( 'dump-args', test_args, [real_cc] + - spack_cppflags + spack_fflags + + spack_cppflags + spack_ldflags + test_include_paths + test_library_paths + -- cgit v1.2.3-60-g2f50