diff options
| -rwxr-xr-x | lib/spack/env/cc | 8 | ||||
| -rw-r--r-- | lib/spack/llnl/util/filesystem.py | 48 | ||||
| -rw-r--r-- | lib/spack/spack/build_environment.py | 11 | ||||
| -rw-r--r-- | lib/spack/spack/test/build_environment.py | 30 | ||||
| -rw-r--r-- | lib/spack/spack/test/conftest.py | 29 | ||||
| -rw-r--r-- | lib/spack/spack/test/llnl/util/filesystem.py | 66 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/libxml2/package.py | 11 |
7 files changed, 174 insertions, 29 deletions
diff --git a/lib/spack/env/cc b/lib/spack/env/cc index 3354448765..699a590912 100755 --- a/lib/spack/env/cc +++ b/lib/spack/env/cc @@ -410,10 +410,10 @@ case "$mode" in ld|ccld) # Set extra RPATHs IFS=':' read -ra extra_rpaths <<< "$SPACK_COMPILER_EXTRA_RPATHS" - for extra_rpath in "${extra_rpaths[@]}"; do - $add_rpaths && rpaths+=("$extra_rpath") - libdirs+=("$extra_rpath") - done + libdirs+=("${extra_rpaths[@]}") + if [[ "$add_rpaths" != "false" ]] ; then + rpaths+=("${extra_rpaths[@]}") + fi # Add SPACK_LDLIBS to args for lib in "${SPACK_LDLIBS[@]}"; do diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index b00b1c78e2..01e9f540f0 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -36,6 +36,7 @@ __all__ = [ 'filter_file', 'find', 'find_headers', + 'find_all_headers', 'find_libraries', 'find_system_libraries', 'fix_darwin_install_name', @@ -956,10 +957,44 @@ class HeaderList(FileList): commonly used compiler flags or names. """ + include_regex = re.compile(r'(.*)(include)(.*)') + def __init__(self, files): super(HeaderList, self).__init__(files) self._macro_definitions = [] + self._directories = None + + @property + def directories(self): + """Directories to be searched for header files.""" + values = self._directories + if values is None: + values = self._default_directories() + return list(dedupe(values)) + + @directories.setter + def directories(self, value): + value = value or [] + # Accept a single directory as input + if isinstance(value, six.string_types): + value = [value] + + self._directories = [os.path.normpath(x) for x in value] + + def _default_directories(self): + """Default computation of directories based on the list of + header files. + """ + dir_list = super(HeaderList, self).directories + values = [] + for d in dir_list: + # If the path contains a subdirectory named 'include' then stop + # there and don't add anything else to the path. + m = self.include_regex.match(d) + value = os.path.join(*m.group(1, 2)) if m else d + values.append(value) + return values @property def headers(self): @@ -1095,6 +1130,19 @@ def find_headers(headers, root, recursive=False): return HeaderList(find(root, headers, recursive)) +def find_all_headers(root): + """Convenience function that returns the list of all headers found + in the directory passed as argument. + + Args: + root (path): directory where to look recursively for header files + + Returns: + List of all headers found in ``root`` and subdirectories. + """ + return find_headers('*', root=root, recursive=True) + + class LibraryList(FileList): """Sequence of absolute paths to libraries diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py index 68f1a11481..614188a281 100644 --- a/lib/spack/spack/build_environment.py +++ b/lib/spack/spack/build_environment.py @@ -292,13 +292,10 @@ def set_build_environment_variables(pkg, env, dirty): if dep in rpath_deps: rpath_dirs.extend(dep_link_dirs) - # TODO: fix the line below, currently the logic is broken for - # TODO: packages that uses directories as namespaces e.g. - # TODO: #include <boost/xxx.hpp> - # include_dirs.extend(query.headers.directories) - - if os.path.isdir(dep.prefix.include): - include_dirs.append(dep.prefix.include) + try: + include_dirs.extend(query.headers.directories) + except spack.spec.NoHeadersError: + tty.debug("No headers found for {0}".format(dep.name)) link_dirs = list(dedupe(filter_system_paths(link_dirs))) include_dirs = list(dedupe(filter_system_paths(include_dirs))) diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py index d2bf5f8f89..cc0cdad583 100644 --- a/lib/spack/spack/test/build_environment.py +++ b/lib/spack/spack/test/build_environment.py @@ -14,7 +14,7 @@ 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 +from llnl.util.filesystem import LibraryList @pytest.fixture @@ -221,7 +221,9 @@ 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, + installation_dir_with_headers +): """Check that build_environment supplies the needed library/include directories via the SPACK_LINK_DIRS and SPACK_INCLUDE_DIRS environment variables. @@ -238,15 +240,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_includes = HeaderList(dep2_inc_paths) + dep2_pkg.spec.prefix = str(installation_dir_with_headers) setattr(dep_pkg, 'libs', dep_libs) - setattr(dep2_pkg, 'headers', dep2_includes) try: pkg = root.package env_mods = EnvironmentModifications() @@ -271,11 +268,16 @@ def test_set_build_environment_variables( 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([str(dep2_include)])) + + # The default implementation looks for header files only + # in <prefix>/include and subdirectories + prefix = str(installation_dir_with_headers) + 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') diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 0e5a2ffad7..774de1cb2f 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -716,6 +716,35 @@ def mutable_mock_env_path(tmpdir_factory): spack.environment.env_path = saved_path +@pytest.fixture() +def installation_dir_with_headers(tmpdir_factory): + """Mock installation tree with a few headers placed in different + subdirectories. Shouldn't be modified by tests as it is session + scoped. + """ + 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 ########## diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index 1d64195a8d..4fc360ec4e 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -215,3 +215,69 @@ 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( + installation_dir_with_headers +): + # Try to inspect recursively from <prefix> and ensure we don't get + # subdirectories of the '<prefix>/include' path + prefix = str(installation_dir_with_headers) + header_list = fs.find_all_headers(prefix) + + # 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 + + +@pytest.mark.parametrize('list_of_headers,expected_directories', [ + (['/pfx/include/foo.h', '/pfx/include/subdir/foo.h'], ['/pfx/include']), + (['/pfx/include/foo.h', '/pfx/subdir/foo.h'], + ['/pfx/include', '/pfx/subdir']), + (['/pfx/include/subdir/foo.h', '/pfx/subdir/foo.h'], + ['/pfx/include', '/pfx/subdir']) +]) +def test_computation_of_header_directories( + list_of_headers, expected_directories +): + hl = fs.HeaderList(list_of_headers) + assert hl.directories == expected_directories + + +def test_headers_directory_setter(): + hl = fs.HeaderList( + ['/pfx/include/subdir/foo.h', '/pfx/include/subdir/bar.h'] + ) + + # Set directories using a list + hl.directories = ['/pfx/include/subdir'] + assert hl.directories == ['/pfx/include/subdir'] + + # If it's a single directory it's fine to not wrap it into a list + # when setting the property + hl.directories = '/pfx/include/subdir' + assert hl.directories == ['/pfx/include/subdir'] + + # Paths are normalized, so it doesn't matter how many backslashes etc. + # are present in the original directory being used + hl.directories = '/pfx/include//subdir/' + assert hl.directories == ['/pfx/include/subdir'] + + # Setting an empty list is allowed and returns an empty list + hl.directories = [] + assert hl.directories == [] + + # Setting directories to None also returns an empty list + hl.directories = None + assert hl.directories == [] diff --git a/var/spack/repos/builtin/packages/libxml2/package.py b/var/spack/repos/builtin/packages/libxml2/package.py index 3463ba6558..117a3eb810 100644 --- a/var/spack/repos/builtin/packages/libxml2/package.py +++ b/var/spack/repos/builtin/packages/libxml2/package.py @@ -36,6 +36,13 @@ class Libxml2(AutotoolsPackage): resource(name='xmlts', url='http://www.w3.org/XML/Test/xmlts20080827.tar.gz', sha256='96151685cec997e1f9f3387e3626d61e6284d4d6e66e0e440c209286c03e9cc7') + @property + def headers(self): + include_dir = self.spec.prefix.include.libxml2 + hl = find_all_headers(include_dir) + hl.directories = include_dir + return hl + def configure_args(self): spec = self.spec @@ -51,10 +58,6 @@ class Libxml2(AutotoolsPackage): return args - def setup_dependent_environment(self, spack_env, run_env, dependent_spec): - spack_env.prepend_path('CPATH', self.prefix.include.libxml2) - run_env.prepend_path('CPATH', self.prefix.include.libxml2) - @run_after('install') @on_package_attributes(run_tests=True) def import_module_test(self): |