Simplify cc: Remove old logic and add better tests.

- removed a lot of old logic that was only still needed for tests.

- Added better unit tests for dependency RPATH, -L, and -I args

- tests now check whether the compiler omits -I args in link mode.

2 files changed, 123 insertions, 142 deletions

diff --git a/lib/spack/env/cc b/lib/spack/env/cc
index 17740250d1..4217159a25 100755
--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -114,7 +114,9 @@ case "$command" in
         ;;
 esac
 
-# If any of the arguments below is present then the mode is vcheck. In vcheck mode nothing is added in terms of extra search paths or libraries
+# If any of the arguments below is present then the mode is vcheck. In
+# vcheck mode nothing is added in terms of extra search paths or
+# libraries
 if [ -z "$mode" ]; then
     for arg in "$@"; do
         if [ "$arg" = -v -o "$arg" = -V -o "$arg" = --version -o "$arg" = -dumpversion ]; then
@@ -125,7 +127,6 @@ if [ -z "$mode" ]; then
 fi
 
 # Finish setting up the mode.
-
 if [ -z "$mode" ]; then
     mode=ccld
     for arg in "$@"; do
@@ -162,127 +163,18 @@ fi
 input_command="$@"
 args=("$@")
 
-# Dump parsed values for unit testing if asked for
-if [[ -n $SPACK_TEST_COMMAND ]]; then
-
-    #
-    # Now do real parsing of the command line args, trying hard to keep
-    # non-rpath linker arguments in the proper order w.r.t. other command line
-    # arguments.  This is important for things like groups.
-    #
-    includes=()
-    libraries=()
-    libs=()
-    rpaths=()
-    other_args=()
-
-    while [ -n "$1" ]; do
-        case "$1" in
-            -I*)
-                arg="${1#-I}"
-                if [ -z "$arg" ]; then shift; arg="$1"; fi
-                includes+=("$arg")
-                ;;
-            -L*)
-                arg="${1#-L}"
-                if [ -z "$arg" ]; then shift; arg="$1"; fi
-                libraries+=("$arg")
-                ;;
-            -l*)
-                arg="${1#-l}"
-                if [ -z "$arg" ]; then shift; arg="$1"; fi
-                libs+=("$arg")
-                ;;
-            -Wl,*)
-                arg="${1#-Wl,}"
-                # TODO: Handle multiple -Wl, continuations of -Wl,-rpath
-                if [[ $arg == -rpath=* ]]; then
-                    arg="${arg#-rpath=}"
-                    for rpath in ${arg//,/ }; do
-                        rpaths+=("$rpath")
-                    done
-                elif [[ $arg == -rpath,* ]]; then
-                    arg="${arg#-rpath,}"
-                    for rpath in ${arg//,/ }; do
-                        rpaths+=("$rpath")
-                    done
-                elif [[ $arg == -rpath ]]; then
-                    shift; arg="$1"
-                    if [[ $arg != '-Wl,'* ]]; then
-                        die "-Wl,-rpath was not followed by -Wl,*"
-                    fi
-                    arg="${arg#-Wl,}"
-                    for rpath in ${arg//,/ }; do
-                        rpaths+=("$rpath")
-                    done
-                else
-                    other_args+=("-Wl,$arg")
-                fi
-                ;;
-            -Xlinker)
-                shift; arg="$1";
-                if [[ $arg = -rpath=* ]]; then
-                    rpaths+=("${arg#-rpath=}")
-                elif [[ $arg = -rpath ]]; then
-                    shift; arg="$1"
-                    if [[ $arg != -Xlinker ]]; then
-                        die "-Xlinker -rpath was not followed by -Xlinker <arg>"
-                    fi
-                    shift; arg="$1"
-                    rpaths+=("$arg")
-                else
-                    other_args+=("-Xlinker")
-                    other_args+=("$arg")
-                fi
-                ;;
-            *)
-                other_args+=("$1")
-                ;;
-        esac
-        shift
-    done
-
-    IFS=$'\n'
-    case "$SPACK_TEST_COMMAND" in
-        dump-includes)   echo "${includes[*]}";;
-        dump-libraries)  echo "${libraries[*]}";;
-        dump-libs)       echo "${libs[*]}";;
-        dump-rpaths)     echo "${rpaths[*]}";;
-        dump-other-args) echo "${other_args[*]}";;
-        dump-all)
-            echo "INCLUDES:"
-            echo "${includes[*]}"
-            echo
-            echo "LIBRARIES:"
-            echo "${libraries[*]}"
-            echo
-            echo "LIBS:"
-            echo "${libs[*]}"
-            echo
-            echo "RPATHS:"
-            echo "${rpaths[*]}"
-            echo
-            echo "ARGS:"
-            echo "${other_args[*]}"
-            ;;
-        *)
-            die "ERROR: Unknown test command"
-            ;;
-    esac
-    exit
-fi
-
 # Read spack dependencies from the path environment variable
 IFS=':' read -ra deps <<< "$SPACK_DEPENDENCIES"
 for dep in "${deps[@]}"; do
