Dynamic library/include paths (#8136)

Fixes #7855
Closes #8070
Closes #2645

When searching for library directories (e.g. to add "-L" arguments to
the compiler wrapper) Spack was only trying the "lib/" and "lib64/"
directories for each dependency install prefix; this missed cases
where packages would install libraries to subdirectories and also was
not customizable. This PR makes use of the ".headers" and ".libs"
properties for more-advanced location of header/library directories.
Since packages can override the default behavior of ".headers" and
".libs", it also allows package writers to customize.

The following environment variables which used to be set by Spack
for a package build have been removed:

* Remove SPACK_PREFIX and SPACK_DEPENDENCIES environment variables as
  they are no-longer used
* Remove SPACK_INSTALL environment variable: it was not used before
  this PR

8 files changed, 261 insertions, 248 deletions

diff --git a/lib/spack/env/cc b/lib/spack/env/cc
index 70f429055d..da1fee8756 100755
--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -24,7 +24,6 @@
 # the script runs. They are set by routines in spack.build_environment
 # as part of spack.package.Package.do_install().
 parameters=(
-    SPACK_PREFIX
     SPACK_ENV_PATH
     SPACK_DEBUG_LOG_DIR
     SPACK_DEBUG_LOG_ID
@@ -46,8 +45,6 @@ parameters=(
 #   SPACK_DEBUG
 # Test command is used to unit test the compiler script.
 #   SPACK_TEST_COMMAND
-# Dependencies can be empty for pkgs with no deps:
-#   SPACK_DEPENDENCIES
 
 # die()
 # Prints a message and exits with error 1.
@@ -385,52 +382,30 @@ case "$mode" in
         flags=("${flags[@]}" "${SPACK_LDFLAGS[@]}") ;;
 esac
 
+# Prepend include directories
+IFS=':' read -ra include_dirs <<< "$SPACK_INCLUDE_DIRS"
+if [[ $mode == cpp || $mode == cc || $mode == as || $mode == ccld ]]; then
+    for include_dir in "${include_dirs[@]}"; do
+        includes=("${includes[@]}" "$include_dir")
+    done
+fi
 
-# 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")
-        $add_rpaths && rpaths+=("$SPACK_PREFIX/lib64")
-        ;;
-esac
-
-# 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 in any compilation mode
-    case "$mode" in
-        cpp|cc|as|ccld)
-            if [[ -d $dep/include ]]; then
-                includes=("${includes[@]}" "$dep/include")
-            fi
-            ;;
-    esac
-
-    # Append lib/lib64 and RPATH directories, but only if we're linking
-    case "$mode" in
-        ld|ccld)
-            if [[ -d $dep/lib ]]; then
-                if [[ $SPACK_RPATH_DEPS == *$dep* ]]; then
-                    $add_rpaths && rpaths=("${rpaths[@]}" "$dep/lib")
-                fi
-                if [[ $SPACK_LINK_DEPS == *$dep* ]]; then
-                    libdirs=("${libdirs[@]}" "$dep/lib")
-                fi
-            fi
+IFS=':' read -ra rpath_dirs <<< "$SPACK_RPATH_DIRS"
+if [[ $mode == ccld || $mode == ld ]]; then
+    for rpath_dir in "${rpath_dirs[@]}"; do
+        # Append RPATH directories. Note that in the case of the
+        # top-level package these directories may not exist yet. For dependencies
+        # it is assumed that paths have already been confirmed.
+        $add_rpaths && rpaths=("${rpaths[@]}" "$rpath_dir")
+    done
+fi
 
-            if [[ -d $dep/lib64 ]]; then
-                if [[ $SPACK_RPATH_DEPS == *$dep* ]]; then
-                    $add_rpaths && rpaths+=("$dep/lib64")
-                fi
-                if [[ $SPACK_LINK_DEPS == *$dep* ]]; then
-                    libdirs+=("$dep/lib64")
-                fi
-            fi
-            ;;
-    esac
-done
+IFS=':' read -ra link_dirs <<< "$SPACK_LINK_DIRS"
+if [[ $mode == ccld || $mode == ld ]]; then
+    for link_dir in "${link_dirs[@]}"; do
+        libdirs=("${libdirs[@]}" "$link_dir")
+    done
+fi
 
 # add RPATHs if we're in in any linking mode
 case "$mode" in
diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 103de3d6ce..1ea0415576 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -53,9 +53,9 @@ import spack.main
 import spack.paths
 import spack.store
 from spack.util.string import plural
-from spack.util.environment import EnvironmentModifications, validate
-from spack.util.environment import preserve_environment
-from spack.util.environment import env_flag, filter_system_paths, get_path
+from spack.util.environment import (
+    env_flag, filter_system_paths, get_path, is_system_path,
+    EnvironmentModifications, validate, preserve_environment)
 from spack.util.environment import system_dirs
 from spack.util.executable import Executable
 from spack.util.module_cmd import load_module, get_path_from_module
@@ -73,7 +73,9 @@ SPACK_NO_PARALLEL_MAKE = 'SPACK_NO_PARALLEL_MAKE'
 # Spack's compiler wrappers.
 #
 SPACK_ENV_PATH = 'SPACK_ENV_PATH'
-SPACK_DEPENDENCIES = 'SPACK_DEPENDENCIES'
+SPACK_INCLUDE_DIRS = 'SPACK_INCLUDE_DIRS'
+SPACK_LINK_DIRS = 'SPACK_LINK_DIRS'
+SPACK_RPATH_DIRS = 'SPACK_RPATH_DIRS'
 SPACK_RPATH_DEPS = 'SPACK_RPATH_DEPS'
 SPACK_LINK_DEPS = 'SPACK_LINK_DEPS'
 SPACK_PREFIX = 'SPACK_PREFIX'
@@ -257,10 +259,47 @@ def set_build_environment_variables(pkg, env, dirty):
     build_link_deps = build_deps | link_deps
     rpath_deps      = get_rpath_deps(pkg)
 
+    link_dirs = []
+    include_dirs = []
+    rpath_dirs = []
+
+    # The top-level package is always RPATHed. It hasn't been installed yet
+    # so the RPATHs are added unconditionally (e.g. even though lib64/ may
+    # not be created for the install).
+    for libdir in ['lib', 'lib64']:
+        lib_path = os.path.join(pkg.prefix, libdir)
+        rpath_dirs.append(lib_path)
+
+    # Set up link, include, RPATH directories that are passed to the
+    # compiler wrapper
+    for dep in link_deps:
+        if is_system_path(dep.prefix):
+            continue
+        # TODO: packages with alternative implementations of .libs which
+        # are external may place libraries in nonstandard directories, so
+        # there should be a check for that
+        query = pkg.spec[dep.name]
+        try:
+            dep_link_dirs = list(query.libs.directories)
+            link_dirs.extend(dep_link_dirs)
+            if dep in rpath_deps:
+                rpath_dirs.extend(dep_link_dirs)
+        except spack.spec.NoLibrariesError:
+            tty.debug("No libraries found for {0}".format(dep.name))
+
+        try:
+            include_dirs.extend(query.headers.directories)
+        except spack.spec.NoHeadersError:
+            tty.debug("No headers found for {0}".format(dep.name))
+        if os.path.isdir(dep.prefix.include):
+            include_dirs.append(dep.prefix.include)
+
+    env.set(SPACK_LINK_DIRS, ':'.join(link_dirs))
+    env.set(SPACK_INCLUDE_DIRS, ':'.join(include_dirs))
+    env.set(SPACK_RPATH_DIRS, ':'.join(rpath_dirs))
+
     build_prefixes      = [dep.prefix for dep in build_deps]
-    link_prefixes       = [dep.prefix for dep in link_deps]
     build_link_prefixes = [dep.prefix for dep in build_link_deps]
-    rpath_prefixes      = [dep.prefix for dep in rpath_deps]
 
     # add run-time dependencies of direct build-time dependencies:
     for build_dep in build_deps:
@@ -273,26 +312,11 @@ def set_build_environment_variables(pkg, env, dirty):
     # contain hundreds of other packages installed in the same directory.
     # If these paths come first, they can overshadow Spack installations.
     build_prefixes      = filter_system_paths(build_prefixes)
-    link_prefixes       = filter_system_paths(link_prefixes)
     build_link_prefixes = filter_system_paths(build_link_prefixes)
-    rpath_prefixes      = filter_system_paths(rpath_prefixes)
-
-    # Prefixes of all of the package's dependencies go in SPACK_DEPENDENCIES
-    env.set_path(SPACK_DEPENDENCIES, build_link_prefixes)
-
-    # These variables control compiler wrapper behavior
-    env.set_path(SPACK_RPATH_DEPS, rpath_prefixes)
-    env.set_path(SPACK_LINK_DEPS, link_prefixes)
 
     # Add dependencies to CMAKE_PREFIX_PATH
     env.set_path('CMAKE_PREFIX_PATH', build_link_prefixes)
 
-    # Install prefix
-    env.set(SPACK_PREFIX, pkg.prefix)
-
-    # Install root prefix
-    env.set(SPACK_INSTALL, spack.store.root)
-
     # Set environment variables if specified for
     # the given compiler
     compiler = pkg.compiler
@@ -656,6 +680,11 @@ def setup_package(pkg, dirty):
         dpkg.setup_dependent_package(pkg.module, spec)
         dpkg.setup_dependent_environment(spack_env, run_env, spec)
 
+    if (not dirty) and (not spack_env.is_unset('CPATH')):
+        tty.warn("A dependency has updated CPATH, this may lead pkg-config"
+                 " to assume that the package is part of the system"
+                 " includes and omit it when invoked with '--cflags'.")
+
     set_module_variables_for_package(pkg)
     pkg.setup_environment(spack_env, run_env)
 
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 9706ab215d..7fcfe82c35 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -107,7 +107,7 @@ import spack.util.spack_yaml as syaml
 
 from spack.dependency import Dependency, all_deptypes, canonical_deptype
 from spack.util.module_cmd import get_path_from_module, load_module
-from spack.error import SpecError, UnsatisfiableSpecError
+from spack.error import SpackError, SpecError, UnsatisfiableSpecError
 from spack.provider_index import ProviderIndex
 from spack.util.crypto import prefix_bits
 from spack.util.executable import Executable
@@ -669,7 +669,7 @@ def _headers_default_handler(descriptor, spec, cls):
         HeaderList: The headers in ``prefix.include``
 
     Raises:
-        RuntimeError: If no headers are found
+        NoHeadersError: If no headers are found
     """
     headers = find_headers('*', root=spec.prefix.include, recursive=True)
 
@@ -677,7 +677,7 @@ def _headers_default_handler(descriptor, spec, cls):
         return headers
     else:
         msg = 'Unable to locate {0} headers in {1}'
-        raise RuntimeError(msg.format(spec.name, spec.prefix.include))
+        raise NoHeadersError(msg.format(spec.name, spec.prefix.include))
 
 
 def _libs_default_handler(descriptor, spec, cls):
@@ -697,7 +697,7 @@ def _libs_default_handler(descriptor, spec, cls):
         LibraryList: The libraries found
 
     Raises:
-        RuntimeError: If no libraries are found
+        NoLibrariesError: If no libraries are found
     """
 
     # Variable 'name' is passed to function 'find_libraries', which supports
@@ -737,7 +737,7 @@ def _libs_default_handler(descriptor, spec, cls):
                 return libs
 
     msg = 'Unable to recursively locate {0} libraries in {1}'
-    raise RuntimeError(msg.format(spec.name, prefix))
+    raise NoLibrariesError(msg.format(spec.name, prefix))
 
 
 class ForwardQueryToPackage(object):
@@ -3721,6 +3721,14 @@ class DuplicateCompilerSpecError(SpecError):
     """Raised when the same compiler occurs in a spec twice."""
 
 
+class NoLibrariesError(SpackError):
+    """Raised when package libraries are requested but cannot be found"""
+
+
+class NoHeadersError(SpackError):
+    """Raised when package headers are requested but cannot be found"""
+
+
 class UnsupportedCompilerError(SpecError):
     """Raised when the user asks for a compiler spack doesn't know about."""
     def __init__(self, compiler_name):
diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py
index de50125f26..2540386109 100644
--- a/lib/spack/spack/test/build_environment.py
+++ b/lib/spack/spack/test/build_environment.py
@@ -12,10 +12,13 @@ from spack.paths import build_env_path
 from spack.build_environment import dso_suffix, _static_to_shared_library
 from spack.util.executable import Executable
 from spack.util.spack_yaml import syaml_dict, syaml_str
+from spack.util.environment import EnvironmentModifications
+
+from llnl.util.filesystem import LibraryList, HeaderList
 
 
 @pytest.fixture
-def build_environment():
+def build_environment(working_env):
     cc = Executable(os.path.join(build_env_path, "cc"))
     cxx = Executable(os.path.join(build_env_path, "c++"))
     fc = Executable(os.path.join(build_env_path, "fc"))
@@ -47,25 +50,15 @@ def build_environment():
 
     yield {'cc': cc, 'cxx': cxx, 'fc': fc}
 
-    for name in ('SPACK_CC', 'SPACK_CXX', 'SPACK_FC', 'SPACK_PREFIX',
-                 'SPACK_ENV_PATH', 'SPACK_DEBUG_LOG_DIR',
-                 'SPACK_COMPILER_SPEC', 'SPACK_SHORT_SPEC',
-                 'SPACK_CC_RPATH_ARG', 'SPACK_CXX_RPATH_ARG',
-                 'SPACK_F77_RPATH_ARG', 'SPACK_FC_RPATH_ARG',
-                 'SPACK_SYSTEM_DIRS'):
-        del os.environ[name]
-
 
 def test_static_to_shared_library(build_environment):
     os.environ['SPACK_TEST_COMMAND'] = 'dump-args'
 
     expected = {
-        'linux': ('/bin/mycc -Wl,-rpath,/spack-test-prefix/lib'
-                  ' -Wl,-rpath,/spack-test-prefix/lib64 -shared'
+        'linux': ('/bin/mycc -shared'
                   ' -Wl,-soname,{2} -Wl,--whole-archive {0}'
                   ' -Wl,--no-whole-archive -o {1}'),
-        'darwin': ('/bin/mycc -Wl,-rpath,/spack-test-prefix/lib'
-                   ' -Wl,-rpath,/spack-test-prefix/lib64 -dynamiclib'
+        'darwin': ('/bin/mycc -dynamiclib'
                    ' -install_name {1} -Wl,-force_load,{0} -o {1}')
     }
 
@@ -87,7 +80,7 @@ def test_static_to_shared_library(build_environment):
 
 @pytest.mark.regression('8345')
 @pytest.mark.usefixtures('config', 'mock_packages')
-def test_cc_not_changed_by_modules(monkeypatch):
+def test_cc_not_changed_by_modules(monkeypatch, working_env):
 
     s = spack.spec.Spec('cmake')
     s.concretize()
@@ -111,7 +104,7 @@ def test_cc_not_changed_by_modules(monkeypatch):
 
 
 @pytest.mark.usefixtures('config', 'mock_packages')
-def test_compiler_config_modifications(monkeypatch):
+def test_compiler_config_modifications(monkeypatch, working_env):
     s = spack.spec.Spec('cmake')
     s.concretize()
     pkg = s.package
@@ -184,15 +177,10 @@ def test_compiler_config_modifications(monkeypatch):
     expected = '/path/first:/path/last'
     assert os.environ['NEW_PATH_LIST'] == expected
 
-    os.environ.pop('SOME_VAR_STR', None)
-    os.environ.pop('SOME_VAR_NUM', None)
-    os.environ.pop('PATH_LIST', None)
-    os.environ.pop('EMPTY_PATH_LIST', None)
-    os.environ.pop('NEW_PATH_LIST', None)
-
 
 @pytest.mark.regression('9107')
-def test_spack_paths_before_module_paths(config, mock_packages, monkeypatch):
+def test_spack_paths_before_module_paths(
+        config, mock_packages, monkeypatch, working_env):
     s = spack.spec.Spec('cmake')
     s.concretize()
     pkg = s.package
@@ -230,3 +218,65 @@ def test_package_inheritance_module_setup(config, mock_packages):
     assert os.environ['TEST_MODULE_VAR'] == 'test_module_variable'
 
     os.environ.pop('TEST_MODULE_VAR')
+
+
+def test_set_build_environment_variables(
+        config, mock_packages, working_env, monkeypatch, tmpdir_factory):
+    """Check that build_environment supplies the needed library/include
+    directories via the SPACK_LINK_DIRS and SPACK_INCLUDE_DIRS environment
+    variables.
+    """
+
+    root = spack.spec.Spec('dt-diamond')
+    root.concretize()
+
+    for s in root.traverse():
+        s.prefix = '/{0}-prefix/'.format(s.name)
+
+    dep_pkg = root['dt-diamond-left'].package
+    dep_lib_paths = ['/test/path/to/ex1.so', '/test/path/to/subdir/ex2.so']
+    dep_lib_dirs = ['/test/path/to', '/test/path/to/subdir']
+    dep_libs = LibraryList(dep_lib_paths)
+
+    dep2_prefix = tmpdir_factory.mktemp('prefix')
+    dep2_include = dep2_prefix.ensure('include', dir=True)
+    dep2_pkg = root['dt-diamond-right'].package
+    dep2_pkg.spec.prefix = str(dep2_prefix)
+    dep2_inc_paths = ['/test2/path/to/ex1.h', '/test2/path/to/subdir/ex2.h']
+    dep2_inc_dirs = ['/test2/path/to', '/test2/path/to/subdir']
+    dep2_includes = HeaderList(dep2_inc_paths)
+
+    setattr(dep_pkg, 'libs', dep_libs)
+    setattr(dep2_pkg, 'headers', dep2_includes)
+    try:
+        pkg = root.package
+        env_mods = EnvironmentModifications()
+        spack.build_environment.set_build_environment_variables(
+            pkg, env_mods, dirty=False)
+
+        env_mods.apply_modifications()
+
+        def normpaths(paths):
+            return list(os.path.normpath(p) for p in paths)
+
+        link_dir_var = os.environ['SPACK_LINK_DIRS']
+        assert (
+            normpaths(link_dir_var.split(':')) == normpaths(dep_lib_dirs))
+
+        root_libdirs = ['/dt-diamond-prefix/lib', '/dt-diamond-prefix/lib64']
+        rpath_dir_var = os.environ['SPACK_RPATH_DIRS']
+        # The 'lib' and 'lib64' subdirectories of the root package prefix
+        # should always be rpathed and should be the first rpaths
+        assert (
+            normpaths(rpath_dir_var.split(':')) ==
+            normpaths(root_libdirs + dep_lib_dirs))
+
+        header_dir_var = os.environ['SPACK_INCLUDE_DIRS']
+        # As long as a dependency package has an 'include' prefix, it is added
+        # (regardless of whether it contains any header files)
+        assert (
+            normpaths(header_dir_var.split(':')) ==
+            normpaths(dep2_inc_dirs + [str(dep2_include)]))
+    finally:
+        delattr(dep_pkg, 'libs')
+        delattr(dep2_pkg, 'headers')
diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py
index fb9117bc0c..44a20ca56a 100644
--- a/lib/spack/spack/test/build_systems.py
+++ b/lib/spack/spack/test/build_systems.py
@@ -21,7 +21,7 @@ DATA_PATH = os.path.join(spack.paths.test_path, 'data')
     'directory',
     glob.iglob(os.path.join(DATA_PATH, 'make', 'affirmative', '*'))
 )
-def test_affirmative_make_check(directory, config, mock_packages):
+def test_affirmative_make_check(directory, config, mock_packages, working_env):
     """Tests that Spack correctly detects targets in a Makefile."""
 
     # Get a fake package
@@ -41,7 +41,7 @@ def test_affirmative_make_check(directory, config, mock_packages):
     glob.iglob(os.path.join(DATA_PATH, 'make', 'negative', '*'))
 )
 @pytest.mark.regression('9067')
-def test_negative_make_check(directory, config, mock_packages):
+def test_negative_make_check(directory, config, mock_packages, working_env):
     """Tests that Spack correctly ignores false positives in a Makefile."""
 
     # Get a fake package
@@ -61,7 +61,8 @@ def test_negative_make_check(directory, config, mock_packages):
     'directory',
     glob.iglob(os.path.join(DATA_PATH, 'ninja', 'affirmative', '*'))
 )
