From 773da7ceba116154e999187e0d9bcef5d58bebd4 Mon Sep 17 00:00:00 2001
From: Harmen Stoppels <harmenstoppels@gmail.com>
Date: Wed, 23 Mar 2022 10:31:23 +0100
Subject: [PATCH] python: drop dependency on `file` for script check (#29513)

`file` was used to detect Python scripts with shebangs, so that the interpreter could be changed from <python prefix> to <view path>. With this change, we detect shebangs using Python instead, so that `file` is no longer required.
---
 lib/spack/llnl/util/filesystem.py             | 23 ++++++++++++++++++
 lib/spack/spack/build_systems/python.py       |  4 ++--
 lib/spack/spack/test/llnl/util/filesystem.py  | 24 +++++++++++++++++++
 .../repos/builtin/packages/python/package.py  |  8 +++++--
 4 files changed, 55 insertions(+), 4 deletions(-)

diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py
index 22ca97c3478..179e1703b9f 100644
--- a/lib/spack/llnl/util/filesystem.py
+++ b/lib/spack/llnl/util/filesystem.py
@@ -617,6 +617,29 @@ def get_filetype(path_name):
     return output.strip()
 
 
+@system_path_filter
+def is_nonsymlink_exe_with_shebang(path):
+    """
+    Returns whether the path is an executable script with a shebang.
+    Return False when the path is a *symlink* to an executable script.
+    """
+    try:
+        st = os.lstat(path)
+        # Should not be a symlink
+        if stat.S_ISLNK(st.st_mode):
+            return False
+
+        # Should be executable
+        if not st.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH):
+            return False
+
+        # Should start with a shebang
+        with open(path, 'rb') as f:
+            return f.read(2) == b'#!'
+    except (IOError, OSError):
+        return False
+
+
 @system_path_filter(arg_slice=slice(1))
 def chgrp_if_not_world_writable(path, group):
     """chgrp path to group if path is not world writable"""
diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py
index b3552f03370..6aa225ba6c4 100644
--- a/lib/spack/spack/build_systems/python.py
+++ b/lib/spack/spack/build_systems/python.py
@@ -11,7 +11,7 @@
 from llnl.util.filesystem import (
     filter_file,
     find,
-    get_filetype,
+    is_nonsymlink_exe_with_shebang,
     path_contains_subdirectory,
     same_path,
     working_dir,
@@ -230,7 +230,7 @@ def add_files_to_view(self, view, merge_map):
                 view.link(src, dst)
             elif not os.path.islink(src):
                 shutil.copy2(src, dst)
-                is_script = 'script' in get_filetype(src)
+                is_script = is_nonsymlink_exe_with_shebang(src)
                 if is_script and not python_is_external:
                     filter_file(
                         python_prefix, os.path.abspath(
diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py
index 2bd6994754f..b5a4594efb2 100644
--- a/lib/spack/spack/test/llnl/util/filesystem.py
+++ b/lib/spack/spack/test/llnl/util/filesystem.py
@@ -674,3 +674,27 @@ def test_temporary_dir_context_manager():
     with fs.temporary_dir() as tmp_dir:
         assert previous_dir != os.path.realpath(os.getcwd())
         assert os.path.realpath(str(tmp_dir)) == os.path.realpath(os.getcwd())
+
+
+@pytest.mark.skipif(sys.platform == 'win32', reason="No shebang on Windows")
+def test_is_nonsymlink_exe_with_shebang(tmpdir):
+    with tmpdir.as_cwd():
+        # Create an executable with shebang.
+        with open('executable_script', 'wb') as f:
+            f.write(b'#!/interpreter')
+        os.chmod('executable_script', 0o100775)
+
+        with open('executable_but_not_script', 'wb') as f:
+            f.write(b'#/not-a-shebang')
+        os.chmod('executable_but_not_script', 0o100775)
+
+        with open('not_executable_with_shebang', 'wb') as f:
+            f.write(b'#!/interpreter')
+        os.chmod('not_executable_with_shebang', 0o100664)
+
+        os.symlink('executable_script', 'symlink_to_executable_script')
+
+        assert fs.is_nonsymlink_exe_with_shebang('executable_script')
+        assert not fs.is_nonsymlink_exe_with_shebang('executable_but_not_script')
+        assert not fs.is_nonsymlink_exe_with_shebang('not_executable_with_shebang')
+        assert not fs.is_nonsymlink_exe_with_shebang('symlink_to_executable_script')
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index be21af5e163..c97407d9fc4 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -13,7 +13,11 @@
 from shutil import copy
 
 import llnl.util.tty as tty
-from llnl.util.filesystem import copy_tree, get_filetype, path_contains_subdirectory
+from llnl.util.filesystem import (
+    copy_tree,
+    is_nonsymlink_exe_with_shebang,
+    path_contains_subdirectory,
+)
 from llnl.util.lang import match_predicate
 
 from spack import *
@@ -1369,7 +1373,7 @@ def add_files_to_view(self, view, merge_map):
                 view.link(src, dst, spec=self.spec)
             elif not os.path.islink(src):
                 copy(src, dst)
-                if 'script' in get_filetype(src):
+                if is_nonsymlink_exe_with_shebang(src):
                     filter_file(
                         self.spec.prefix,
                         os.path.abspath(