From a79b1bd9afbaf854c56633fac6e702c154164ed8 Mon Sep 17 00:00:00 2001 From: "John W. Parent" <45471568+johnwparent@users.noreply.github.com> Date: Fri, 10 May 2024 14:00:40 -0400 Subject: [PATCH] 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 | 7 +++++++ lib/spack/llnl/util/filesystem.py | 5 +++-- lib/spack/llnl/util/symlink.py | 12 ++++++------ lib/spack/spack/binary_distribution.py | 4 +++- lib/spack/spack/installer.py | 1 + lib/spack/spack/relocate.py | 5 +++-- lib/spack/spack/util/archive.py | 8 ++++++-- 7 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/spack/llnl/path.py b/lib/spack/llnl/path.py index 9ef90eec4c..4c5da8472d 100644 --- a/lib/spack/llnl/path.py +++ b/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("\\\\?\\") diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 57baa509c3..84373f7f4a 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/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): diff --git a/lib/spack/llnl/util/symlink.py b/lib/spack/llnl/util/symlink.py index ec45787a96..934aba552b 100644 --- a/lib/spack/llnl/util/symlink.py +++ b/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() - tty.error(err) - raise SymlinkError("Make junction command returned a non-zero return code.", err) + err_str = err.decode() + tty.error(err_str) + 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: diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 4292d6449d..8fe272db7d 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/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) diff --git a/lib/spack/spack/installer.py b/lib/spack/spack/installer.py index 289a48568d..09eeecaca5 100644 --- a/lib/spack/spack/installer.py +++ b/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() diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 509d610225..357dd92f84 100644 --- a/lib/spack/spack/relocate.py +++ b/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. diff --git a/lib/spack/spack/util/archive.py b/lib/spack/spack/util/archive.py index 8bde40017c..48e624dee7 100644 --- a/lib/spack/spack/util/archive.py +++ b/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 - file_info.linkname = os.readlink(entry.path) + # strip off long path reg prefix on Windows + 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):