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 <prefix>` 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.

2 files changed, 99 insertions, 6 deletions

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
+    ]