-def test_affirmative_ninja_check(directory, config, mock_packages):
+def test_affirmative_ninja_check(
+        directory, config, mock_packages, working_env):
     """Tests that Spack correctly detects targets in a Ninja build script."""
 
     # Get a fake package
@@ -85,7 +86,7 @@ def test_affirmative_ninja_check(directory, config, mock_packages):
     'directory',
     glob.iglob(os.path.join(DATA_PATH, 'ninja', 'negative', '*'))
 )
-def test_negative_ninja_check(directory, config, mock_packages):
+def test_negative_ninja_check(directory, config, mock_packages, working_env):
     """Tests that Spack correctly ignores false positives in a Ninja
     build script."""
 
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
index 2ad638ca8e..2a4d7ec3a8 100644
--- a/lib/spack/spack/test/cc.py
+++ b/lib/spack/spack/test/cc.py
@@ -61,19 +61,6 @@ test_args_without_paths = [
 #: The prefix of the package being mock installed
 pkg_prefix = '/spack-test-prefix'
 
-#
-# Expected RPATHs for the package itself.  The package is expected to
-# have only one of /lib or /lib64, but we add both b/c we can't know
-# before installing.
-#
-pkg_wl_rpaths = [
-    '-Wl,-rpath,' + pkg_prefix + '/lib',
-    '-Wl,-rpath,' + pkg_prefix + '/lib64']
-
-pkg_rpaths = [
-    '-rpath', '/spack-test-prefix/lib',
-    '-rpath', '/spack-test-prefix/lib64']
-
 # Compilers to use during tests
 cc = Executable(os.path.join(build_env_path, "cc"))
 ld = Executable(os.path.join(build_env_path, "ld"))
@@ -110,7 +97,9 @@ def wrapper_environment():
             SPACK_CXX_RPATH_ARG='-Wl,-rpath,',
             SPACK_F77_RPATH_ARG='-Wl,-rpath,',
             SPACK_FC_RPATH_ARG='-Wl,-rpath,',
-            SPACK_DEPENDENCIES=None):
+            SPACK_LINK_DIRS=None,
+            SPACK_INCLUDE_DIRS=None,
+            SPACK_RPATH_DIRS=None):
         yield
 
 
