compiler wrappers: don't override -isystem with -I (#16077)

If the Spack compiler wrapper encounters any "-isystem" option, then
when adding include directories for Spack dependencies, Spack will
use "-isystem" instead of "-I". This prevents Spack-generated "-I"
options from overriding the "-isystem" options generated by the build
system. To ensure that build-system "-isystem" directories are
searched first, Spack places all of its inserted "-isystem"
directories after.

The new ordering of -isystem includes is:

* -isystem from build system (not system directories)
* -isystem from Spack
* -isystem from build system (for directories like /usr/include)

The prior order of "-I" arguments is preserved (although as of this
commit Spack no longer generates -I if -isystem is detected):

* -I from build system (not system directories)
* -I from Spack (only if there are no "-isystem" options)
* -I from build system (for directories like /usr/include)

2 files changed, 102 insertions, 14 deletions

diff --git a/lib/spack/env/cc b/lib/spack/env/cc
index f2b8bf577f..b5913c5f10 100755
--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -15,9 +15,9 @@
 # 1. It allows Spack to swap compilers into and out of builds easily.
 # 2. It adds several options to the compile line so that spack
 #    packages can find their dependencies at build time and run time:
-#      -I           arguments for dependency /include directories.
-#      -L           arguments for dependency /lib directories.
-#      -Wl,-rpath   arguments for dependency /lib directories.
+#      -I and/or -isystem arguments for dependency /include directories.
+#      -L                 arguments for dependency /lib directories.
+#      -Wl,-rpath         arguments for dependency /lib directories.
 #
 
 # This is an array of environment variables that need to be set before
@@ -251,10 +251,11 @@ input_command="$*"
 #
 # Parse the command line arguments.
 #
-# We extract -L, -I, and -Wl,-rpath arguments from the command line and
-# recombine them with Spack arguments later.  We parse these out so that
-# we can make sure that system paths come last, that package arguments
-# come first, and that Spack arguments are injected properly.
+# We extract -L, -I, -isystem and -Wl,-rpath arguments from the
+# command line and recombine them with Spack arguments later.  We
+# parse these out so that we can make sure that system paths come
+# last, that package arguments come first, and that Spack arguments
+# are injected properly.
 #
 # All other arguments, including -l arguments, are treated as
 # 'other_args' and left in their original order.  This ensures that
@@ -273,12 +274,24 @@ system_libdirs=()
 system_rpaths=()
 libs=()
 other_args=()
+isystem_system_includes=()
+isystem_includes=()
 
 while [ -n "$1" ]; do
     # an RPATH to be added after the case statement.
     rp=""
 
     case "$1" in
+        -isystem*)
+            arg="${1#-isystem}"
+	    isystem_was_used=true
+            if [ -z "$arg" ]; then shift; arg="$1"; fi
+            if system_dir "$arg"; then
+                isystem_system_includes+=("$arg")
+            else
+                isystem_includes+=("$arg")
+            fi
+            ;;
         -I*)
             arg="${1#-I}"
             if [ -z "$arg" ]; then shift; arg="$1"; fi
@@ -425,12 +438,6 @@ then
     esac
 fi
 
-# Prepend include directories
-IFS=':' read -ra include_dirs <<< "$SPACK_INCLUDE_DIRS"
-if [[ $mode == cpp || $mode == cc || $mode == as || $mode == ccld ]]; then
-    includes=("${includes[@]}" "${include_dirs[@]}")
-fi
-
 IFS=':' read -ra rpath_dirs <<< "$SPACK_RPATH_DIRS"
 if [[ $mode == ccld || $mode == ld ]]; then
 
@@ -481,9 +488,22 @@ args=()
 # flags assembled earlier
 args+=("${flags[@]}")
 
-# include directory search paths
+# Insert include directories just prior to any system include directories
+
 for dir in "${includes[@]}";         do args+=("-I$dir"); done
+for dir in "${isystem_includes[@]}";         do args+=("-isystem$dir"); done
+
+IFS=':' read -ra spack_include_dirs <<< "$SPACK_INCLUDE_DIRS"
+if [[ $mode == cpp || $mode == cc || $mode == as || $mode == ccld ]]; then
+    if [[ "$isystem_was_used" == "true" ]] ; then
+	for dir in "${spack_include_dirs[@]}";  do args+=("-isystem$dir"); done
+    else
+	for dir in "${spack_include_dirs[@]}";  do args+=("-I$dir"); done
+    fi
+fi
+
 for dir in "${system_includes[@]}";  do args+=("-I$dir"); done
+for dir in "${isystem_system_includes[@]}";  do args+=("-isystem$dir"); done
 
 # Library search paths
 for dir in "${libdirs[@]}";          do args+=("-L$dir"); done
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
index c1e1db914f..7b8d34fbde 100644
--- a/lib/spack/spack/test/cc.py
+++ b/lib/spack/spack/test/cc.py
@@ -338,6 +338,36 @@ def test_ccld_deps():
             test_args_without_paths)
 
 
+def test_ccld_deps_isystem():
+    """Ensure all flags are added in ccld mode.
+       When a build uses -isystem, Spack should inject it's
+       include paths using -isystem. Spack will insert these
+       after any provided -isystem includes, but before any
+       system directories included using -isystem"""
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
+        mytest_args = test_args + ['-isystemfooinc']
+        check_args(
+            cc, mytest_args,
+            [real_cc] +
+            test_include_paths +
+            ['-isystemfooinc',
+             '-isystemxinc',
+             '-isystemyinc',
+             '-isystemzinc'] +
+            test_library_paths +
+            ['-Lxlib',
+             '-Lylib',
+             '-Lzlib'] +
+            ['-Wl,--disable-new-dtags'] +
+            test_wl_rpaths +
+            ['-Wl,-rpath,xlib',
+             '-Wl,-rpath,ylib',
+             '-Wl,-rpath,zlib'] +
+            test_args_without_paths)
+
+
 def test_cc_deps():
     """Ensure -L and RPATHs are not added in cc mode."""
     with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
@@ -390,6 +420,44 @@ def test_ccld_with_system_dirs():
             test_args_without_paths)
 
 
+def test_ccld_with_system_dirs_isystem():
+    """Ensure all flags are added in ccld mode.
+       Ensure that includes are in the proper
+       place when a build uses -isystem, and uses
+       system directories in the include paths"""
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
+
+        sys_path_args = ['-isystem/usr/include',
+                         '-L/usr/local/lib',
+                         '-Wl,-rpath,/usr/lib64',
+                         '-isystem/usr/local/include',
+                         '-L/lib64/']
+        check_args(
+            cc, sys_path_args + test_args,
+            [real_cc] +
+            test_include_paths +
+            ['-isystemxinc',
+             '-isystemyinc',
+             '-isystemzinc'] +
+            ['-isystem/usr/include',
+             '-isystem/usr/local/include'] +
+            test_library_paths +
+            ['-Lxlib',
+             '-Lylib',
+             '-Lzlib'] +
+            ['-L/usr/local/lib',
+             '-L/lib64/'] +
+            ['-Wl,--disable-new-dtags'] +
+            test_wl_rpaths +
+            ['-Wl,-rpath,xlib',
+             '-Wl,-rpath,ylib',
+             '-Wl,-rpath,zlib'] +
+            ['-Wl,-rpath,/usr/lib64'] +
+            test_args_without_paths)
+
+
 def test_ld_deps():
     """Ensure no (extra) -I args or -Wl, are passed in ld mode."""
     with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',