Buildcache/ensure symlinks proper prefix (#43851)

* archive: relative links only

Ensure all links written into tarfiles generated from Spack prefixes do not contain symlinks pointing outside the prefix

* binary_distribution: limit extraction to prefix

Ensure files extracted from spackballs are not links pointing outside of the prefix

* Ensure rpaths are properly set on Windows

* hard error on extraction of absolute links

* refactor for non link-modifying approach

* Restore tarball extraction to original impl

* use custom readlink

* cleanup symlink module

* make lstrip

lib/spack/llnl/path.py

@@ -98,3 +98,10 @@ def path_filter_caller(*args, **kwargs):
     if _func:
         return holder_func(_func)
     return holder_func
+
+
+def sanitize_win_longpath(path: str) -> str:
+    """Strip Windows extended path prefix from strings
+    Returns sanitized string.
+    no-op if extended path prefix is not present"""
+    return path.lstrip("\\\\?\\")

lib/spack/llnl/util/filesystem.py

@@ -2429,9 +2429,10 @@ def add_library_dependent(self, *dest):
         """
         for pth in dest:
             if os.path.isfile(pth):
-                self._additional_library_dependents.add(pathlib.Path(pth).parent)
+                new_pth = pathlib.Path(pth).parent
             else:
-                self._additional_library_dependents.add(pathlib.Path(pth))
+                new_pth = pathlib.Path(pth)
+            self._additional_library_dependents.add(new_pth)
 
     @property
     def rpaths(self):

lib/spack/llnl/util/symlink.py

@@ -11,7 +11,7 @@
 
 from llnl.util import lang, tty
 
-from ..path import system_path_filter
+from ..path import sanitize_win_longpath, system_path_filter
 
 if sys.platform == "win32":
     from win32file import CreateHardLink
@@ -247,9 +247,9 @@ def _windows_create_junction(source: str, link: str):
     out, err = proc.communicate()
     tty.debug(out.decode())
     if proc.returncode != 0:
-        err = err.decode()
+        err_str = err.decode()
-        tty.error(err)
+        tty.error(err_str)
-        raise SymlinkError("Make junction command returned a non-zero return code.", err)
+        raise SymlinkError("Make junction command returned a non-zero return code.", err_str)
 
 
 def _windows_create_hard_link(path: str, link: str):
@@ -269,14 +269,14 @@ def _windows_create_hard_link(path: str, link: str):
         CreateHardLink(link, path)
 
 
-def readlink(path: str):
+def readlink(path: str, *, dir_fd=None):
     """Spack utility to override of os.readlink method to work cross platform"""
     if _windows_is_hardlink(path):
         return _windows_read_hard_link(path)
     elif _windows_is_junction(path):
         return _windows_read_junction(path)
     else:
-        return os.readlink(path)
+        return sanitize_win_longpath(os.readlink(path, dir_fd=dir_fd))
 
 
 def _windows_read_hard_link(link: str) -> str:

lib/spack/spack/binary_distribution.py

@@ -29,6 +29,7 @@
 import llnl.util.lang
 import llnl.util.tty as tty
 from llnl.util.filesystem import BaseDirectoryVisitor, mkdirp, visit_directory_tree
+from llnl.util.symlink import readlink
 
 import spack.caches
 import spack.cmd
@@ -658,7 +659,7 @@ def get_buildfile_manifest(spec):
     #   2. paths are used as strings.
     for rel_path in visitor.symlinks:
         abs_path = os.path.join(root, rel_path)
-        link = os.readlink(abs_path)
+        link = readlink(abs_path)
         if os.path.isabs(link) and link.startswith(spack.store.STORE.layout.root):
             data["link_to_relocate"].append(rel_path)
 
@@ -2001,6 +2002,7 @@ def install_root_node(spec, unsigned=False, force=False, sha256=None):
     with spack.util.path.filter_padding():
         tty.msg('Installing "{0}" from a buildcache'.format(spec.format()))
         extract_tarball(spec, download_result, force)
+        spec.package.windows_establish_runtime_linkage()
         spack.hooks.post_install(spec, False)
         spack.store.STORE.db.add(spec, spack.store.STORE.layout)
 

lib/spack/spack/installer.py

@@ -488,6 +488,7 @@ def _process_binary_cache_tarball(
 
     with timer.measure("install"), spack.util.path.filter_padding():
         binary_distribution.extract_tarball(pkg.spec, download_result, force=False, timer=timer)
+        pkg.windows_establish_runtime_linkage()
 
         if hasattr(pkg, "_post_buildcache_install_hook"):
             pkg._post_buildcache_install_hook()

lib/spack/spack/relocate.py

@@ -16,7 +16,7 @@
 import llnl.util.lang
 import llnl.util.tty as tty
 from llnl.util.lang import memoized
-from llnl.util.symlink import symlink
+from llnl.util.symlink import readlink, symlink
 
 import spack.paths
 import spack.platforms
@@ -25,6 +25,7 @@
 import spack.store
 import spack.util.elf as elf
 import spack.util.executable as executable
+import spack.util.path
 
 from .relocate_text import BinaryFilePrefixReplacer, TextFilePrefixReplacer
 
@@ -613,7 +614,7 @@ def relocate_links(links, prefix_to_prefix):
     """Relocate links to a new install prefix."""
     regex = re.compile("|".join(re.escape(p) for p in prefix_to_prefix.keys()))
     for link in links:
-        old_target = os.readlink(link)
+        old_target = readlink(link)
         match = regex.match(old_target)
 
         # No match.

lib/spack/spack/util/archive.py

@@ -12,6 +12,8 @@
 from gzip import GzipFile
 from typing import Callable, Dict, Tuple
 
+from llnl.util.symlink import readlink
+
 
 class ChecksumWriter(io.BufferedIOBase):
     """Checksum writer computes a checksum while writing to a file."""
@@ -193,12 +195,14 @@ def reproducible_tarfile_from_prefix(
             file_info = tarfile.TarInfo(path_to_name(entry.path))
 
             if entry.is_symlink():
-                file_info.type = tarfile.SYMTYPE
+                # strip off long path reg prefix on Windows
-                file_info.linkname = os.readlink(entry.path)
+                link_dest = readlink(entry.path)
+                file_info.linkname = link_dest
                 # According to POSIX: "the value of the file mode bits returned in the
                 # st_mode field of the stat structure is unspecified." So we set it to
                 # something sensible without lstat'ing the link.
                 file_info.mode = 0o755
+                file_info.type = tarfile.SYMTYPE
                 tar.addfile(file_info)
 
             elif entry.is_file(follow_symlinks=False):