From 87450f3688d5d4799bb08526f82061ab4b9cb944 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Tue, 28 Sep 2021 00:38:14 +0200 Subject: 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 | 51 ++++++++ lib/spack/spack/build_systems/autotools.py | 136 +++++++++++++++------- lib/spack/spack/test/build_systems.py | 91 ++++++++++++++- 3 files changed, 234 insertions(+), 44 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/build_systems/autotoolspackage.rst b/lib/spack/docs/build_systems/autotoolspackage.rst index 0cb87a58eb..71d8d7d866 100644 --- a/lib/spack/docs/build_systems/autotoolspackage.rst +++ b/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 """""""""""""""" diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index 66b5379975..9a00764e22 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/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.filesystem as fs 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 - patch_config_files = True + + @property + 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 @@ class AutotoolsPackage(PackageBase): 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 ( - not self.spec.satisfies('target=ppc64le:') and - not self.spec.satisfies('target=aarch64:') - ): + if not self.patch_config_files: return # TODO: Expand this to select the 'config.sub'-compatible architecture @@ -138,39 +152,69 @@ class AutotoolsPackage(PackageBase): return True - # Compute the list of files that needs to be patched - search_dir = self.stage.path - to_be_patched = fs.find( - search_dir, files=['config.sub', 'config.guess'], recursive=True - ) + # Get the list of files that needs to be patched + to_be_patched = fs.find(self.stage.path, files=['config.sub', 'config.guess']) 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 - # over the failing ones - good_file_dirs = ['/usr/share'] - if 'automake' in self.spec: - good_file_dirs.insert(0, self.spec['automake'].prefix) + # Otherwise, require `gnuconfig` to be a build dependency + self._require_build_deps( + pkgs=['gnuconfig'], + spec=self.spec, + 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)) + gnuconfig = self.spec['gnuconfig'] + gnuconfig_dir = gnuconfig.prefix + + # 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)] substitutes = {} - for directory in good_file_dirs: - candidates = fs.find(directory, files=to_be_found, recursive=True) - candidates = [f for f in candidates if runs_ok(f)] - for name, good_files in itertools.groupby( - candidates, key=os.path.basename - ): - substitutes[name] = next(good_files) - to_be_found.remove(name) + for candidate in candidates: + config_file = os.path.basename(candidate) + substitutes[config_file] = candidate + 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}' - raise RuntimeError(msg.format(', '.join(to_be_found))) + msg = """\ +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 @@ class AutotoolsPackage(PackageBase): if self.force_autoreconf: force_remove(self.configure_abs_path) - def _autoreconf_warning(self, spec, missing): - msg = ("Cannot generate configure: missing dependencies {0}.\n\nPlease add " - "the following lines to the package:\n\n".format(", ".join(missing))) + def _require_build_deps(self, pkgs, spec, err): + """Require `pkgs` to be direct build dependencies of `spec`. Raises a + 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) - - return msg + raise RuntimeError(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: - raise RuntimeError(self._autoreconf_warning(spec, missing)) + # Else try to regenerate it, which reuquires a few build dependencies + 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('*********************************************************') diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py index 4675d37c38..cd42e7b4e2 100644 --- a/lib/spack/spack/test/build_systems.py +++ b/lib/spack/spack/test/build_systems.py @@ -10,8 +10,10 @@ import pytest 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 @@ class TestAutotoolsPackage(object): 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 @@ class TestAutotoolsPackage(object): ): # 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 @@ class TestAutotoolsPackage(object): # 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): -- cgit v1.2.3-60-g2f50