Fix multiple issues with Python in views (#42601)

This fixes bugs, performance issues, and removes no longer necessary code.

Short version:

1. Creating views from Python extensions would error if the Spack `opt` dir itself was in some symlinked directory. Use of `realpath` would expand those, and keying into `merge_map` would fail.
2. Creating views from Python extensions (and Python itself, potentially) could fail if the `bin/` dir contains symlinks pointing outside the package prefix -- Spack keyed into `merge_map[target_of_symlink]` incorrectly.
3. In the `python` package the `remove_files_from_view` function was broken after a breaking API change two years ago (#24355). However, the entire function body was redundant anyways, so solved it by removing it.
4. Notions of "global view" (i.e. python extensions being linked into Python's own prefix instead of into a view) are completely outdated, and removed. It used to be supported but was removed years ago.
5. Views for Python extension would _always_ copy non-symlinks in `./bin/*`, which is a big mistake, since all we care about is rewriting shebangs of scripts; we don't want to copy binaries. Now we first check if the file is executable, and then read two bytes to check if it has a shebang, and only if so, copy the entire file and patch up shebangs.

The bug fixes for (1) and (2) basically consist of getting rid of `realpath` entirely, and instead simply keep track of file identifiers of files that are copied/modified in the view. Only after patching up regular files do we iterate over symlinks and check if they target one of those. If so, retarget it to the modified file in the view.

1 files changed, 40 insertions, 32 deletions

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"""