From b84812256dae57d4bccbe46d7f7956585f57c0d6 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 13 Oct 2020 18:15:48 +0200 Subject: autotools: add attribute to delete libtool archives .la files (#18850) * autotools: add attribute to delete libtool archives .la files According to Autotools Mythbuster (https://autotools.io/libtool/lafiles.html) libtool archive files are mostly vestigial, but they might create issues when relocating binary packages as shown in #18694. For GCC specifically, most distributions remove these files with explicit commands: https://git.stg.centos.org/rpms/gcc/blob/master/f/gcc.spec#_1303 Considered all of that, this commit adds an easy way for each AutotoolsPackage to remove every .la file that has been installed. The default, for the time being, is to maintain them - to be consistent with what Spack was doing previously. * autotools: delete libtool archive files by default Following review this commit changes the default for libtool archive files deletion and adds test to verify the behavior. --- lib/spack/spack/build_systems/autotools.py | 35 +++++++++++++++++-- lib/spack/spack/test/build_systems.py | 40 +++++++++++++++++++--- .../packages/libtool-deletion/package.py | 35 +++++++++++++++++++ 3 files changed, 102 insertions(+), 8 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/libtool-deletion/package.py diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index e4b9f566ae..21ac535b32 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -2,8 +2,6 @@ # Spack Project Developers. See the top-level COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) - - import inspect import os import os.path @@ -80,10 +78,25 @@ class AutotoolsPackage(PackageBase): #: Options to be passed to autoreconf when using the default implementation autoreconf_extra_args = [] + #: If False deletes all the .la files in the prefix folder + #: after the installation. If True instead it installs them. + install_libtool_archives = False + + @property + def _removed_la_files_log(self): + """File containing the list of remove libtool archives""" + build_dir = self.build_directory + if not os.path.isabs(self.build_directory): + build_dir = os.path.join(self.stage.path, build_dir) + return os.path.join(build_dir, 'removed_la_files.txt') + @property def archive_files(self): """Files to archive for packages based on autotools""" - return [os.path.join(self.build_directory, 'config.log')] + files = [os.path.join(self.build_directory, 'config.log')] + if not self.install_libtool_archives: + files.append(self._removed_la_files_log) + return files @run_after('autoreconf') def _do_patch_config_files(self): @@ -534,3 +547,19 @@ class AutotoolsPackage(PackageBase): # Check that self.prefix is there after installation run_after('install')(PackageBase.sanity_check_prefix) + + @run_after('install') + def remove_libtool_archives(self): + """Remove all .la files in prefix sub-folders if the package sets + ``install_libtool_archives`` to be False. + """ + # If .la files are to be installed there's nothing to do + if self.install_libtool_archives: + return + + # Remove the files and create a log of what was removed + libtool_files = fs.find(str(self.prefix), '*.la', recursive=True) + with fs.safe_remove(*libtool_files): + fs.mkdirp(os.path.dirname(self._removed_la_files_log)) + with open(self._removed_la_files_log, mode='w') as f: + f.write('\n'.join(libtool_files)) diff --git a/lib/spack/spack/test/build_systems.py b/lib/spack/spack/test/build_systems.py index 367faeef66..1606ba8ebd 100644 --- a/lib/spack/spack/test/build_systems.py +++ b/lib/spack/spack/test/build_systems.py @@ -7,8 +7,8 @@ import glob import os import pytest +import llnl.util.filesystem as fs import spack.repo -from llnl.util.filesystem import working_dir from spack.build_environment import get_std_cmake_args, setup_package from spack.spec import Spec from spack.util.executable import which @@ -30,7 +30,7 @@ def test_affirmative_make_check(directory, config, mock_packages, working_env): pkg = spack.repo.get(s) setup_package(pkg, False) - with working_dir(directory): + with fs.working_dir(directory): assert pkg._has_make_target('check') pkg._if_make_target_execute('check') @@ -50,7 +50,7 @@ def test_negative_make_check(directory, config, mock_packages, working_env): pkg = spack.repo.get(s) setup_package(pkg, False) - with working_dir(directory): + with fs.working_dir(directory): assert not pkg._has_make_target('check') pkg._if_make_target_execute('check') @@ -71,7 +71,7 @@ def test_affirmative_ninja_check( pkg = spack.repo.get(s) setup_package(pkg, False) - with working_dir(directory): + with fs.working_dir(directory): assert pkg._has_ninja_target('check') pkg._if_ninja_target_execute('check') @@ -96,7 +96,7 @@ def test_negative_ninja_check(directory, config, mock_packages, working_env): pkg = spack.repo.get(s) setup_package(pkg, False) - with working_dir(directory): + with fs.working_dir(directory): assert not pkg._has_ninja_target('check') pkg._if_ninja_target_execute('check') @@ -182,6 +182,36 @@ class TestAutotoolsPackage(object): assert '--without-baz' in options assert '--no-fee' in options + 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.concretize() + s.package.do_install(explicit=True) + + # Assert the libtool archive is not there and we have + # a log of removed files + assert not os.path.exists(s.package.libtool_archive_file) + search_directory = os.path.join(s.prefix, '.spack') + libtool_deletion_log = fs.find( + search_directory, 'removed_la_files.txt', recursive=True + ) + assert libtool_deletion_log + + def test_libtool_archive_files_might_be_installed_on_demand( + self, mutable_database, monkeypatch + ): + # Install a package that creates a mock libtool archive, + # patch its package to preserve the installation + s = spack.spec.Spec('libtool-deletion') + s.concretize() + monkeypatch.setattr(s.package, 'install_libtool_archives', True) + s.package.do_install(explicit=True) + + # Assert libtool archives are installed + assert os.path.exists(s.package.libtool_archive_file) + @pytest.mark.usefixtures('config', 'mock_packages') class TestCMakePackage(object): diff --git a/var/spack/repos/builtin.mock/packages/libtool-deletion/package.py b/var/spack/repos/builtin.mock/packages/libtool-deletion/package.py new file mode 100644 index 0000000000..6ca8ce03d8 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/libtool-deletion/package.py @@ -0,0 +1,35 @@ +# Copyright 2013-2020 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.path + + +class LibtoolDeletion(AutotoolsPackage): + """Mock AutotoolsPackage to check proper deletion + of libtool archives. + """ + homepage = "https://www.gnu.org/software/make/" + url = "http://www.example.com/libtool-deletion-1.0.tar.gz" + version('4.2.1', sha256='e40b8f018c1da64edd1cc9a6fce5fa63b2e707e404e20cad91fbae337c98a5b7') + + def do_stage(self): + mkdirp(self.stage.source_path) + + def autoreconf(self, spec, prefix): + mkdirp(os.path.dirname(self.configure_abs_path)) + touch(self.configure_abs_path) + + def configure(self, spec, prefix): + pass + + def build(self, spec, prefix): + pass + + def install(self, spec, prefix): + mkdirp(os.path.dirname(self.libtool_archive_file)) + touch(self.libtool_archive_file) + + @property + def libtool_archive_file(self): + return os.path.join(str(self.prefix.lib), 'libfoo.la') -- cgit v1.2.3-70-g09d2