diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 14bd7babf0..b9fcece027 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -306,6 +306,19 @@ def set_compiler_environment_variables(pkg, env): return env +def _place_externals_last(spec_container): + """ + For a (possibly unordered) container of specs, return an ordered list + where all external specs are at the end of the list. External packages + may be installed in merged prefixes with other packages, and so + they should be deprioritized for any search order (i.e. in PATH, or + for a set of -L entries in a compiler invocation). + """ + first = list(x for x in spec_container if not x.external) + second = list(x for x in spec_container if x.external) + return first + second + + def set_build_environment_variables(pkg, env, dirty): """Ensure a clean install environment when we build packages. @@ -323,6 +336,29 @@ def set_build_environment_variables(pkg, env, dirty): link_deps = set(pkg.spec.traverse(root=False, deptype=('link'))) build_link_deps = build_deps | link_deps rpath_deps = get_rpath_deps(pkg) + # This includes all build dependencies and any other dependencies that + # should be added to PATH (e.g. supporting executables run by build + # dependencies) + build_and_supporting_deps = set() + for build_dep in build_deps: + build_and_supporting_deps.update(build_dep.traverse(deptype='run')) + + # Establish an arbitrary but fixed ordering of specs so that resulting + # environment variable values are stable + def _order(specs): + return sorted(specs, key=lambda x: x.name) + + # External packages may be installed in a prefix which contains many other + # package installs. To avoid having those installations override + # Spack-installed packages, they are placed at the end of search paths. + # System prefixes are removed entirely later on since they are already + # searched. + build_deps = _place_externals_last(_order(build_deps)) + link_deps = _place_externals_last(_order(link_deps)) + build_link_deps = _place_externals_last(_order(build_link_deps)) + rpath_deps = _place_externals_last(_order(rpath_deps)) + build_and_supporting_deps = _place_externals_last( + _order(build_and_supporting_deps)) link_dirs = [] include_dirs = [] @@ -369,21 +405,10 @@ def set_build_environment_variables(pkg, env, dirty): 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] - build_link_prefixes = [dep.prefix for dep in build_link_deps] - - # add run-time dependencies of direct build-time dependencies: - for build_dep in build_deps: - for run_dep in build_dep.traverse(deptype='run'): - build_prefixes.append(run_dep.prefix) - - # Filter out system paths: ['/', '/usr', '/usr/local'] - # These paths can be introduced into the build when an external package - # is added as a dependency. The problem with these paths is that they often - # 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) - build_link_prefixes = filter_system_paths(build_link_prefixes) + build_and_supporting_prefixes = filter_system_paths( + x.prefix for x in build_and_supporting_deps) + build_link_prefixes = filter_system_paths( + x.prefix for x in build_link_deps) # Add dependencies to CMAKE_PREFIX_PATH env.set_path('CMAKE_PREFIX_PATH', build_link_prefixes) @@ -398,7 +423,10 @@ def set_build_environment_variables(pkg, env, dirty): env.set('SPACK_COMPILER_EXTRA_RPATHS', extra_rpaths) # Add bin directories from dependencies to the PATH for the build. - for prefix in build_prefixes: + # These directories are added to the beginning of the search path, and in + # the order given by 'build_and_supporting_prefixes' (the iteration order + # is reversed because each entry is prepended) + for prefix in reversed(build_and_supporting_prefixes): for dirname in ['bin', 'bin64']: bin_dir = os.path.join(prefix, dirname) if os.path.isdir(bin_dir): @@ -442,7 +470,7 @@ def set_build_environment_variables(pkg, env, dirty): env.set(SPACK_CCACHE_BINARY, ccache) # Add any pkgconfig directories to PKG_CONFIG_PATH - for prefix in build_link_prefixes: + for prefix in reversed(build_link_prefixes): for directory in ('lib', 'lib64', 'share'): pcdir = os.path.join(prefix, directory, 'pkgconfig') if os.path.isdir(pcdir): diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index 841a723f96..7aca1dea59 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -11,6 +11,7 @@ import spack.build_environment import spack.config import spack.spec +import spack.util.spack_yaml as syaml from spack.paths import build_env_path from spack.build_environment import dso_suffix, _static_to_shared_library from spack.build_environment import determine_number_of_jobs @@ -299,6 +300,45 @@ def normpaths(paths): delattr(dep_pkg, 'libs') +def test_external_prefixes_last(mutable_config, mock_packages, working_env, + monkeypatch): + # Sanity check: under normal circumstances paths associated with + # dt-diamond-left would appear first. We'll mark it as external in + # the test to check if the associated paths are placed last. + assert 'dt-diamond-left' < 'dt-diamond-right' + + cfg_data = syaml.load_config("""\ +dt-diamond-left: + externals: + - spec: dt-diamond-left@1.0 + prefix: /fake/path1 + buildable: false +""") + spack.config.set("packages", cfg_data) + top = spack.spec.Spec('dt-diamond').concretized() + + def _trust_me_its_a_dir(path): + return True + monkeypatch.setattr( + os.path, 'isdir', _trust_me_its_a_dir + ) + + env_mods = EnvironmentModifications() + spack.build_environment.set_build_environment_variables( + top.package, env_mods, False) + + env_mods.apply_modifications() + link_dir_var = os.environ['SPACK_LINK_DIRS'] + link_dirs = link_dir_var.split(':') + external_lib_paths = set(['/fake/path1/lib', '/fake/path1/lib64']) + # The external lib paths should be the last two entries of the list and + # should not appear anywhere before the last two entries + assert (set(os.path.normpath(x) for x in link_dirs[-2:]) == + external_lib_paths) + assert not (set(os.path.normpath(x) for x in link_dirs[:-2]) & + external_lib_paths) + + def test_parallel_false_is_not_propagating(config, mock_packages): class AttributeHolder(object): pass