@@ -126,36 +115,6 @@ def wrapper_flags():
         yield
 
 
-@pytest.fixture(scope='session')
-def dep1(tmpdir_factory):
-    path = tmpdir_factory.mktemp('cc-dep1')
-    path.mkdir('include')
-    path.mkdir('lib')
-    yield str(path)
-
-
-@pytest.fixture(scope='session')
-def dep2(tmpdir_factory):
-    path = tmpdir_factory.mktemp('cc-dep2')
-    path.mkdir('lib64')
-    yield str(path)
-
-
-@pytest.fixture(scope='session')
-def dep3(tmpdir_factory):
-    path = tmpdir_factory.mktemp('cc-dep3')
-    path.mkdir('include')
-    path.mkdir('lib64')
-    yield str(path)
-
-
-@pytest.fixture(scope='session')
-def dep4(tmpdir_factory):
-    path = tmpdir_factory.mktemp('cc-dep4')
-    path.mkdir('include')
-    yield str(path)
-
-
 pytestmark = pytest.mark.usefixtures('wrapper_environment')
 
 
@@ -167,7 +126,8 @@ def check_args(cc, args, expected):
     contain spaces are parsed correctly.
     """
     with set_env(SPACK_TEST_COMMAND='dump-args'):
-        assert expected == cc(*args, output=str).strip().split('\n')
+        cc_modified_args = cc(*args, output=str).strip().split('\n')
+        assert expected == cc_modified_args
 
 
 def dump_mode(cc, args):
@@ -217,7 +177,6 @@ def test_ld_flags(wrapper_flags):
         test_include_paths +
         test_library_paths +
         test_rpaths +
-        pkg_rpaths +
         test_args_without_paths +
         spack_ldlibs)
 
@@ -242,7 +201,6 @@ def test_cc_flags(wrapper_flags):
         test_include_paths +
         test_library_paths +
         test_wl_rpaths +
-        pkg_wl_rpaths +
         test_args_without_paths +
         spack_ldlibs)
 
@@ -257,7 +215,6 @@ def test_cxx_flags(wrapper_flags):
         test_include_paths +
         test_library_paths +
         test_wl_rpaths +
-        pkg_wl_rpaths +
         test_args_without_paths +
         spack_ldlibs)
 
@@ -272,7 +229,6 @@ def test_fc_flags(wrapper_flags):
         test_include_paths +
         test_library_paths +
         test_wl_rpaths +
-        pkg_wl_rpaths +
         test_args_without_paths +
         spack_ldlibs)
 
@@ -285,122 +241,108 @@ def test_dep_rpath():
         test_include_paths +
         test_library_paths +
         test_wl_rpaths +
-        pkg_wl_rpaths +
         test_args_without_paths)
 
 
-def test_dep_include(dep4):
+def test_dep_include():
     """Ensure a single dependency include directory is added."""
-    with set_env(SPACK_DEPENDENCIES=dep4,
-                 SPACK_RPATH_DEPS=dep4,
-                 SPACK_LINK_DEPS=dep4):
+    with set_env(SPACK_INCLUDE_DIRS='x'):
         check_args(
             cc, test_args,
             [real_cc] +
             test_include_paths +
-            ['-I' + dep4 + '/include'] +
+            ['-Ix'] +
             test_library_paths +
             test_wl_rpaths +
-            pkg_wl_rpaths +
             test_args_without_paths)
 
 
-def test_dep_lib(dep2):
+def test_dep_lib():
     """Ensure a single dependency RPATH is added."""
-    with set_env(SPACK_DEPENDENCIES=dep2,
-                 SPACK_RPATH_DEPS=dep2,
-                 SPACK_LINK_DEPS=dep2):
+    with set_env(SPACK_LINK_DIRS='x',
+                 SPACK_RPATH_DIRS='x'):
         check_args(
             cc, test_args,
             [real_cc] +
             test_include_paths +
             test_library_paths +
-            ['-L' + dep2 + '/lib64'] +
+            ['-Lx'] +
             test_wl_rpaths +
-            pkg_wl_rpaths +
-            ['-Wl,-rpath,' + dep2 + '/lib64'] +
+            ['-Wl,-rpath,x'] +
             test_args_without_paths)
 
 
-def test_dep_lib_no_rpath(dep2):
+def test_dep_lib_no_rpath():
     """Ensure a single dependency link flag is added with no dep RPATH."""
-    with set_env(SPACK_DEPENDENCIES=dep2,
-                 SPACK_LINK_DEPS=dep2):
+    with set_env(SPACK_LINK_DIRS='x'):
         check_args(
             cc, test_args,
             [real_cc] +
             test_include_paths +
             test_library_paths +
-            ['-L' + dep2 + '/lib64'] +
+            ['-Lx'] +
             test_wl_rpaths +
-            pkg_wl_rpaths +
             test_args_without_paths)
 
 
-def test_dep_lib_no_lib(dep2):
+def test_dep_lib_no_lib():
     """Ensure a single dependency RPATH is added with no -L."""
-    with set_env(SPACK_DEPENDENCIES=dep2,
-                 SPACK_RPATH_DEPS=dep2):
+    with set_env(SPACK_RPATH_DIRS='x'):
         check_args(
             cc, test_args,
             [real_cc] +
             test_include_paths +
             test_library_paths +
             test_wl_rpaths +
-            pkg_wl_rpaths +
-            ['-Wl,-rpath,' + dep2 + '/lib64'] +
+            ['-Wl,-rpath,x'] +
             test_args_without_paths)
 
 
-def test_ccld_deps(dep1, dep2, dep3, dep4):
+def test_ccld_deps():
     """Ensure all flags are added in ccld mode."""
-    deps = ':'.join((dep1, dep2, dep3, dep4))
-    with set_env(SPACK_DEPENDENCIES=deps,
-                 SPACK_RPATH_DEPS=deps,
-                 SPACK_LINK_DEPS=deps):
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
         check_args(
             cc, test_args,
             [real_cc] +
             test_include_paths +
-            ['-I' + dep1 + '/include',
-             '-I' + dep3 + '/include',
-             '-I' + dep4 + '/include'] +
+            ['-Ixinc',
+             '-Iyinc',
+             '-Izinc'] +
             test_library_paths +
-            ['-L' + dep1 + '/lib',
-             '-L' + dep2 + '/lib64',
-             '-L' + dep3 + '/lib64'] +
+            ['-Lxlib',
+             '-Lylib',
+             '-Lzlib'] +
             test_wl_rpaths +
-            pkg_wl_rpaths +
-            ['-Wl,-rpath,' + dep1 + '/lib',
-             '-Wl,-rpath,' + dep2 + '/lib64',
-             '-Wl,-rpath,' + dep3 + '/lib64'] +
+            ['-Wl,-rpath,xlib',
+             '-Wl,-rpath,ylib',
+             '-Wl,-rpath,zlib'] +
             test_args_without_paths)
 
 
-def test_cc_deps(dep1, dep2, dep3, dep4):
+def test_cc_deps():
     """Ensure -L and RPATHs are not added in cc mode."""
-    deps = ':'.join((dep1, dep2, dep3, dep4))
-    with set_env(SPACK_DEPENDENCIES=deps,
-                 SPACK_RPATH_DEPS=deps,
-                 SPACK_LINK_DEPS=deps):
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
         check_args(
             cc, ['-c'] + test_args,
             [real_cc] +
             test_include_paths +
-            ['-I' + dep1 + '/include',
-             '-I' + dep3 + '/include',
-             '-I' + dep4 + '/include'] +
+            ['-Ixinc',
+             '-Iyinc',
+             '-Izinc'] +
             test_library_paths +
             ['-c'] +
             test_args_without_paths)
 
 
-def test_ccld_with_system_dirs(dep1, dep2, dep3, dep4):
+def test_ccld_with_system_dirs():
     """Ensure all flags are added in ccld mode."""
-    deps = ':'.join((dep1, dep2, dep3, dep4))
-    with set_env(SPACK_DEPENDENCIES=deps,
-                 SPACK_RPATH_DEPS=deps,
-                 SPACK_LINK_DEPS=deps):
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
 
         sys_path_args = ['-I/usr/include',
                          '-L/usr/local/lib',
@@ -411,91 +353,84 @@ def test_ccld_with_system_dirs(dep1, dep2, dep3, dep4):
             cc, sys_path_args + test_args,
             [real_cc] +
             test_include_paths +
-            ['-I' + dep1 + '/include',
-             '-I' + dep3 + '/include',
-             '-I' + dep4 + '/include'] +
+            ['-Ixinc',
+             '-Iyinc',
+             '-Izinc'] +
             ['-I/usr/include',
              '-I/usr/local/include'] +
             test_library_paths +
-            ['-L' + dep1 + '/lib',
-             '-L' + dep2 + '/lib64',
-             '-L' + dep3 + '/lib64'] +
+            ['-Lxlib',
+             '-Lylib',
+             '-Lzlib'] +
             ['-L/usr/local/lib',
              '-L/lib64/'] +
             test_wl_rpaths +
-            pkg_wl_rpaths +
-            ['-Wl,-rpath,' + dep1 + '/lib',
-             '-Wl,-rpath,' + dep2 + '/lib64',
-             '-Wl,-rpath,' + dep3 + '/lib64'] +
+            ['-Wl,-rpath,xlib',
+             '-Wl,-rpath,ylib',
+             '-Wl,-rpath,zlib'] +
             ['-Wl,-rpath,/usr/lib64'] +
             test_args_without_paths)
 
 
-def test_ld_deps(dep1, dep2, dep3, dep4):
+def test_ld_deps():
     """Ensure no (extra) -I args or -Wl, are passed in ld mode."""
-    deps = ':'.join((dep1, dep2, dep3, dep4))
-    with set_env(SPACK_DEPENDENCIES=deps,
-                 SPACK_RPATH_DEPS=deps,
-                 SPACK_LINK_DEPS=deps):
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
         check_args(
             ld, test_args,
             ['ld'] +
             test_include_paths +
             test_library_paths +
-            ['-L' + dep1 + '/lib',
-             '-L' + dep2 + '/lib64',
-             '-L' + dep3 + '/lib64'] +
+            ['-Lxlib',
+             '-Lylib',
+             '-Lzlib'] +
             test_rpaths +
-            pkg_rpaths +
-            ['-rpath', dep1 + '/lib',
-             '-rpath', dep2 + '/lib64',
-             '-rpath', dep3 + '/lib64'] +
+            ['-rpath', 'xlib',
+             '-rpath', 'ylib',
+             '-rpath', 'zlib'] +
             test_args_without_paths)
 
 
-def test_ld_deps_no_rpath(dep1, dep2, dep3, dep4):
+def test_ld_deps_no_rpath():
     """Ensure SPACK_LINK_DEPS controls -L for ld."""
-    deps = ':'.join((dep1, dep2, dep3, dep4))
-    with set_env(SPACK_DEPENDENCIES=deps,
-                 SPACK_LINK_DEPS=deps):
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_LINK_DIRS='xlib:ylib:zlib'):
         check_args(
             ld, test_args,
             ['ld'] +
             test_include_paths +
             test_library_paths +
-            ['-L' + dep1 + '/lib',
-             '-L' + dep2 + '/lib64',
-             '-L' + dep3 + '/lib64'] +
+            ['-Lxlib',
+             '-Lylib',
+             '-Lzlib'] +
             test_rpaths +
-            pkg_rpaths +
             test_args_without_paths)
 
 
-def test_ld_deps_no_link(dep1, dep2, dep3, dep4):
+def test_ld_deps_no_link():
     """Ensure SPACK_RPATH_DEPS controls -rpath for ld."""
-    deps = ':'.join((dep1, dep2, dep3, dep4))
-    with set_env(SPACK_DEPENDENCIES=deps,
-                 SPACK_RPATH_DEPS=deps):
+    with set_env(SPACK_INCLUDE_DIRS='xinc:yinc:zinc',
+                 SPACK_RPATH_DIRS='xlib:ylib:zlib'):
         check_args(
             ld, test_args,
             ['ld'] +
             test_include_paths +
             test_library_paths +
             test_rpaths +
-            pkg_rpaths +
-            ['-rpath', dep1 + '/lib',
-             '-rpath', dep2 + '/lib64',
-             '-rpath', dep3 + '/lib64'] +
+            ['-rpath', 'xlib',
+             '-rpath', 'ylib',
+             '-rpath', 'zlib'] +
             test_args_without_paths)
 
 
-def test_ld_deps_partial(dep1):
+def test_ld_deps_partial():
     """Make sure ld -r (partial link) is handled correctly on OS's where it
        doesn't accept rpaths.
     """
-    with set_env(SPACK_DEPENDENCIES=dep1,
-                 SPACK_RPATH_DEPS=dep1,
-                 SPACK_LINK_DEPS=dep1):
+    with set_env(SPACK_INCLUDE_DIRS='xinc',
+                 SPACK_RPATH_DIRS='xlib',
+                 SPACK_LINK_DIRS='xlib'):
         # 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"
@@ -504,10 +439,9 @@ def test_ld_deps_partial(dep1):
             ['ld'] +
             test_include_paths +
             test_library_paths +
-            ['-L' + dep1 + '/lib'] +
+            ['-Lxlib'] +
             test_rpaths +
-            pkg_rpaths +
-            ['-rpath', dep1 + '/lib'] +
+            ['-rpath', 'xlib'] +
             ['-r'] +
             test_args_without_paths)
 
@@ -519,7 +453,7 @@ def test_ld_deps_partial(dep1):
             ['ld'] +
             test_include_paths +
             test_library_paths +
-            ['-L' + dep1 + '/lib'] +
+            ['-Lxlib'] +
             test_rpaths +
             ['-r'] +
             test_args_without_paths)
@@ -534,7 +468,6 @@ def test_ccache_prepend_for_cc():
             test_include_paths +
             test_library_paths +
             test_wl_rpaths +
-            pkg_wl_rpaths +
             test_args_without_paths)
 
 
@@ -546,5 +479,4 @@ def test_no_ccache_prepend_for_fc():
         test_include_paths +
         test_library_paths +
         test_wl_rpaths +
-        pkg_wl_rpaths +
         test_args_without_paths)
diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py
index a364b417a8..0e5a2ffad7 100644
--- a/lib/spack/spack/test/conftest.py
+++ b/lib/spack/spack/test/conftest.py
@@ -137,6 +137,13 @@ def remove_whatever_it_is(path):
         shutil.rmtree(path)
 
 
+@pytest.fixture
+def working_env():
+    saved_env = os.environ.copy()
+    yield
+    os.environ = saved_env
+
+
 @pytest.fixture(scope='function', autouse=True)
 def check_for_leftover_stage_files(request, mock_stage, _ignore_stage_files):
     """Ensure that each test leaves a clean stage when done.
diff --git a/lib/spack/spack/util/environment.py b/lib/spack/spack/util/environment.py
index 6ef91f88d8..14e4c06501 100644
--- a/lib/spack/spack/util/environment.py
+++ b/lib/spack/spack/util/environment.py
@@ -338,6 +338,17 @@ class EnvironmentModifications(object):
             modifications[item.name].append(item)
         return modifications
 
+    def is_unset(self, var_name):
+        modifications = self.group_by_name()
+        var_updates = modifications.get(var_name, None)
+        if not var_updates:
+            # We did not explicitly unset it
+            return False
+
+        # The last modification must unset the variable for it to be considered
+        # unset
+        return (type(var_updates[-1]) == UnsetEnv)
+
     def clear(self):
         """
         Clears the current list of modifications