+    # Prepend include directories
     if [[ -d $dep/include ]]; then
         if [[ $mode = cpp || $mode = cc || $mode = as || $mode = ccld ]]; then
             args=("-I$dep/include" "${args[@]}")
         fi
     fi
 
+    # Prepend lib and RPATH directories
     if [[ -d $dep/lib ]]; then
-        # libraries+=("$dep/lib")
         if [[ $mode = ccld ]]; then
             args=("-L$dep/lib" "-Wl,-rpath,$dep/lib" "${args[@]}")
         elif [[ $mode = ld ]]; then
@@ -290,8 +182,8 @@ for dep in "${deps[@]}"; do
         fi
     fi
 
+    # Prepend lib64 and RPATH directories
     if [[ -d $dep/lib64 ]]; then
-        # libraries+=("$dep/lib64")
         if [[ $mode = ccld ]]; then
             args=("-L$dep/lib64" "-Wl,-rpath,$dep/lib64" "${args[@]}")
         elif [[ $mode = ld ]]; then
@@ -302,18 +194,8 @@ done
 
 # Include all -L's and prefix/whatever dirs in rpath
 if [[ $mode = ccld ]]; then
-    # for dir in "${libraries[@]}"; do
-    #     if [[ dir = $SPACK_INSTALL* ]]; then
-    #         args=("-Wl,-rpath,$dir" "${args[@]}")
-    #     fi
-    # done
     args=("-Wl,-rpath,$SPACK_PREFIX/lib" "-Wl,-rpath,$SPACK_PREFIX/lib64" "${args[@]}")
 elif [[ $mode = ld ]]; then
-    # for dir in "${libraries[@]}"; do
-    #     if [[ dir = $SPACK_INSTALL* ]]; then
-    #         args=("-rpath" "$dir" "${args[@]}")
-    #     fi
-    # done
     args=("-rpath" "$SPACK_PREFIX/lib" "-rpath" "$SPACK_PREFIX/lib64" "${args[@]}")
 fi
 
@@ -345,6 +227,14 @@ export PATH
 
 full_command=("$command" "${args[@]}")
 
