sbang: put sbang in the install_tree (#11598)

`sbang` is not always accessible to users of packages, e.g., if Spack
is installed in someone's home directory and they deploy software
for others.  Avoid this by:

1. Always installing the `sbang` script in the `install_tree`
2. Relocating binaries to point to the copy in the `install_tree` 
   and not the one in the Spack installation.

This PR also:
- ensures that `sbang` is reinstalled if it is modified in Spack
- adds tests
- updates the way `gobject-introspection` patches Makefiles
   to support `sbang`

Co-authored-by: Todd Gamblin <tgamblin@llnl.gov>

lib/spack/spack/hooks/sbang.py

@@ -3,21 +3,30 @@
 #
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
+import filecmp
 import os
 import stat
 import re
 import sys
 
 import llnl.util.tty as tty
+import llnl.util.filesystem as fs
 
-import spack.paths
 import spack.modules
+import spack.paths
+import spack.store
 
-# Character limit for shebang line.  Using Linux's 127 characters
-# here, as it is the shortest I could find on a modern OS.
+
+#: Character limit for shebang line.  Using Linux's 127 characters
+#: here, as it is the shortest I could find on a modern OS.
 shebang_limit = 127
 
 
+def sbang_install_path():
+    """Location sbang should be installed within Spack's ``install_tree``."""
+    return os.path.join(spack.store.layout.root, "bin", "sbang")
+
+
 def shebang_too_long(path):
     """Detects whether a file has a shebang line that is too long."""
     if not os.path.isfile(path):
@@ -42,7 +51,7 @@ def filter_shebang(path):
             original = original.decode('UTF-8')
 
     # This line will be prepended to file
-    new_sbang_line = '#!/bin/bash %s/bin/sbang\n' % spack.paths.prefix
+    new_sbang_line = '#!/bin/bash %s\n' % sbang_install_path()
 
     # Skip files that are already using sbang.
     if original.startswith(new_sbang_line):
@@ -102,6 +111,26 @@ def filter_shebangs_in_directory(directory, filenames=None):
             filter_shebang(path)
 
 
+def install_sbang():
+    """Ensure that ``sbang`` is installed in the root of Spack's install_tree.
+
+    This is the shortest known publicly accessible path, and installing
+    ``sbang`` here ensures that users can access the script and that
+    ``sbang`` itself is in a short path.
+    """
+    # copy in a new version of sbang if it differs from what's in spack
+    sbang_path = sbang_install_path()
+    if os.path.exists(sbang_path) and filecmp.cmp(
+            spack.paths.sbang_script, sbang_path):
+        return
+
+    # make $install_tree/bin and copy in a new version of sbang if needed
+    sbang_bin_dir = os.path.dirname(sbang_path)
+    fs.mkdirp(sbang_bin_dir)
+    fs.install(spack.paths.sbang_script, sbang_path)
+    fs.set_install_permissions(sbang_bin_dir)
+
+
 def post_install(spec):
     """This hook edits scripts so that they call /bin/bash
     $spack_prefix/bin/sbang instead of something longer than the
@@ -111,5 +140,7 @@ def post_install(spec):
         tty.debug('SKIP: shebang filtering [external package]')
         return
 
+    install_sbang()
+
     for directory, _, filenames in os.walk(spec.prefix):
         filter_shebangs_in_directory(directory, filenames)

lib/spack/spack/paths.py

@@ -25,6 +25,9 @@
 #: The spack script itself
 spack_script = os.path.join(bin_path, "spack")
 
+#: The sbang script in the spack installation
+sbang_script = os.path.join(bin_path, "sbang")
+
 # spack directory hierarchy
 lib_path              = os.path.join(prefix, "lib", "spack")
 external_path         = os.path.join(lib_path, "external")

lib/spack/spack/test/sbang.py

@@ -13,10 +13,11 @@
 import shutil
 import filecmp
 
-from llnl.util.filesystem import mkdirp
+import llnl.util.filesystem as fs
 
 import spack.paths
-from spack.hooks.sbang import shebang_too_long, filter_shebangs_in_directory
+import spack.store
+import spack.hooks.sbang as sbang
 from spack.util.executable import which
 
 
@@ -28,7 +29,7 @@
 node_line         = "#!/this/" + ('x' * 200) + "/is/node\n"
 node_in_text      = ("line\n") * 100 + "lua\n" + ("line\n" * 100)
 node_line_patched = "//!/this/" + ('x' * 200) + "/is/node\n"
-sbang_line        = '#!/bin/bash %s/bin/sbang\n' % spack.paths.prefix
+sbang_line        = '#!/bin/bash %s/bin/sbang\n' % spack.store.layout.root
 last_line         = "last!\n"
 
 
@@ -38,7 +39,7 @@ def __init__(self):
         self.tempdir = tempfile.mkdtemp()
 
         self.directory = os.path.join(self.tempdir, 'dir')
-        mkdirp(self.directory)
+        fs.mkdirp(self.directory)
 
         # Script with short shebang
         self.short_shebang = os.path.join(self.tempdir, 'short')
@@ -102,15 +103,15 @@ def script_dir():
 
 
 def test_shebang_handling(script_dir):
-    assert shebang_too_long(script_dir.lua_shebang)
-    assert shebang_too_long(script_dir.long_shebang)
+    assert sbang.shebang_too_long(script_dir.lua_shebang)
+    assert sbang.shebang_too_long(script_dir.long_shebang)
 
-    assert not shebang_too_long(script_dir.short_shebang)
-    assert not shebang_too_long(script_dir.has_sbang)
-    assert not shebang_too_long(script_dir.binary)
-    assert not shebang_too_long(script_dir.directory)
+    assert not sbang.shebang_too_long(script_dir.short_shebang)
+    assert not sbang.shebang_too_long(script_dir.has_sbang)
+    assert not sbang.shebang_too_long(script_dir.binary)
+    assert not sbang.shebang_too_long(script_dir.directory)
 
-    filter_shebangs_in_directory(script_dir.tempdir)
+    sbang.filter_shebangs_in_directory(script_dir.tempdir)
 
     # Make sure this is untouched
     with open(script_dir.short_shebang, 'r') as f:
@@ -157,3 +158,41 @@ def test_shebang_handles_non_writable_files(script_dir):
 
     st = os.stat(script_dir.long_shebang)
     assert oct(not_writable_mode) == oct(st.st_mode)
+
+
+def check_sbang():
+    sbang_path = sbang.sbang_install_path()
+    sbang_bin_dir = os.path.dirname(sbang_path)
+    assert sbang_path.startswith(spack.store.layout.root)
+
+    assert os.path.exists(sbang_path)
+    assert fs.is_exe(sbang_path)
+
+    status = os.stat(sbang_path)
+    assert (status.st_mode & 0o777) == 0o755
+
+    status = os.stat(sbang_bin_dir)
+    assert (status.st_mode & 0o777) == 0o755
+
+
+def test_install_sbang(install_mockery):
+    sbang_path = sbang.sbang_install_path()
+    sbang_bin_dir = os.path.dirname(sbang_path)
+
+    assert sbang_path.startswith(spack.store.layout.root)
+    assert not os.path.exists(sbang_bin_dir)
+
+    sbang.install_sbang()
+    check_sbang()
+
+    # put an invalid file in for sbang
+    fs.mkdirp(sbang_bin_dir)
+    with open(sbang_path, "w") as f:
+        f.write("foo")
+
+    sbang.install_sbang()
+    check_sbang()
+
+    # install again and make sure sbang is still fine
+    sbang.install_sbang()
+    check_sbang()

var/spack/repos/builtin/packages/gobject-introspection/package.py

@@ -4,7 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 from spack import *
-from spack.paths import spack_root
+import spack.hooks.sbang as sbang
 
 
 class GobjectIntrospection(Package):
@@ -72,7 +72,7 @@ def install(self, spec, prefix):
         make("install")
 
     def setup_build_environment(self, env):
-        env.set('SPACK_SBANG', "%s/bin/sbang" % spack_root)
+        env.set('SPACK_SBANG', sbang.sbang_install_path())
 
     @property
     def parallel(self):