diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index d5c508372e8..5eefb95466c 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -2125,10 +2125,9 @@ def fetch_url_to_mirror(url): def dedupe_hardlinks_if_necessary(root, buildinfo): - """Updates a buildinfo dict for old archives that did - not dedupe hardlinks. De-duping hardlinks is necessary - when relocating files in parallel and in-place. This - means we must preserve inodes when relocating.""" + """Updates a buildinfo dict for old archives that did not dedupe hardlinks. De-duping hardlinks + is necessary when relocating files in parallel and in-place. This means we must preserve inodes + when relocating.""" # New archives don't need this. if buildinfo.get("hardlinks_deduped", False): @@ -2157,69 +2156,46 @@ def dedupe_hardlinks_if_necessary(root, buildinfo): buildinfo[key] = new_list -def relocate_package(spec): - """ - Relocate the given package - """ - workdir = str(spec.prefix) - buildinfo = read_buildinfo_file(workdir) - new_layout_root = str(spack.store.STORE.layout.root) - new_prefix = str(spec.prefix) - new_rel_prefix = str(os.path.relpath(new_prefix, new_layout_root)) - new_spack_prefix = str(spack.paths.prefix) - - old_sbang_install_path = None - if "sbang_install_path" in buildinfo: - old_sbang_install_path = str(buildinfo["sbang_install_path"]) +def relocate_package(spec: spack.spec.Spec) -> None: + """Relocate binaries and text files in the given spec prefix, based on its buildinfo file.""" + buildinfo = read_buildinfo_file(spec.prefix) old_layout_root = str(buildinfo["buildpath"]) - old_spack_prefix = str(buildinfo.get("spackprefix")) - old_rel_prefix = buildinfo.get("relative_prefix") - old_prefix = os.path.join(old_layout_root, old_rel_prefix) - # Warn about old style tarballs created with the now removed --rel flag. + # Warn about old style tarballs created with the --rel flag (removed in Spack v0.20) if buildinfo.get("relative_rpaths", False): tty.warn( - f"Tarball for {spec} uses relative rpaths, " "which can cause library loading issues." + f"Tarball for {spec} uses relative rpaths, which can cause library loading issues." ) - # In the past prefix_to_hash was the default and externals were not dropped, so prefixes - # were not unique. + # In Spack 0.19 and older prefix_to_hash was the default and externals were not dropped, so + # prefixes were not unique. if "hash_to_prefix" in buildinfo: hash_to_old_prefix = buildinfo["hash_to_prefix"] elif "prefix_to_hash" in buildinfo: - hash_to_old_prefix = dict((v, k) for (k, v) in buildinfo["prefix_to_hash"].items()) + hash_to_old_prefix = {v: k for (k, v) in buildinfo["prefix_to_hash"].items()} else: - hash_to_old_prefix = dict() + raise NewLayoutException( + "Package tarball was created from an install prefix with a different directory layout " + "and an older buildcache create implementation. It cannot be relocated." + ) - if old_rel_prefix != new_rel_prefix and not hash_to_old_prefix: - msg = "Package tarball was created from an install " - msg += "prefix with a different directory layout and an older " - msg += "buildcache create implementation. It cannot be relocated." - raise NewLayoutException(msg) + prefix_to_prefix = {} - # Spurious replacements (e.g. sbang) will cause issues with binaries - # For example, the new sbang can be longer than the old one. - # Hence 2 dictionaries are maintained here. - prefix_to_prefix_text = collections.OrderedDict() - prefix_to_prefix_bin = collections.OrderedDict() + if "sbang_install_path" in buildinfo: + old_sbang_install_path = str(buildinfo["sbang_install_path"]) + prefix_to_prefix[old_sbang_install_path] = spack.hooks.sbang.sbang_install_path() - if old_sbang_install_path: - install_path = spack.hooks.sbang.sbang_install_path() - prefix_to_prefix_text[old_sbang_install_path] = install_path + # First match specific prefix paths. Possibly the *local* install prefix of some dependency is + # in an upstream, so we cannot assume the original spack store root can be mapped uniformly to + # the new spack store root. - # First match specific prefix paths. Possibly the *local* install prefix - # of some dependency is in an upstream, so we cannot assume the original - # spack store root can be mapped uniformly to the new spack store root. - # - # If the spec is spliced, we need to handle the simultaneous mapping - # from the old install_tree to the new install_tree and from the build_spec - # to the spliced spec. - # Because foo.build_spec is foo for any non-spliced spec, we can simplify - # by checking for spliced-in nodes by checking for nodes not in the build_spec - # without any explicit check for whether the spec is spliced. - # An analog in this algorithm is any spec that shares a name or provides the same virtuals - # in the context of the relevant root spec. This ensures that the analog for a spec s - # is the spec that s replaced when we spliced. + # If the spec is spliced, we need to handle the simultaneous mapping from the old install_tree + # to the new install_tree and from the build_spec to the spliced spec. Because foo.build_spec + # is foo for any non-spliced spec, we can simplify by checking for spliced-in nodes by checking + # for nodes not in the build_spec without any explicit check for whether the spec is spliced. + # An analog in this algorithm is any spec that shares a name or provides the same virtuals in + # the context of the relevant root spec. This ensures that the analog for a spec s is the spec + # that s replaced when we spliced. relocation_specs = specs_to_relocate(spec) build_spec_ids = set(id(s) for s in spec.build_spec.traverse(deptype=dt.ALL & ~dt.BUILD)) for s in relocation_specs: @@ -2239,72 +2215,48 @@ def relocate_package(spec): lookup_dag_hash = analog.dag_hash() if lookup_dag_hash in hash_to_old_prefix: old_dep_prefix = hash_to_old_prefix[lookup_dag_hash] - prefix_to_prefix_bin[old_dep_prefix] = str(s.prefix) - prefix_to_prefix_text[old_dep_prefix] = str(s.prefix) + prefix_to_prefix[old_dep_prefix] = str(s.prefix) # Only then add the generic fallback of install prefix -> install prefix. - prefix_to_prefix_text[old_prefix] = new_prefix - prefix_to_prefix_bin[old_prefix] = new_prefix - prefix_to_prefix_text[old_layout_root] = new_layout_root - prefix_to_prefix_bin[old_layout_root] = new_layout_root + prefix_to_prefix[old_layout_root] = str(spack.store.STORE.layout.root) - # This is vestigial code for the *old* location of sbang. Previously, - # sbang was a bash script, and it lived in the spack prefix. It is - # now a POSIX script that lives in the install prefix. Old packages - # will have the old sbang location in their shebangs. - orig_sbang = "#!/bin/bash {0}/bin/sbang".format(old_spack_prefix) - new_sbang = spack.hooks.sbang.sbang_shebang_line() - prefix_to_prefix_text[orig_sbang] = new_sbang + # Delete identity mappings from prefix_to_prefix + prefix_to_prefix = {k: v for k, v in prefix_to_prefix.items() if k != v} - tty.debug("Relocating package from", "%s to %s." % (old_layout_root, new_layout_root)) + # If there's nothing to relocate, we're done. + if not prefix_to_prefix: + return + + for old, new in prefix_to_prefix.items(): + tty.debug(f"Relocating: {old} => {new}.") # Old archives may have hardlinks repeated. - dedupe_hardlinks_if_necessary(workdir, buildinfo) + dedupe_hardlinks_if_necessary(spec.prefix, buildinfo) # Text files containing the prefix text - text_names = [os.path.join(workdir, f) for f in buildinfo["relocate_textfiles"]] + textfiles = [os.path.join(spec.prefix, f) for f in buildinfo["relocate_textfiles"]] + binaries = [os.path.join(spec.prefix, f) for f in buildinfo.get("relocate_binaries")] + links = [os.path.join(spec.prefix, f) for f in buildinfo.get("relocate_links", [])] - # If we are not installing back to the same install tree do the relocation - if old_prefix != new_prefix: - files_to_relocate = [ - os.path.join(workdir, filename) for filename in buildinfo.get("relocate_binaries") - ] - # If the buildcache was not created with relativized rpaths - # do the relocation of path in binaries - platform = spack.platforms.by_name(spec.platform) - if "macho" in platform.binary_formats: - relocate.relocate_macho_binaries(files_to_relocate, prefix_to_prefix_bin) - elif "elf" in platform.binary_formats: - # The new ELF dynamic section relocation logic only handles absolute to - # absolute relocation. - relocate.relocate_elf_binaries(files_to_relocate, prefix_to_prefix_bin) + platform = spack.platforms.by_name(spec.platform) + if "macho" in platform.binary_formats: + relocate.relocate_macho_binaries(binaries, prefix_to_prefix) + elif "elf" in platform.binary_formats: + relocate.relocate_elf_binaries(binaries, prefix_to_prefix) - # Relocate links to the new install prefix - links = [os.path.join(workdir, f) for f in buildinfo.get("relocate_links", [])] - relocate.relocate_links(links, prefix_to_prefix_bin) + relocate.relocate_links(links, prefix_to_prefix) + relocate.relocate_text(textfiles, prefix_to_prefix) + changed_files = relocate.relocate_text_bin(binaries, prefix_to_prefix) - # For all buildcaches - # relocate the install prefixes in text files including dependencies - relocate.relocate_text(text_names, prefix_to_prefix_text) - - # relocate the install prefixes in binary files including dependencies - changed_files = relocate.relocate_text_bin(files_to_relocate, prefix_to_prefix_bin) - - # Add ad-hoc signatures to patched macho files when on macOS. - if "macho" in platform.binary_formats and sys.platform == "darwin": - codesign = which("codesign") - if not codesign: - return - for binary in changed_files: - # preserve the original inode by running codesign on a copy - with fsys.edit_in_place_through_temporary_file(binary) as tmp_binary: - codesign("-fs-", tmp_binary) - - # If we are installing back to the same location - # relocate the sbang location if the spack directory changed - else: - if old_spack_prefix != new_spack_prefix: - relocate.relocate_text(text_names, prefix_to_prefix_text) + # Add ad-hoc signatures to patched macho files when on macOS. + if "macho" in platform.binary_formats and sys.platform == "darwin": + codesign = which("codesign") + if not codesign: + return + for binary in changed_files: + # preserve the original inode by running codesign on a copy + with fsys.edit_in_place_through_temporary_file(binary) as tmp_binary: + codesign("-fs-", tmp_binary) def _extract_inner_tarball(spec, filename, extract_to, signature_required: bool, remote_checksum): diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index f0eda441ed3..1b3ac455a25 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/lib/spack/spack/filesystem_view.py @@ -35,7 +35,6 @@ import spack.config import spack.directory_layout -import spack.paths import spack.projections import spack.relocate import spack.schema.projections @@ -44,7 +43,6 @@ import spack.util.spack_json as s_json import spack.util.spack_yaml as s_yaml from spack.error import SpackError -from spack.hooks import sbang __all__ = ["FilesystemView", "YamlFilesystemView"] @@ -94,12 +92,6 @@ def view_copy( spack.relocate.relocate_text_bin(binaries=[dst], prefixes=prefix_to_projection) else: prefix_to_projection[spack.store.STORE.layout.root] = view._root - - # This is vestigial code for the *old* location of sbang. - prefix_to_projection[f"#!/bin/bash {spack.paths.spack_root}/bin/sbang"] = ( - sbang.sbang_shebang_line() - ) - spack.relocate.relocate_text(files=[dst], prefixes=prefix_to_projection) # The os module on Windows does not have a chown function. diff --git a/lib/spack/spack/test/bindist.py b/lib/spack/spack/test/bindist.py index 0845be0e4b5..a67ccc1632f 100644 --- a/lib/spack/spack/test/bindist.py +++ b/lib/spack/spack/test/bindist.py @@ -36,13 +36,13 @@ import spack.oci.image import spack.paths import spack.spec -import spack.stage import spack.store import spack.util.gpg import spack.util.spack_yaml as syaml import spack.util.url as url_util import spack.util.web as web_util from spack.binary_distribution import CannotListKeys, GenerateIndexError +from spack.installer import PackageInstaller from spack.paths import test_path from spack.spec import Spec @@ -492,74 +492,40 @@ def mock_list_url(url, recursive=False): assert f"Encountered problem listing packages at {url}" in capfd.readouterr().err -@pytest.mark.usefixtures("mock_fetch", "install_mockery") -def test_update_sbang(tmpdir, temporary_mirror): - """Test the creation and installation of buildcaches with default rpaths - into the non-default directory layout scheme, triggering an update of the - sbang. - """ - spec_str = "old-sbang" - # Concretize a package with some old-fashioned sbang lines. - old_spec = Spec(spec_str).concretized() - old_spec_hash_str = "/{0}".format(old_spec.dag_hash()) +def test_update_sbang(tmp_path, temporary_mirror, mock_fetch, install_mockery): + """Test relocation of the sbang shebang line in a package script""" + s = Spec("old-sbang").concretized() + PackageInstaller([s.package]).install() + old_prefix, old_sbang_shebang = s.prefix, sbang.sbang_shebang_line() + old_contents = f"""\ +{old_sbang_shebang} +#!/usr/bin/env python3 - # Need a fake mirror with *function* scope. - mirror_dir = temporary_mirror - - # Assume all commands will concretize old_spec the same way. - install_cmd("--no-cache", old_spec.name) +{s.prefix.bin} +""" + with open(os.path.join(s.prefix.bin, "script.sh"), encoding="utf-8") as f: + assert f.read() == old_contents # Create a buildcache with the installed spec. - buildcache_cmd("push", "-u", mirror_dir, old_spec_hash_str) - - # Need to force an update of the buildcache index - buildcache_cmd("update-index", mirror_dir) - - # Uninstall the original package. - uninstall_cmd("-y", old_spec_hash_str) + buildcache_cmd("push", "--update-index", "--unsigned", temporary_mirror, f"/{s.dag_hash()}") # Switch the store to the new install tree locations - newtree_dir = tmpdir.join("newtree") - with spack.store.use_store(str(newtree_dir)): - new_spec = Spec("old-sbang").concretized() - assert new_spec.dag_hash() == old_spec.dag_hash() + with spack.store.use_store(str(tmp_path)): + s._prefix = None # clear the cached old prefix + new_prefix, new_sbang_shebang = s.prefix, sbang.sbang_shebang_line() + assert old_prefix != new_prefix + assert old_sbang_shebang != new_sbang_shebang + PackageInstaller([s.package], cache_only=True, unsigned=True).install() - # Install package from buildcache - buildcache_cmd("install", "-u", "-f", new_spec.name) + # Check that the sbang line refers to the new install tree + new_contents = f"""\ +{sbang.sbang_shebang_line()} +#!/usr/bin/env python3 - # Continue blowing away caches - bindist.clear_spec_cache() - spack.stage.purge() - - # test that the sbang was updated by the move - sbang_style_1_expected = """{0} -#!/usr/bin/env python - -{1} -""".format( - sbang.sbang_shebang_line(), new_spec.prefix.bin - ) - sbang_style_2_expected = """{0} -#!/usr/bin/env python - -{1} -""".format( - sbang.sbang_shebang_line(), new_spec.prefix.bin - ) - - installed_script_style_1_path = new_spec.prefix.bin.join("sbang-style-1.sh") - assert ( - sbang_style_1_expected - == open(str(installed_script_style_1_path), encoding="utf-8").read() - ) - - installed_script_style_2_path = new_spec.prefix.bin.join("sbang-style-2.sh") - assert ( - sbang_style_2_expected - == open(str(installed_script_style_2_path), encoding="utf-8").read() - ) - - uninstall_cmd("-y", "/%s" % new_spec.dag_hash()) +{s.prefix.bin} +""" + with open(os.path.join(s.prefix.bin, "script.sh"), encoding="utf-8") as f: + assert f.read() == new_contents @pytest.mark.skipif( diff --git a/var/spack/repos/builtin.mock/packages/old-sbang/package.py b/var/spack/repos/builtin.mock/packages/old-sbang/package.py index fe4c362a4bc..87420587dbe 100644 --- a/var/spack/repos/builtin.mock/packages/old-sbang/package.py +++ b/var/spack/repos/builtin.mock/packages/old-sbang/package.py @@ -1,13 +1,14 @@ # Copyright Spack Project Developers. See COPYRIGHT file for details. # # SPDX-License-Identifier: (Apache-2.0 OR MIT) -import spack.paths -import spack.store +import os + +from spack.hooks.sbang import sbang_shebang_line from spack.package import * class OldSbang(Package): - """Toy package for testing the old sbang replacement problem""" + """Package for testing sbang relocation""" homepage = "https://www.example.com" url = "https://www.example.com/old-sbang.tar.gz" @@ -16,23 +17,11 @@ class OldSbang(Package): def install(self, spec, prefix): mkdirp(prefix.bin) + contents = f"""\ +{sbang_shebang_line()} +#!/usr/bin/env python3 - sbang_style_1 = """#!/bin/bash {0}/bin/sbang -#!/usr/bin/env python - -{1} -""".format( - spack.paths.prefix, prefix.bin - ) - sbang_style_2 = """#!/bin/sh {0}/bin/sbang -#!/usr/bin/env python - -{1} -""".format( - spack.store.STORE.unpadded_root, prefix.bin - ) - with open("%s/sbang-style-1.sh" % self.prefix.bin, "w", encoding="utf-8") as f: - f.write(sbang_style_1) - - with open("%s/sbang-style-2.sh" % self.prefix.bin, "w", encoding="utf-8") as f: - f.write(sbang_style_2) +{prefix.bin} +""" + with open(os.path.join(self.prefix.bin, "script.sh"), "w", encoding="utf-8") as f: + f.write(contents)