Skip to content

Commit

Permalink
'prefix/include' is not added to the include directories unconditionally
Browse files Browse the repository at this point in the history
Added unit tests to check the default behavior for packages and custom
search behavior when header files are not under <prefix>/include
  • Loading branch information
alalazo committed Feb 18, 2019
1 parent 7e6e43c commit 09de4a0
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 18 deletions.
3 changes: 0 additions & 3 deletions lib/spack/spack/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,6 @@ def set_build_environment_variables(pkg, env, dirty):
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)

link_dirs = list(dedupe(filter_system_paths(link_dirs)))
include_dirs = list(dedupe(filter_system_paths(include_dirs)))
rpath_dirs = list(dedupe(filter_system_paths(rpath_dirs)))
Expand Down
29 changes: 14 additions & 15 deletions lib/spack/spack/test/build_environment.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from spack.util.spack_yaml import syaml_dict, syaml_str
from spack.util.environment import EnvironmentModifications

from llnl.util.filesystem import LibraryList, HeaderList
from llnl.util.filesystem import LibraryList


@pytest.fixture
Expand Down Expand Up @@ -221,7 +221,7 @@ def test_package_inheritance_module_setup(config, mock_packages):


def test_set_build_environment_variables(
config, mock_packages, working_env, monkeypatch, tmpdir_factory):
config, mock_packages, working_env, monkeypatch, tmp_installation_dir):
"""Check that build_environment supplies the needed library/include
directories via the SPACK_LINK_DIRS and SPACK_INCLUDE_DIRS environment
variables.
Expand All @@ -238,16 +238,10 @@ def test_set_build_environment_variables(
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)
dep2_pkg.spec.prefix = str(tmp_installation_dir)

setattr(dep_pkg, 'libs', dep_libs)
setattr(dep2_pkg, 'headers', dep2_includes)
try:
pkg = root.package
env_mods = EnvironmentModifications()
Expand All @@ -272,11 +266,16 @@ def normpaths(paths):
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)]))

# The default implementation looks for header files only
# in <prefix>/include and subdirectories
prefix = str(tmp_installation_dir)
include_dirs = normpaths(header_dir_var.split(':'))

assert os.path.join(prefix, 'include') in include_dirs
assert os.path.join(prefix, 'include', 'boost') not in include_dirs
assert os.path.join(prefix, 'path', 'to') not in include_dirs
assert os.path.join(prefix, 'path', 'to', 'subdir') not in include_dirs

finally:
delattr(dep_pkg, 'libs')
delattr(dep2_pkg, 'headers')
25 changes: 25 additions & 0 deletions lib/spack/spack/test/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,31 @@ def mutable_mock_env_path(tmpdir_factory):
spack.environment.env_path = saved_path


@pytest.fixture(scope='session')
def tmp_installation_dir(tmpdir_factory):
root = tmpdir_factory.mktemp('prefix')

# Create a few header files:
#
# <prefix>
# ├── include
# │   ├── boost
# │   │   └── ex3.h
# │   └── ex3.h
# └── path
# └── to
# ├── ex1.h
# └── subdir
# └── ex2.h
#
root.ensure('include', 'boost', 'ex3.h')
root.ensure('include', 'ex3.h')
root.ensure('path', 'to', 'ex1.h')
root.ensure('path', 'to', 'subdir', 'ex2.h')

return root


##########
# Mock packages
##########
Expand Down
22 changes: 22 additions & 0 deletions lib/spack/spack/test/llnl/util/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,25 @@ def test_move_transaction_rollback(tmpdir):
pass

assert h == fs.hash_directory(str(tmpdir))


@pytest.mark.regression('10601')
@pytest.mark.regression('10603')
def test_recursive_search_of_headers_from_prefix(tmp_installation_dir):
# Try to inspect recursively from <prefix> and ensure we don't get
# subdirectories of the '<prefix>/include' path
prefix = str(tmp_installation_dir)
header_list = fs.find_headers('*', root=prefix, recursive=True)

# Check that the header files we expect are all listed
assert os.path.join(prefix, 'include', 'ex3.h') in header_list
assert os.path.join(prefix, 'include', 'boost', 'ex3.h') in header_list
assert os.path.join(prefix, 'path', 'to', 'ex1.h') in header_list
assert os.path.join(prefix, 'path', 'to', 'subdir', 'ex2.h') in header_list

# Check that when computing directories we exclude <prefix>/include/boost
include_dirs = header_list.directories
assert os.path.join(prefix, 'include') in include_dirs
assert os.path.join(prefix, 'include', 'boost') not in include_dirs
assert os.path.join(prefix, 'path', 'to') in include_dirs
assert os.path.join(prefix, 'path', 'to', 'subdir') in include_dirs

0 comments on commit 09de4a0

Please sign in to comment.