+# In test command mode, write out full command for Spack tests.
+if [[ $SPACK_TEST_COMMAND = dump-args ]]; then
+    echo "${full_command[@]}"
+    exit
+elif [[ -n $SPACK_TEST_COMMAND ]]; then
+    die "ERROR: Unknown test command"
+fi
+
 #
 # Write the input and output commands to debug logs if it's asked for.
 #
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
index f3f6d4a22e..0b1aeb2a8f 100644
--- a/lib/spack/spack/test/cc.py
+++ b/lib/spack/spack/test/cc.py
@@ -28,6 +28,8 @@ arguments correctly.
 """
 import os
 import unittest
+import tempfile
+import shutil
 
 from llnl.util.filesystem import *
 import spack
@@ -55,13 +57,40 @@ class CompilerTest(unittest.TestCase):
         self.ld = Executable(join_path(spack.build_env_path, "ld"))
         self.cpp = Executable(join_path(spack.build_env_path, "cpp"))
 
-        os.environ['SPACK_CC'] = "/bin/mycc"
-        os.environ['SPACK_PREFIX'] = "/usr"
+        self.realcc = "/bin/mycc"
+        self.prefix = "/spack-test-prefix"
+
+        os.environ['SPACK_CC'] = self.realcc
+        os.environ['SPACK_PREFIX'] = self.prefix
         os.environ['SPACK_ENV_PATH']="test"
         os.environ['SPACK_DEBUG_LOG_DIR'] = "."
         os.environ['SPACK_COMPILER_SPEC'] = "gcc@4.4.7"
         os.environ['SPACK_SHORT_SPEC'] = "foo@1.2"
 
+        # Make some fake dependencies
+        self.tmp_deps = tempfile.mkdtemp()
+        self.dep1 = join_path(self.tmp_deps, 'dep1')
+        self.dep2 = join_path(self.tmp_deps, 'dep2')
+        self.dep3 = join_path(self.tmp_deps, 'dep3')
+        self.dep4 = join_path(self.tmp_deps, 'dep4')
+
+        mkdirp(join_path(self.dep1, 'include'))
+        mkdirp(join_path(self.dep1, 'lib'))
+
+        mkdirp(join_path(self.dep2, 'lib64'))
+
+        mkdirp(join_path(self.dep3, 'include'))
+        mkdirp(join_path(self.dep3, 'lib64'))
+
+        mkdirp(join_path(self.dep4, 'include'))
+
+        if 'SPACK_DEPENDENCIES' in os.environ:
+            del os.environ['SPACK_DEPENDENCIES']
+
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp_deps, True)
+
 
     def check_cc(self, command, args, expected):
         os.environ['SPACK_TEST_COMMAND'] = command
@@ -92,6 +121,10 @@ class CompilerTest(unittest.TestCase):
         self.check_cpp('dump-mode', [], "cpp")
 
 
+    def test_as_mode(self):
+        self.check_cc('dump-mode', ['-S'], "as")
+
+
     def test_ccld_mode(self):
         self.check_cc('dump-mode', [], "ccld")
         self.check_cc('dump-mode', ['foo.c', '-o', 'foo'], "ccld")
@@ -104,27 +137,85 @@ class CompilerTest(unittest.TestCase):
         self.check_ld('dump-mode', ['foo.o', 'bar.o', 'baz.o', '-o', 'foo', '-Wl,-rpath,foo'], "ld")
 
 
-    def test_includes(self):
-        self.check_cc('dump-includes', test_command,
-                      "\n".join(["/test/include", "/other/include"]))
+    def test_dep_rpath(self):
+        """Ensure RPATHs for root package are added."""
+        self.check_cc('dump-args', test_command,
+                      self.realcc + ' ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib64 ' +
+                      ' '.join(test_command))
+
+
+    def test_dep_include(self):
+        """Ensure a single dependency include directory is added."""
+        os.environ['SPACK_DEPENDENCIES'] = self.dep4
+        self.check_cc('dump-args', test_command,
+                      self.realcc + ' ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib64 ' +
+                      '-I' + self.dep4 + '/include ' +
+                      ' '.join(test_command))
+
+
+    def test_dep_lib(self):
+        """Ensure a single dependency RPATH is added."""
+        os.environ['SPACK_DEPENDENCIES'] = self.dep2
+        self.check_cc('dump-args', test_command,
+                      self.realcc + ' ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib64 ' +
+                      '-L' + self.dep2 + '/lib64 ' +
+                      '-Wl,-rpath,' + self.dep2 + '/lib64 ' +
+                      ' '.join(test_command))
+
+
+    def test_all_deps(self):
+        """Ensure includes and RPATHs for all deps are added. """
+        os.environ['SPACK_DEPENDENCIES'] = ':'.join([
+            self.dep1, self.dep2, self.dep3, self.dep4])
+
+        # This is probably more constrained than it needs to be; it
+        # checks order within prepended args and doesn't strictly have
+        # to.  We could loosen that if it becomes necessary
+        self.check_cc('dump-args', test_command,
+                      self.realcc + ' ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib ' +
+                      '-Wl,-rpath,' + self.prefix + '/lib64 ' +
+
+                      '-I' + self.dep4 + '/include ' +
+
+                      '-L' + self.dep3 + '/lib64 ' +
+                      '-Wl,-rpath,' + self.dep3 + '/lib64 ' +
+                      '-I' + self.dep3 + '/include ' +
+
+                      '-L' + self.dep2 + '/lib64 ' +
+                      '-Wl,-rpath,' + self.dep2 + '/lib64 ' +
+
+                      '-L' + self.dep1 + '/lib ' +
+                      '-Wl,-rpath,' + self.dep1 + '/lib ' +
+                      '-I' + self.dep1 + '/include ' +
+
+                      ' '.join(test_command))
 
 
-    def test_libraries(self):
-        self.check_cc('dump-libraries', test_command,
-                      "\n".join(["/test/lib", "/other/lib"]))
+    def test_ld_deps(self):
+        """Ensure no (extra) -I args or -Wl, are passed in ld mode."""
+        os.environ['SPACK_DEPENDENCIES'] = ':'.join([
+            self.dep1, self.dep2, self.dep3, self.dep4])
 
+        self.check_ld('dump-args', test_command,
+                      'ld ' +
+                      '-rpath ' + self.prefix + '/lib ' +
+                      '-rpath ' + self.prefix + '/lib64 ' +
 
-    def test_libs(self):
-        self.check_cc('dump-libs', test_command,
-                      "\n".join(["lib1", "lib2", "lib3", "lib4"]))
+                      '-L' + self.dep3 + '/lib64 ' +
+                      '-rpath ' + self.dep3 + '/lib64 ' +
 
+                      '-L' + self.dep2 + '/lib64 ' +
+                      '-rpath ' + self.dep2 + '/lib64 ' +
 
-    def test_rpaths(self):
-        self.check_cc('dump-rpaths', test_command,
-                      "\n".join(["/first/rpath", "/second/rpath", "/third/rpath", "/fourth/rpath"]))
+                      '-L' + self.dep1 + '/lib ' +
+                      '-rpath ' + self.dep1 + '/lib ' +
 
+                      ' '.join(test_command))
 
-    def test_other_args(self):
-        self.check_cc('dump-other-args', test_command,
-                      "\n".join(["arg1", "-Wl,--start-group", "arg2", "arg3", "arg4",
-                                 "-Wl,--end-group", "arg5", "arg6"]))