From 5bd184aaafd94d8dd8601c7c0d924948e636e8f3 Mon Sep 17 00:00:00 2001 From: "John W. Parent" <45471568+johnwparent@users.noreply.github.com> Date: Thu, 27 Feb 2025 22:16:00 -0500 Subject: [PATCH] Windows Rpath: Allow package test rpaths (#47072) On Windows, libraries search their directory for dependencies, and we help libraries in Spack-built packages locate their dependencies by symlinking them into the dependent's directory (we refer to this as simulated RPATHing). We extend the convenience functionality here to support base library directories outside of the package prefix: this is primarily for running tests in the build directory (which is not located inside of the final install prefix chosen by spack). --- lib/spack/llnl/util/filesystem.py | 86 +++++++++++++++++-- lib/spack/spack/package_base.py | 8 +- .../builtin/packages/libcatalyst/package.py | 20 ++++- 3 files changed, 102 insertions(+), 12 deletions(-) diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index cd4d8484c67..80e7d4622ee 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -2454,26 +2454,69 @@ class WindowsSimulatedRPath: and vis versa. """ - def __init__(self, package, link_install_prefix=True): + def __init__( + self, + package, + base_modification_prefix: Optional[Union[str, pathlib.Path]] = None, + link_install_prefix: bool = True, + ): """ Args: package (spack.package_base.PackageBase): Package requiring links + base_modification_prefix (str|pathlib.Path): Path representation indicating + the root directory in which to establish the simulated rpath, ie where the + symlinks that comprise the "rpath" behavior will be installed. + + Note: This is a mutually exclusive option with `link_install_prefix` using + both is an error. + + Default: None link_install_prefix (bool): Link against package's own install or stage root. Packages that run their own executables during build and require rpaths to - the build directory during build time require this option. Default: install + the build directory during build time require this option. + + Default: install root + + Note: This is a mutually exclusive option with `base_modification_prefix`, using + both is an error. """ self.pkg = package - self._addl_rpaths = set() + self._addl_rpaths: set[str] = set() + if link_install_prefix and base_modification_prefix: + raise RuntimeError( + "Invalid combination of arguments given to WindowsSimulated RPath.\n" + "Select either `link_install_prefix` to create an install prefix rpath" + " or specify a `base_modification_prefix` for any other link type. " + "Specifying both arguments is invalid." + ) + if not (link_install_prefix or base_modification_prefix): + raise RuntimeError( + "Insufficient arguments given to WindowsSimulatedRpath.\n" + "WindowsSimulatedRPath requires one of link_install_prefix" + " or base_modification_prefix to be specified." + " Neither was provided." + ) + self.link_install_prefix = link_install_prefix - self._additional_library_dependents = set() + if base_modification_prefix: + self.base_modification_prefix = pathlib.Path(base_modification_prefix) + else: + self.base_modification_prefix = pathlib.Path(self.pkg.prefix) + self._additional_library_dependents: set[pathlib.Path] = set() + if not self.link_install_prefix: + tty.debug(f"Generating rpath for non install context: {base_modification_prefix}") @property def library_dependents(self): """ Set of directories where package binaries/libraries are located. """ - return set([pathlib.Path(self.pkg.prefix.bin)]) | self._additional_library_dependents + base_pths = set() + if self.link_install_prefix: + base_pths.add(pathlib.Path(self.pkg.prefix.bin)) + base_pths |= self._additional_library_dependents + return base_pths def add_library_dependent(self, *dest): """ @@ -2489,6 +2532,12 @@ def add_library_dependent(self, *dest): new_pth = pathlib.Path(pth).parent else: new_pth = pathlib.Path(pth) + path_is_in_prefix = new_pth.is_relative_to(self.base_modification_prefix) + if not path_is_in_prefix: + raise RuntimeError( + f"Attempting to generate rpath symlink out of rpath context:\ +{str(self.base_modification_prefix)}" + ) self._additional_library_dependents.add(new_pth) @property @@ -2577,6 +2626,33 @@ def establish_link(self): self._link(library, lib_dir) +def make_package_test_rpath(pkg, test_dir: Union[str, pathlib.Path]): + """Establishes a temp Windows simulated rpath for the pkg in the testing directory + so an executable can test the libraries/executables with proper access + to dependent dlls + + Note: this is a no-op on all other platforms besides Windows + + Args: + pkg (spack.package_base.PackageBase): the package for which the rpath should be computed + test_dir: the testing directory in which we should construct an rpath + """ + # link_install_prefix as false ensures we're not linking into the install prefix + mini_rpath = WindowsSimulatedRPath(pkg, link_install_prefix=False) + # add the testing directory as a location to install rpath symlinks + mini_rpath.add_library_dependent(test_dir) + + # check for whether build_directory is available, if not + # assume the stage root is the build dir + build_dir_attr = getattr(pkg, "build_directory", None) + build_directory = build_dir_attr if build_dir_attr else pkg.stage.path + # add the build dir & build dir bin + mini_rpath.add_rpath(os.path.join(build_directory, "bin")) + mini_rpath.add_rpath(os.path.join(build_directory)) + # construct rpath + mini_rpath.establish_link() + + @system_path_filter @memoized def can_access_dir(path): diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index 9a120776160..04f13d5d603 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -125,9 +125,10 @@ def windows_establish_runtime_linkage(self): # Spack should in general not modify things it has not installed # we can reasonably expect externals to have their link interface properly established if sys.platform == "win32" and not self.spec.external: - self.win_rpath.add_library_dependent(*self.win_add_library_dependent()) - self.win_rpath.add_rpath(*self.win_add_rpath()) - self.win_rpath.establish_link() + win_rpath = fsys.WindowsSimulatedRPath(self) + win_rpath.add_library_dependent(*self.win_add_library_dependent()) + win_rpath.add_rpath(*self.win_add_rpath()) + win_rpath.establish_link() #: Registers which are the detectable packages, by repo and package name @@ -742,7 +743,6 @@ def __init__(self, spec): # Set up timing variables self._fetch_time = 0.0 - self.win_rpath = fsys.WindowsSimulatedRPath(self) super().__init__() def __getitem__(self, key: str) -> "PackageBase": diff --git a/var/spack/repos/builtin/packages/libcatalyst/package.py b/var/spack/repos/builtin/packages/libcatalyst/package.py index 2b32d8c9849..27ac3ff229c 100644 --- a/var/spack/repos/builtin/packages/libcatalyst/package.py +++ b/var/spack/repos/builtin/packages/libcatalyst/package.py @@ -3,6 +3,10 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import subprocess +import sys + +import llnl.util.filesystem as fsys +import llnl.util.tty as tty from spack.package import * @@ -57,19 +61,29 @@ def setup_run_environment(self, env): @on_package_attributes(run_tests=True) @run_after("install") def build_test(self): - testdir = "smoke_test_build" + testdir = join_path(self.stage.source_path, "smoke_test_build") cmakeExampleDir = join_path(self.stage.source_path, "examples") cmake_args = [ cmakeExampleDir, "-DBUILD_SHARED_LIBS=ON", self.define("CMAKE_PREFIX_PATH", self.prefix), ] + adapter0_test_path = join_path(testdir, "adaptor0/adaptor0_test") + if sys.platform == "win32": + # Specify ninja generator for `cmake` call used to generate test artifact + # (this differs from the build of `libcatalyst` itself); if unspecified, the + # default is to use Visual Studio, which generates a more-complex path + # (adapter0//adaptor0_test rather than adaptor0/adaptor0_test). + cmake_args.append("-GNinja") + # To run the test binary on Windows, we need to construct an rpath + # for the current package being tested, including the package + # itself + fsys.make_package_test_rpath(self, adapter0_test_path) cmake = which(self.spec["cmake"].prefix.bin.cmake) with working_dir(testdir, create=True): cmake(*cmake_args) cmake(*(["--build", "."])) tty.info("Running Catalyst test") - - res = subprocess.run(["adaptor0/adaptor0_test", "catalyst"]) + res = subprocess.run([adapter0_test_path, "catalyst"]) assert res.returncode == 0