From a80d221bfa1c9be4b2b9eff9f057edf62c34e50b Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Fri, 30 Oct 2020 17:06:05 -0700 Subject: sbang: fixes for sbang relocation This fixes sbang relocation when using old binary packages, and updates code in `relocate.py`. There are really two places where we would want to handle an `sbang` relocation: 1. Installing an old package that uses `sbang` with shebang lines like `#!/bin/bash $spack_prefix/sbang` 2. Installing a *new* package that uses `sbang` with shebang lines like `#!/bin/sh $install_tree/sbang` The second case is actually handled automatically by our text relocation; we don't need any special relocation logic for new shebangs, as our relocation logic already changes references to the build-time `install_tree` to point to the `install_tree` at intall-time. Case 1 was not properly handled -- we would not take an old binary package and point its shebangs at the new `sbang` location. This PR fixes that and updates the code in `relocation.py` with some notes. There is one more case we don't currently handle: if a binary package is created from an installation in a short prefix that does *not* need `sbang` and is installed to a long prefix that *does* need `sbang`, we won't do anything. We should just patch the file as we would for a normal install. In some upcoming PR we should probably change *all* `sbang` relocation logic to be idempotent and to apply to any sort of shebang'd file. Then we'd only have to worry about which files to `sbang`-ify at install time and wouldn't need to care about these special cases. --- lib/spack/spack/hooks/sbang.py | 14 +++++++++++- lib/spack/spack/relocate.py | 26 ++++++++++++++++------- lib/spack/spack/test/relocate.py | 46 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 9 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py index 76b571f3d2..12a2cbf6ef 100644 --- a/lib/spack/spack/hooks/sbang.py +++ b/lib/spack/spack/hooks/sbang.py @@ -27,6 +27,18 @@ def sbang_install_path(): return os.path.join(spack.store.layout.root, "bin", "sbang") +def sbang_shebang_line(): + """Full shebang line that should be prepended to files to use sbang. + + The line returned does not have a final newline (caller should add it + if needed). + + This should be the only place in Spack that knows about what + interpreter we use for ``sbang``. + """ + return '#!/bin/sh %s' % sbang_install_path() + + def shebang_too_long(path): """Detects whether a file has a shebang line that is too long.""" if not os.path.isfile(path): @@ -51,7 +63,7 @@ def filter_shebang(path): original = original.decode('UTF-8') # This line will be prepended to file - new_sbang_line = '#!/bin/sh %s\n' % sbang_install_path() + new_sbang_line = '%s\n' % sbang_shebang_line() # Skip files that are already using sbang. if original.startswith(new_sbang_line): diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index a87156902f..2685b0c6ab 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -136,7 +136,7 @@ def _make_relative(reference_file, path_root, paths): reference_file (str): file from which the reference directory is computed path_root (str): root of the relative paths - paths: paths to be examined + paths: (list) paths to be examined Returns: List of relative paths @@ -790,10 +790,10 @@ def relocate_text( files, orig_layout_root, new_layout_root, orig_install_prefix, new_install_prefix, orig_spack, new_spack, new_prefixes ): - """Relocate text file from the original installation prefix to the - new prefix. + """Relocate text file from the original ``install_tree`` to the new one. - Relocation also affects the the path in Spack's sbang script. + This also handles relocating Spack's sbang scripts to point at the + new install tree. Args: files (list): text files to be relocated @@ -805,19 +805,29 @@ def relocate_text( new_spack (str): path to the new Spack new_prefixes (dict): dictionary that maps the original prefixes to where they should be relocated + """ # TODO: reduce the number of arguments (8 seems too much) + + # 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. + import spack.hooks.sbang as sbang orig_sbang = '#!/bin/bash {0}/bin/sbang'.format(orig_spack) - new_sbang = '#!/bin/bash {0}/bin/sbang'.format(new_spack) + new_sbang = sbang.sbang_shebang_line() + # Do relocations on text that refers to the install tree for filename in files: _replace_prefix_text(filename, orig_install_prefix, new_install_prefix) for orig_dep_prefix, new_dep_prefix in new_prefixes.items(): _replace_prefix_text(filename, orig_dep_prefix, new_dep_prefix) _replace_prefix_text(filename, orig_layout_root, new_layout_root) - # relocate the sbang location only if the spack directory changed - if orig_spack != new_spack: - _replace_prefix_text(filename, orig_sbang, new_sbang) + + # Point old packages at the new sbang location. Packages that + # already use the new sbang location will already have been + # handled by the prior call to _replace_prefix_text + _replace_prefix_text(filename, orig_sbang, new_sbang) def relocate_text_bin( diff --git a/lib/spack/spack/test/relocate.py b/lib/spack/spack/test/relocate.py index f9e72ea0d7..e883bd44c5 100644 --- a/lib/spack/spack/test/relocate.py +++ b/lib/spack/spack/test/relocate.py @@ -12,6 +12,7 @@ import llnl.util.filesystem import pytest import spack.architecture import spack.concretize +import spack.hooks.sbang as sbang import spack.paths import spack.relocate import spack.spec @@ -401,3 +402,48 @@ def test_relocate_text_bin_raise_if_new_prefix_is_longer(): spack.relocate.relocate_text_bin( ['item'], short_prefix, long_prefix, None, None, None ) + + +@pytest.mark.parametrize("sbang_line", [ + "#!/bin/bash /path/to/orig/spack/bin/sbang", + "#!/bin/sh /orig/layout/root/bin/sbang" +]) +def test_relocate_text_old_sbang(tmpdir, sbang_line): + """Ensure that old and new sbang styles are relocated.""" + + old_install_prefix = "/orig/layout/root/orig/install/prefix" + new_install_prefix = os.path.join( + spack.store.layout.root, "new", "install", "prefix" + ) + + # input file with an sbang line + original = """\ +{0} +#!/usr/bin/env python + +/orig/layout/root/orig/install/prefix +""".format(sbang_line) + + # expected relocation + expected = """\ +{0} +#!/usr/bin/env python + +{1} +""".format(sbang.sbang_shebang_line(), new_install_prefix) + + path = tmpdir.ensure("path", "to", "file") + with path.open("w") as f: + f.write(original) + + spack.relocate.relocate_text( + [str(path)], + "/orig/layout/root", spack.store.layout.root, + old_install_prefix, new_install_prefix, + "/path/to/orig/spack", spack.paths.spack_root, + { + old_install_prefix: new_install_prefix + } + ) + + assert expected == open(str(path)).read() -- cgit v1.2.3-60-g2f50