Use gnuconfig package for config file replacement (#26035)

* Use gnuconfig package for config file replacement

Currently the autotools build system tries to pick up config.sub and
config.guess files from the system (in /usr/share) on arm and power.
This is introduces an implicit system dependency which we can avoid by
distributing config.guess and config.sub files in a separate package,
such as the new `gnuconfig` package which is very lightweight/text only
(unlike automake where we previously pulled these files from as a
backup). This PR adds `gnuconfig` as an unconditional build dependency
for arm and power archs.

In case the user needs a system version of config.sub and config.guess,
they are free to mark `gnuconfig` as an external package with the prefix
pointing to the directory containing the config files:

```yaml
    gnuconfig:
      externals:
      - spec: gnuconfig@master
        prefix: /tmp/tmp.ooBlkyAKdw/lol
      buildable: false
```

Apart from that, this PR gives some better instructions for users when
replacing config files goes wrong.

* Mock needs this package too now, because autotools adds a depends_on

* Add documentation

* Make patch_config_files a prop, fix the docs, add integrations tests

* Make macOS happy

lib/spack/docs/build_systems/autotoolspackage.rst

@@ -159,6 +159,57 @@ create a new patch that directly modifies ``configure``. That way,
 Spack can use the secondary patch and additional build system
 dependencies aren't necessary.
 
+""""""""""""""""""""""""""""
+Old Autotools helper scripts
+""""""""""""""""""""""""""""
+
+Autotools based tarballs come with helper scripts such as ``config.sub`` and
+``config.guess``. It is the responsibility of the developers to keep these files
+up to date so that they run on every platform, but for very old software
+releases this is impossible. In these cases Spack can help to replace these
+files with newer ones, without having to add the heavy dependency on
+``automake``.
+
+Automatic helper script replacement is currently enabled by default on
+``ppc64le`` and ``aarch64``, as these are the known cases where old scripts fail.
+On these targets, ``AutotoolsPackage`` adds a build dependency on ``gnuconfig``,
+which is a very light-weight package with newer versions of the helper files.
+Spack then tries to run all the helper scripts it can find in the release, and
+replaces them on failure with the helper scripts from ``gnuconfig``.
+
+To opt out of this feature, use the following setting:
+
+.. code-block:: python
+
+   patch_config_files = False
+
+To enable it conditionally on different architectures, define a property and
+make the package depend on ``gnuconfig`` as a build dependency:
+
+.. code-block
+
+   depends_on('gnuconfig', when='@1.0:')
+
+   @property
+   def patch_config_files(self):
+      return self.spec.satisfies("@1.0:")
+
+.. note::
+
+    On some exotic architectures it is necessary to use system provided
+    ``config.sub`` and ``config.guess`` files. In this case, the most
+    transparent solution is to mark the ``gnuconfig`` package as external and
+    non-buildable, with a prefix set to the directory containing the files:
+
+   .. code-block:: yaml
+
+       gnuconfig:
+         buildable: false
+         externals:
+         - spec: gnuconfig@master
+           prefix: /usr/share/configure_files/
+
+
 """"""""""""""""
 force_autoreconf
 """"""""""""""""

lib/spack/spack/build_systems/autotools.py

@@ -3,7 +3,6 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import inspect
-import itertools
 import os
 import os.path
 import stat
@@ -14,6 +13,8 @@
 import llnl.util.tty as tty
 from llnl.util.filesystem import force_remove, working_dir
 
+from spack.build_environment import InstallError
+from spack.directives import depends_on
 from spack.package import PackageBase, run_after, run_before
 from spack.util.executable import Executable
 
@@ -54,9 +55,22 @@ class AutotoolsPackage(PackageBase):
     #: This attribute is used in UI queries that need to know the build
     #: system base class
     build_system_class = 'AutotoolsPackage'
-    #: Whether or not to update ``config.guess`` and ``config.sub`` on old
+
-    #: architectures
+    @property
-    patch_config_files = True
+    def patch_config_files(self):
+        """
+        Whether or not to update old ``config.guess`` and ``config.sub`` files
+        distributed with the tarball. This currently only applies to ``ppc64le:``
+        and ``aarch64:`` target architectures. The substitutes are taken from the
+        ``gnuconfig`` package, which is automatically added as a build dependency
+        for these architectures. In case system versions of these config files are
+        required, the ``gnuconfig`` package can be marked external with a prefix
+        pointing to the directory containing the system ``config.guess`` and
+        ``config.sub`` files.
+        """
+        return (self.spec.satisfies('target=ppc64le:')
+                or self.spec.satisfies('target=aarch64:'))
+
     #: Whether or not to update ``libtool``
     #: (currently only for Arm/Clang/Fujitsu compilers)
     patch_libtool = True
@@ -83,6 +97,9 @@ class AutotoolsPackage(PackageBase):
     #: after the installation. If True instead it installs them.
     install_libtool_archives = False
 
+    depends_on('gnuconfig', type='build', when='target=ppc64le:')
+    depends_on('gnuconfig', type='build', when='target=aarch64:')
+
     @property
     def _removed_la_files_log(self):
         """File containing the list of remove libtool archives"""
@@ -106,10 +123,7 @@ def _do_patch_config_files(self):
         In particular, config.guess fails for PPC64LE for version prior
         to a 2013-06-10 build date (automake 1.13.4) and for ARM (aarch64).
         """
-        if not self.patch_config_files or (
+        if not self.patch_config_files:
-                not self.spec.satisfies('target=ppc64le:') and
-                not self.spec.satisfies('target=aarch64:')
-        ):
             return
 
         # TODO: Expand this to select the 'config.sub'-compatible architecture
@@ -138,39 +152,69 @@ def runs_ok(script_abs_path):
 
             return True
 
-        # Compute the list of files that needs to be patched
+        # Get the list of files that needs to be patched
-        search_dir = self.stage.path
+        to_be_patched = fs.find(self.stage.path, files=['config.sub', 'config.guess'])
-        to_be_patched = fs.find(
-            search_dir, files=['config.sub', 'config.guess'], recursive=True
-        )
         to_be_patched = [f for f in to_be_patched if not runs_ok(f)]
 
         # If there are no files to be patched, return early
         if not to_be_patched:
             return
 
-        # Directories where to search for files to be copied
+        # Otherwise, require `gnuconfig` to be a build dependency
-        # over the failing ones
+        self._require_build_deps(
-        good_file_dirs = ['/usr/share']
+            pkgs=['gnuconfig'],
-        if 'automake' in self.spec:
+            spec=self.spec,
-            good_file_dirs.insert(0, self.spec['automake'].prefix)
+            err="Cannot patch config files")
 
-        # List of files to be found in the directories above
+        # Get the config files we need to patch (config.sub / config.guess).
         to_be_found = list(set(os.path.basename(f) for f in to_be_patched))
-        substitutes = {}
+        gnuconfig = self.spec['gnuconfig']
-        for directory in good_file_dirs:
+        gnuconfig_dir = gnuconfig.prefix
-            candidates = fs.find(directory, files=to_be_found, recursive=True)
+
+        # An external gnuconfig may not not have a prefix.
+        if gnuconfig_dir is None:
+            raise InstallError("Spack could not find substitutes for GNU config "
+                               "files because no prefix is available for the "
+                               "`gnuconfig` package. Make sure you set a prefix "
+                               "path instead of modules for external `gnuconfig`.")
+
+        candidates = fs.find(gnuconfig_dir, files=to_be_found, recursive=False)
+
+        # For external packages the user may have specified an incorrect prefix.
+        # otherwise the installation is just corrupt.
+        if not candidates:
+            msg = ("Spack could not find `config.guess` and `config.sub` "
+                   "files in the `gnuconfig` prefix `{0}`. This means the "
+                   "`gnuconfig` package is broken").format(gnuconfig_dir)
+            if gnuconfig.external:
+                msg += (" or the `gnuconfig` package prefix is misconfigured as"
+                        " an external package")
+            raise InstallError(msg)
+
+        # Filter working substitutes
         candidates = [f for f in candidates if runs_ok(f)]
-            for name, good_files in itertools.groupby(
+        substitutes = {}
-                    candidates, key=os.path.basename
+        for candidate in candidates:
-            ):
+            config_file = os.path.basename(candidate)
-                substitutes[name] = next(good_files)
+            substitutes[config_file] = candidate
-                to_be_found.remove(name)
+            to_be_found.remove(config_file)
 
         # Check that we found everything we needed
         if to_be_found:
-            msg = 'Failed to find suitable substitutes for {0}'
+            msg = """\
-            raise RuntimeError(msg.format(', '.join(to_be_found)))
+Spack could not find working replacements for the following autotools config
+files: {0}.
+
+To resolve this problem, please try the following:
+1. Try to rebuild with `patch_config_files = False` in the package `{1}`, to
+   rule out that Spack tries to replace config files not used by the build.
+2. Verify that the `gnuconfig` package is up-to-date.
+3. On some systems you need to use system-provided `config.guess` and `config.sub`
+   files. In this case, mark `gnuconfig` as an non-buildable external package,
+   and set the prefix to the directory containing the `config.guess` and
+   `config.sub` files.
+"""
+            raise InstallError(msg.format(', '.join(to_be_found), self.name))
 
         # Copy the good files over the bad ones
         for abs_path in to_be_patched:
@@ -252,30 +296,40 @@ def delete_configure_to_force_update(self):
         if self.force_autoreconf:
             force_remove(self.configure_abs_path)
 
-    def _autoreconf_warning(self, spec, missing):
+    def _require_build_deps(self, pkgs, spec, err):
-        msg = ("Cannot generate configure: missing dependencies {0}.\n\nPlease add "
+        """Require `pkgs` to be direct build dependencies of `spec`. Raises a
-               "the following lines to the package:\n\n".format(", ".join(missing)))
+        RuntimeError with a helpful error messages when any dep is missing."""
 
-        for dep in missing:
+        build_deps = [d.name for d in spec.dependencies(deptype='build')]
+        missing_deps = [x for x in pkgs if x not in build_deps]
+
+        if not missing_deps:
+            return
+
+        # Raise an exception on missing deps.
+        msg = ("{0}: missing dependencies: {1}.\n\nPlease add "
+               "the following lines to the package:\n\n"
+               .format(err, ", ".join(missing_deps)))
+
+        for dep in missing_deps:
             msg += ("    depends_on('{0}', type='build', when='@{1}')\n"
                     .format(dep, spec.version))
 
         msg += "\nUpdate the version (when='@{0}') as needed.".format(spec.version)
-
+        raise RuntimeError(msg)
-        return msg
 
     def autoreconf(self, spec, prefix):
         """Not needed usually, configure should be already there"""
+
         # If configure exists nothing needs to be done
         if os.path.exists(self.configure_abs_path):
             return
-        # Else try to regenerate it
-        needed_dependencies = ['autoconf', 'automake', 'libtool']
-        build_deps = [d.name for d in spec.dependencies(deptype='build')]
-        missing = [x for x in needed_dependencies if x not in build_deps]
 
-        if missing:
+        # Else try to regenerate it, which reuquires a few build dependencies
-            raise RuntimeError(self._autoreconf_warning(spec, missing))
+        self._require_build_deps(
+            pkgs=['autoconf', 'automake', 'libtool'],
+            spec=spec,
+            err="Cannot generate configure")
 
         tty.msg('Configure script not found: trying to generate it')
         tty.warn('*********************************************************')

lib/spack/spack/test/build_systems.py

@@ -10,8 +10,10 @@
 
 import llnl.util.filesystem as fs
 
+import spack.architecture
+import spack.environment
 import spack.repo
-from spack.build_environment import get_std_cmake_args, setup_package
+from spack.build_environment import ChildError, get_std_cmake_args, setup_package
 from spack.spec import Spec
 from spack.util.executable import which
 
@@ -190,7 +192,7 @@ def test_libtool_archive_files_are_deleted_by_default(
             self, mutable_database
     ):
         # Install a package that creates a mock libtool archive
-        s = spack.spec.Spec('libtool-deletion')
+        s = Spec('libtool-deletion')
         s.concretize()
         s.package.do_install(explicit=True)
 
@@ -208,7 +210,7 @@ def test_libtool_archive_files_might_be_installed_on_demand(
     ):
         # Install a package that creates a mock libtool archive,
         # patch its package to preserve the installation
-        s = spack.spec.Spec('libtool-deletion')
+        s = Spec('libtool-deletion')
         s.concretize()
         monkeypatch.setattr(s.package, 'install_libtool_archives', True)
         s.package.do_install(explicit=True)
@@ -216,6 +218,89 @@ def test_libtool_archive_files_might_be_installed_on_demand(
         # Assert libtool archives are installed
         assert os.path.exists(s.package.libtool_archive_file)
 
+    def test_autotools_gnuconfig_replacement(self, mutable_database):
+        """
+        Tests whether only broken config.sub and config.guess are replaced with
+        files from working alternatives from the gnuconfig package.
+        """
+        s = Spec('autotools-config-replacement +patch_config_files +gnuconfig')
+        s.concretize()
+        s.package.do_install()
+
+        with open(os.path.join(s.prefix.broken, 'config.sub')) as f:
+            assert "gnuconfig version of config.sub" in f.read()
+
+        with open(os.path.join(s.prefix.broken, 'config.guess')) as f:
+            assert "gnuconfig version of config.guess" in f.read()
+
+        with open(os.path.join(s.prefix.working, 'config.sub')) as f:
+            assert "gnuconfig version of config.sub" not in f.read()
+
+        with open(os.path.join(s.prefix.working, 'config.guess')) as f:
+            assert "gnuconfig version of config.guess" not in f.read()
+
+    def test_autotools_gnuconfig_replacement_disabled(self, mutable_database):
+        """
+        Tests whether disabling patch_config_files
+        """
+        s = Spec('autotools-config-replacement ~patch_config_files +gnuconfig')
+        s.concretize()
+        s.package.do_install()
+
+        with open(os.path.join(s.prefix.broken, 'config.sub')) as f:
+            assert "gnuconfig version of config.sub" not in f.read()
+
+        with open(os.path.join(s.prefix.broken, 'config.guess')) as f:
+            assert "gnuconfig version of config.guess" not in f.read()
+
+        with open(os.path.join(s.prefix.working, 'config.sub')) as f:
+            assert "gnuconfig version of config.sub" not in f.read()
+
+        with open(os.path.join(s.prefix.working, 'config.guess')) as f:
+            assert "gnuconfig version of config.guess" not in f.read()
+
+    @pytest.mark.disable_clean_stage_check
+    def test_autotools_gnuconfig_replacement_no_gnuconfig(self, mutable_database):
+        """
+        Tests whether a useful error message is shown when patch_config_files is
+        enabled, but gnuconfig is not listed as a direct build dependency.
+        """
+        s = Spec('autotools-config-replacement +patch_config_files ~gnuconfig')
+        s.concretize()
+
+        msg = "Cannot patch config files: missing dependencies: gnuconfig"
+        with pytest.raises(ChildError, match=msg):
+            s.package.do_install()
+
+    @pytest.mark.disable_clean_stage_check
+    def test_broken_external_gnuconfig(self, mutable_database, tmpdir):
+        """
+        Tests whether we get a useful error message when gnuconfig is marked
+        external, but the install prefix is misconfigured and no config.guess
+        and config.sub substitute files are found in the provided prefix.
+        """
+        env_dir = str(tmpdir.ensure('env', dir=True))
+        gnuconfig_dir = str(tmpdir.ensure('gnuconfig', dir=True))  # empty dir
+        with open(os.path.join(env_dir, 'spack.yaml'), 'w') as f:
+            f.write("""\
+spack:
+  specs:
+  - 'autotools-config-replacement +patch_config_files +gnuconfig'
+  packages:
+    gnuconfig:
+      buildable: false
+      externals:
+      - spec: gnuconfig@1.0.0
+        prefix: {0}
+""".format(gnuconfig_dir))
+
+        msg = ("Spack could not find `config.guess`.*misconfigured as an "
+               "external package")
+        with spack.environment.Environment(env_dir) as e:
+            e.concretize()
+            with pytest.raises(ChildError, match=msg):
+                e.install_all()
+
 
 @pytest.mark.usefixtures('config', 'mock_packages')
 class TestCMakePackage(object):

var/spack/repos/builtin.mock/packages/autotools-config-replacement/package.py

@@ -0,0 +1,88 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import os
+
+from spack import *
+
+
+class AutotoolsConfigReplacement(AutotoolsPackage):
+    """
+    This package features broken and working config.sub and config.guess files,
+    that should be replaced by the ones provided by gnuconfig. It allows testing
+    with / without patches and with / without substitutes available.
+    """
+
+    has_code = False
+
+    version('1.0.0')
+    variant('patch_config_files', default=False)
+    variant('gnuconfig', default=False)
+
+    depends_on('gnuconfig', type='build', when='+gnuconfig')
+
+    @property
+    def patch_config_files(self):
+        return self.spec.satisfies('+patch_config_files')
+
+    def autoreconf(self, spec, prefix):
+        pass
+
+    def configure(self, spec, prefix):
+        pass
+
+    def build(self, spec, prefix):
+        pass
+
+    def install(self, spec, prefix):
+        broken = os.path.join(self.stage.source_path, 'broken')
+        working = os.path.join(self.stage.source_path, 'working')
+        install_tree(broken, self.prefix.broken)
+        install_tree(working, self.prefix.working)
+
+    @run_before('autoreconf')
+    def create_the_package_sources(self):
+        # Creates the following file structure:
+        # ./broken/config.sub    -- not executable
+        # ./broken/config.guess  -- exectuable & exit code 1
+        # ./working/config.sub   -- executable & exit code 0
+        # ./working/config.guess -- executable & exit code 0
+        # Automatic config helper script substitution should replace the two
+        # broken scripts with those from the gnuconfig package.
+
+        broken = os.path.join(self.stage.source_path, 'broken')
+        working = os.path.join(self.stage.source_path, 'working')
+
+        mkdirp(broken)
+        mkdirp(working)
+
+        # a configure script is required
+        configure_script = join_path(self.stage.source_path, 'configure')
+        with open(configure_script, 'w') as f:
+            f.write("#!/bin/sh\nexit 0")
+        os.chmod(configure_script, 0o775)
+
+        # broken config.sub (not executable)
+        broken_config_sub = join_path(broken, 'config.sub')
+        with open(broken_config_sub, 'w') as f:
+            f.write("#!/bin/sh\nexit 0")
+
+        # broken config.guess (exectuable but with error return code)
+        broken_config_guess = join_path(broken, 'config.guess')
+        with open(broken_config_guess, 'w') as f:
+            f.write("#!/bin/sh\nexit 1")
+        os.chmod(broken_config_guess, 0o775)
+
+        # working config.sub
+        working_config_sub = join_path(working, 'config.sub')
+        with open(working_config_sub, 'w') as f:
+            f.write("#!/bin/sh\nexit 0")
+        os.chmod(working_config_sub, 0o775)
+
+        # working config.guess
+        working_config_guess = join_path(working, 'config.guess')
+        with open(working_config_guess, 'w') as f:
+            f.write("#!/bin/sh\nexit 0")
+        os.chmod(working_config_guess, 0o775)

var/spack/repos/builtin.mock/packages/gnuconfig/package.py

@@ -0,0 +1,35 @@
+# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+
+import os
+
+from spack import *
+
+
+class Gnuconfig(Package):
+    """
+    The GNU config.guess and config.sub scripts versioned by timestamp.
+    This package can be used as a build dependency for autotools packages that
+    ship a tarball with outdated config.guess and config.sub files.
+    """
+
+    has_code = False
+
+    version('2021-08-14')
+
+    def install(self, spec, prefix):
+        config_sub = join_path(prefix, 'config.sub')
+        config_guess = join_path(prefix, 'config.guess')
+
+        # Create files
+        with open(config_sub, 'w') as f:
+            f.write("#!/bin/sh\necho gnuconfig version of config.sub")
+
+        with open(config_guess, 'w') as f:
+            f.write("#!/bin/sh\necho gnuconfig version of config.guess")
+
+        # Make executable
+        os.chmod(config_sub, 0o775)
+        os.chmod(config_guess, 0o775)