Use Package.headers for -I options (#10623)
This restores the use of Package.headers when computing -I options for building a package that was added in #8136 and reverted in #10604. #8136 used utility logic that located all header files in an installation prefix, and calculated the -I options as the immediate roots containing those header files. In some cases, for a package containing a directory structure like prefix/ include/ ex1.h subdir/ ex2.h dependents may expect to include ex2.h relative to 'include', and adding 'prefix/include/subdir' as a -I was causing errors, in particular if ex2.h has the same name as a system header. This updates header utility logic to by default return the base "include" directory when it exists, rather than subdirectories. It also makes it possible for package implementers to override Package.headers to return the subdirectory when it is required (for example with libxml2).
This commit is contained in:
parent
ad25e7f3b0
commit
42386dbe94
8
lib/spack/env/cc
vendored
8
lib/spack/env/cc
vendored
@ -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
|
||||
|
@ -36,6 +36,7 @@
|
||||
'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
|
||||
|
||||
|
@ -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)))
|
||||
|
@ -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
|
||||
@ -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 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([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')
|
||||
|
@ -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
|
||||
##########
|
||||
|
@ -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 == []
|
||||
|
@ -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 @@ def configure_args(self):
|
||||
|
||||
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):
|
||||
|
Loading…
Reference in New Issue
Block a user