cc: parse RPATHs when in ld mode

In the pure `ld` case, we weren't actually parsing `RPATH` arguments separately as we
do for `ccld`. Fix this by adding *another* nested case statement for raw `RPATH`
parsing.

There are now 3 places where we deal with `-rpath` and friends, but I don't see a great
way to unify them, as `-Wl,`, `-Xlinker`, and raw `-rpath` arguments are all ever so
slightly different.

Also, this Fixes ordering of assertions to make `pytest` diffs more intelligible.
The meaning of `+` and `-` in diffs changed in `pytest` 6.0 and the "preferred" order
for assertions became `assert actual == expected` instead of the other way around.

Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>

lib/spack/env/cc

@@ -101,10 +101,9 @@ setsep() {
     esac
 }
 
-# prepend LISTNAME ELEMENT [SEP]
+# prepend LISTNAME ELEMENT
 #
-# Prepend ELEMENT to the list stored in the variable LISTNAME,
-# assuming the list is separated by SEP.
+# Prepend ELEMENT to the list stored in the variable LISTNAME.
 # Handles empty lists and single-element lists.
 prepend() {
     varname="$1"
@@ -682,7 +681,36 @@ categorize_arguments() {
             "$dtags_to_strip")
                 ;;
             *)
-                append return_other_args_list "$1"
+                # if mode is not ld, we can just add to other args
+                if [ "$mode" != "ld" ]; then
+                    append return_other_args_list "$1"
+                    shift
+                    continue
+                fi
+
+                # if we're in linker mode, we need to parse raw RPATH args
+                case "$1" in
+                    -rpath=*)
+                        arg="${1#-rpath=}"
+                        append_path_lists return_rpath_dirs_list "$arg"
+                        ;;
+                    --rpath=*)
+                        arg="${1#--rpath=}"
+                        append_path_lists return_rpath_dirs_list "$arg"
+                        ;;
+                    -rpath|--rpath)
+                        if [ $# -eq 1 ]; then
+                            # -rpath without value: let the linker raise an error.
+                            append return_other_args_list "$1"
+                            break
+                        fi
+                        shift
+                        append_path_lists return_rpath_dirs_list "$1"
+                        ;;
+                    *)
+                        append return_other_args_list "$1"
+                        ;;
+                esac
                 ;;
         esac
         shift
@@ -890,35 +918,34 @@ extend args_list system_spack_flags_lib_dirs_list "-L"
 extend args_list system_lib_dirs_list "-L"
 
 # RPATHs arguments
+rpath_prefix=""
 case "$mode" in
     ccld)
         if [ -n "$dtags_to_add" ] ; then
             append args_list "$linker_arg$dtags_to_add"
         fi
-        extend args_list spack_store_spack_flags_rpath_dirs_list "$rpath"
-        extend args_list spack_store_rpath_dirs_list "$rpath"
-
-        extend args_list spack_flags_rpath_dirs_list "$rpath"
-        extend args_list rpath_dirs_list "$rpath"
-
-        extend args_list system_spack_flags_rpath_dirs_list "$rpath"
-        extend args_list system_rpath_dirs_list "$rpath"
+        rpath_prefix="$rpath"
         ;;
     ld)
         if [ -n "$dtags_to_add" ] ; then
             append args_list "$dtags_to_add"
         fi
-        extend args_list spack_store_spack_flags_rpath_dirs_list "-rpath${lsep}"
-        extend args_list spack_store_rpath_dirs_list "-rpath${lsep}"
-
-        extend args_list spack_flags_rpath_dirs_list "-rpath${lsep}"
-        extend args_list rpath_dirs_list "-rpath${lsep}"
-
-        extend args_list system_spack_flags_rpath_dirs_list "-rpath${lsep}"
-        extend args_list system_rpath_dirs_list "-rpath${lsep}"
+        rpath_prefix="-rpath${lsep}"
         ;;
 esac
 
+# if mode is ccld or ld, extend RPATH lists with the prefix determined above
+if [ -n "$rpath_prefix" ]; then
+    extend args_list spack_store_spack_flags_rpath_dirs_list "$rpath_prefix"
+    extend args_list spack_store_rpath_dirs_list "$rpath_prefix"
+
+    extend args_list spack_flags_rpath_dirs_list "$rpath_prefix"
+    extend args_list rpath_dirs_list "$rpath_prefix"
+
+    extend args_list system_spack_flags_rpath_dirs_list "$rpath_prefix"
+    extend args_list system_rpath_dirs_list "$rpath_prefix"
+fi
+
 # Other arguments from the input command
 extend args_list other_args_list
 extend args_list spack_flags_other_args_list

lib/spack/spack/test/cc.py

@@ -199,7 +199,7 @@ def check_args(cc, args, expected):
     """
     with set_env(SPACK_TEST_COMMAND="dump-args"):
         cc_modified_args = cc(*args, output=str).strip().split("\n")
-        assert expected == cc_modified_args
+        assert cc_modified_args == expected
 
 
 def check_args_contents(cc, args, must_contain, must_not_contain):
@@ -272,6 +272,43 @@ def test_ld_mode(wrapper_environment):
     assert dump_mode(ld, ["foo.o", "bar.o", "baz.o", "-o", "foo", "-Wl,-rpath,foo"]) == "ld"
 
 
+def test_ld_unterminated_rpath(wrapper_environment):
+    check_args(
+        ld,
+        ["foo.o", "bar.o", "baz.o", "-o", "foo", "-rpath"],
+        ["ld", "--disable-new-dtags", "foo.o", "bar.o", "baz.o", "-o", "foo", "-rpath"],
+    )
+
+
+def test_xlinker_unterminated_rpath(wrapper_environment):
+    check_args(
+        cc,
+        ["foo.o", "bar.o", "baz.o", "-o", "foo", "-Xlinker", "-rpath"],
+        [real_cc]
+        + target_args
+        + [
+            "-Wl,--disable-new-dtags",
+            "foo.o",
+            "bar.o",
+            "baz.o",
+            "-o",
+            "foo",
+            "-Xlinker",
+            "-rpath",
+        ],
+    )
+
+
+def test_wl_unterminated_rpath(wrapper_environment):
+    check_args(
+        cc,
+        ["foo.o", "bar.o", "baz.o", "-o", "foo", "-Wl,-rpath"],
+        [real_cc]
+        + target_args
+        + ["-Wl,--disable-new-dtags", "foo.o", "bar.o", "baz.o", "-o", "foo", "-Wl,-rpath"],
+    )
+
+
 def test_ld_flags(wrapper_environment, wrapper_flags):
     check_args(
         ld,