From 799553d302a16804d541524b6f56b5b2adef891d Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Fri, 22 Jul 2022 15:11:38 +0200 Subject: autotools: add -I flag when non-standard libtool (#31677) When 1. Spack installs libtool, 2. system libtool is installed too, and 3. system automake is used Spack passes system automake's `-I ` flag to itself, even though it's a default search path. This takes precedence over spack's libtool prefix dir. This causes the wrong `libtool.m4` file to be used (since system libtool is in the same prefix as system automake). And that leads to error messages about incompatible libtool, something something LT_INIT. --- lib/spack/spack/build_systems/autotools.py | 40 +++++++++++++++--- lib/spack/spack/test/build_systems.py | 65 ++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 6 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index 6909fda1b4..bf972bcf8b 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -359,12 +359,11 @@ To resolve this problem, please try the following: @property def autoreconf_search_path_args(self): - """Arguments to autoreconf to modify the search paths""" - search_path_args = [] - for dep in self.spec.dependencies(deptype='build'): - if os.path.exists(dep.prefix.share.aclocal): - search_path_args.extend(['-I', dep.prefix.share.aclocal]) - return search_path_args + """Search path includes for autoreconf. Add an -I flag for all `aclocal` dirs + of build deps, skips the default path of automake, move external include + flags to the back, since they might pull in unrelated m4 files shadowing + spack dependencies.""" + return _autoreconf_search_path_args(self.spec) @run_after('autoreconf') def set_configure_or_die(self): @@ -668,3 +667,32 @@ To resolve this problem, please try the following: # On macOS, force rpaths for shared library IDs and remove duplicate rpaths run_after('install')(PackageBase.apply_macos_rpath_fixups) + + +def _autoreconf_search_path_args(spec): + dirs_seen = set() + flags_spack, flags_external = [], [] + + # We don't want to add an include flag for automake's default search path. + for automake in spec.dependencies(name='automake', deptype='build'): + try: + s = os.stat(automake.prefix.share.aclocal) + if stat.S_ISDIR(s.st_mode): + dirs_seen.add((s.st_ino, s.st_dev)) + except OSError: + pass + + for dep in spec.dependencies(deptype='build'): + path = dep.prefix.share.aclocal + # Skip non-existing aclocal paths + try: + s = os.stat(path) + except OSError: + continue + # Skip things seen before, as well as non-dirs. + if (s.st_ino, s.st_dev) in dirs_seen or not stat.S_ISDIR(s.st_mode): + continue + dirs_seen.add((s.st_ino, s.st_dev)) + flags = flags_external if dep.external else flags_spack + flags.extend(['-I', path]) + return flags_spack + flags_external diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py index c59e28a180..3158696666 100644 --- a/lib/spack/spack/test/build_systems.py +++ b/lib/spack/spack/test/build_systems.py @@ -12,6 +12,7 @@ import pytest import llnl.util.filesystem as fs +import spack.build_systems.autotools import spack.environment import spack.platforms import spack.repo @@ -353,3 +354,67 @@ def test_autotools_args_from_conditional_variant(config, mock_packages): s = Spec('autotools-conditional-variants-test').concretized() assert 'example' not in s.variants assert len(s.package._activate_or_not('example', 'enable', 'disable')) == 0 + + +def test_autoreconf_search_path_args_multiple(config, mock_packages, tmpdir): + """autoreconf should receive the right -I flags with search paths for m4 files + for build deps.""" + spec = Spec('dttop').concretized() + aclocal_fst = str(tmpdir.mkdir("fst").mkdir("share").mkdir("aclocal")) + aclocal_snd = str(tmpdir.mkdir("snd").mkdir("share").mkdir("aclocal")) + build_dep_one, build_dep_two = spec.dependencies(deptype='build') + build_dep_one.prefix = str(tmpdir.join("fst")) + build_dep_two.prefix = str(tmpdir.join("snd")) + assert spack.build_systems.autotools._autoreconf_search_path_args(spec) == [ + '-I', aclocal_fst, '-I', aclocal_snd + ] + + +def test_autoreconf_search_path_args_skip_automake(config, mock_packages, tmpdir): + """automake's aclocal dir should not be added as -I flag as it is a default + 3rd party dir search path, and if it's a system version it usually includes + m4 files shadowing spack deps.""" + spec = Spec('dttop').concretized() + tmpdir.mkdir("fst").mkdir("share").mkdir("aclocal") + aclocal_snd = str(tmpdir.mkdir("snd").mkdir("share").mkdir("aclocal")) + build_dep_one, build_dep_two = spec.dependencies(deptype='build') + build_dep_one.name = 'automake' + build_dep_one.prefix = str(tmpdir.join("fst")) + build_dep_two.prefix = str(tmpdir.join("snd")) + assert spack.build_systems.autotools._autoreconf_search_path_args(spec) == [ + '-I', aclocal_snd + ] + + +def test_autoreconf_search_path_args_external_order(config, mock_packages, tmpdir): + """When a build dep is external, its -I flag should occur last""" + spec = Spec('dttop').concretized() + aclocal_fst = str(tmpdir.mkdir("fst").mkdir("share").mkdir("aclocal")) + aclocal_snd = str(tmpdir.mkdir("snd").mkdir("share").mkdir("aclocal")) + build_dep_one, build_dep_two = spec.dependencies(deptype='build') + build_dep_one.external_path = str(tmpdir.join("fst")) + build_dep_two.prefix = str(tmpdir.join("snd")) + assert spack.build_systems.autotools._autoreconf_search_path_args(spec) == [ + '-I', aclocal_snd, '-I', aclocal_fst + ] + + +def test_autoreconf_search_path_skip_nonexisting(config, mock_packages, tmpdir): + """Skip -I flags for non-existing directories""" + spec = Spec('dttop').concretized() + build_dep_one, build_dep_two = spec.dependencies(deptype='build') + build_dep_one.prefix = str(tmpdir.join("fst")) + build_dep_two.prefix = str(tmpdir.join("snd")) + assert spack.build_systems.autotools._autoreconf_search_path_args(spec) == [] + + +def test_autoreconf_search_path_dont_repeat(config, mock_packages, tmpdir): + """Do not add the same -I flag twice to keep things readable for humans""" + spec = Spec('dttop').concretized() + aclocal = str(tmpdir.mkdir("prefix").mkdir("share").mkdir("aclocal")) + build_dep_one, build_dep_two = spec.dependencies(deptype='build') + build_dep_one.external_path = str(tmpdir.join("prefix")) + build_dep_two.external_path = str(tmpdir.join("prefix")) + assert spack.build_systems.autotools._autoreconf_search_path_args(spec) == [ + '-I', aclocal + ] -- cgit v1.2.3-60-g2f50