diff options
| -rw-r--r-- | lib/spack/llnl/util/filesystem.py | 34 | ||||
| -rw-r--r-- | lib/spack/spack/build_systems/python.py | 54 | ||||
| -rw-r--r-- | var/spack/repos/builtin/packages/python/package.py | 72 |
3 files changed, 97 insertions, 63 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index cff512a167..b13fc00bd0 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -920,28 +920,34 @@ def get_filetype(path_name): return output.strip() +def has_shebang(path): + """Returns whether a path has a shebang line. Returns False if the file cannot be opened.""" + try: + with open(path, "rb") as f: + return f.read(2) == b"#!" + except OSError: + return False + + @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. - """ + """Returns whether the path is an executable regular file with a shebang. Returns False too + when the path is a symlink to a script, and also when the file cannot be opened.""" try: st = os.lstat(path) - # Should not be a symlink - if stat.S_ISLNK(st.st_mode): - return False + except OSError: + return False - # Should be executable - if not st.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH): - return False + # Should not be a symlink + if stat.S_ISLNK(st.st_mode): + return False - # Should start with a shebang - with open(path, "rb") as f: - return f.read(2) == b"#!" - except (IOError, OSError): + # Should be executable + if not st.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH): return False + return has_shebang(path) + @system_path_filter(arg_slice=slice(1)) def chgrp_if_not_world_writable(path, group): diff --git a/lib/spack/spack/build_systems/python.py b/lib/spack/spack/build_systems/python.py index 39dc7519e8..824d634b3e 100644 --- a/lib/spack/spack/build_systems/python.py +++ b/lib/spack/spack/build_systems/python.py @@ -6,7 +6,8 @@ import inspect import os import re import shutil -from typing import Iterable, List, Mapping, Optional +import stat +from typing import Dict, Iterable, List, Mapping, Optional, Tuple import archspec @@ -136,31 +137,50 @@ class PythonExtension(spack.package_base.PackageBase): return conflicts def add_files_to_view(self, view, merge_map, skip_if_exists=True): - if not self.extendee_spec: + # Patch up shebangs to the python linked in the view only if python is built by Spack. + if not self.extendee_spec or self.extendee_spec.external: return super().add_files_to_view(view, merge_map, skip_if_exists) + # We only patch shebangs in the bin directory. + copied_files: Dict[Tuple[int, int], str] = {} # File identifier -> source + delayed_links: List[Tuple[str, str]] = [] # List of symlinks from merge map + bin_dir = self.spec.prefix.bin python_prefix = self.extendee_spec.prefix - python_is_external = self.extendee_spec.external - global_view = fs.same_path(python_prefix, view.get_projection_for_spec(self.spec)) for src, dst in merge_map.items(): - if os.path.exists(dst): + if skip_if_exists and os.path.lexists(dst): continue - elif global_view or not fs.path_contains_subdirectory(src, bin_dir): + + if not fs.path_contains_subdirectory(src, bin_dir): view.link(src, dst) - elif not os.path.islink(src): + continue + + s = os.lstat(src) + + # Symlink is delayed because we may need to re-target if its target is copied in view + if stat.S_ISLNK(s.st_mode): + delayed_links.append((src, dst)) + continue + + # If it's executable and has a shebang, copy and patch it. + if (s.st_mode & 0b111) and fs.has_shebang(src): + copied_files[(s.st_dev, s.st_ino)] = dst shutil.copy2(src, dst) - is_script = fs.is_nonsymlink_exe_with_shebang(src) - if is_script and not python_is_external: - fs.filter_file( - python_prefix, - os.path.abspath(view.get_projection_for_spec(self.spec)), - dst, - ) + fs.filter_file( + python_prefix, os.path.abspath(view.get_projection_for_spec(self.spec)), dst + ) + + # Finally re-target the symlinks that point to copied files. + for src, dst in delayed_links: + try: + s = os.stat(src) + target = copied_files[(s.st_dev, s.st_ino)] + except (OSError, KeyError): + target = None + if target: + os.symlink(target, dst) else: - orig_link_target = os.path.realpath(src) - new_link_target = os.path.abspath(merge_map[orig_link_target]) - view.link(new_link_target, dst) + view.link(src, dst, spec=self.spec) def remove_files_from_view(self, view, merge_map): ignore_namespace = False diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py index 4ebf32693e..8ca289b196 100644 --- a/var/spack/repos/builtin/packages/python/package.py +++ b/var/spack/repos/builtin/packages/python/package.py @@ -8,10 +8,11 @@ import json import os import platform import re +import stat import subprocess import sys from shutil import copy -from typing import Dict, List +from typing import Dict, List, Tuple import llnl.util.tty as tty from llnl.util.filesystem import is_nonsymlink_exe_with_shebang, path_contains_subdirectory @@ -1265,42 +1266,49 @@ print(json.dumps(config)) module.python_purelib = join_path(dependent_spec.prefix, self.purelib) def add_files_to_view(self, view, merge_map, skip_if_exists=True): + # The goal is to copy the `python` executable, so that its search paths are relative to the + # view instead of the install prefix. This is an obsolete way of creating something that + # resembles a virtual environnent. Also we copy scripts with shebang lines. Finally we need + # to re-target symlinks pointing to copied files. bin_dir = self.spec.prefix.bin if sys.platform != "win32" else self.spec.prefix + copied_files: Dict[Tuple[int, int], str] = {} # File identifier -> source + delayed_links: List[Tuple[str, str]] = [] # List of symlinks from merge map for src, dst in merge_map.items(): + if skip_if_exists and os.path.lexists(dst): + continue + + # Files not in the bin dir are linked the default way. if not path_contains_subdirectory(src, bin_dir): view.link(src, dst, spec=self.spec) - elif not os.path.islink(src): - copy(src, dst) - if is_nonsymlink_exe_with_shebang(src): - filter_file( - self.spec.prefix, - os.path.abspath(view.get_projection_for_spec(self.spec)), - dst, - backup=False, - ) - else: - # orig_link_target = os.path.realpath(src) is insufficient when - # the spack install tree is located at a symlink or a - # descendent of a symlink. What we need here is the real - # relative path from the python prefix to src - # TODO: generalize this logic in the link_tree object - # add a method to resolve a link relative to the link_tree - # object root. - realpath_src = os.path.realpath(src) - realpath_prefix = os.path.realpath(self.spec.prefix) - realpath_rel = os.path.relpath(realpath_src, realpath_prefix) - orig_link_target = os.path.join(self.spec.prefix, realpath_rel) - - new_link_target = os.path.abspath(merge_map[orig_link_target]) - view.link(new_link_target, dst, spec=self.spec) - - def remove_files_from_view(self, view, merge_map): - bin_dir = self.spec.prefix.bin if sys.platform != "win32" else self.spec.prefix - for src, dst in merge_map.items(): - if not path_contains_subdirectory(src, bin_dir): - view.remove_file(src, dst) + continue + + s = os.lstat(src) + + # Symlink is delayed because we may need to re-target if its target is copied in view + if stat.S_ISLNK(s.st_mode): + delayed_links.append((src, dst)) + continue + + # Anything that's not a symlink gets copied. Scripts with shebangs are immediately + # updated when necessary. + copied_files[(s.st_dev, s.st_ino)] = dst + copy(src, dst) + if is_nonsymlink_exe_with_shebang(src): + filter_file( + self.spec.prefix, os.path.abspath(view.get_projection_for_spec(self.spec)), dst + ) + + # Finally re-target the symlinks that point to copied files. + for src, dst in delayed_links: + try: + s = os.stat(src) + target = copied_files[(s.st_dev, s.st_ino)] + except (OSError, KeyError): + target = None + if target: + os.symlink(target, dst) else: - os.remove(dst) + view.link(src, dst, spec=self.spec) def test_hello_world(self): """run simple hello world program""" |