From 060bc012734e8d0ecd23910d89496aab7c25056f Mon Sep 17 00:00:00 2001 From: "John W. Parent" <45471568+johnwparent@users.noreply.github.com> Date: Fri, 15 Sep 2023 15:55:18 -0400 Subject: Windows RPATHing: fix symlink error (#39933) With 349ba83, you cannot symlink() if the link already exists. Update the simulated RPATHing logic on Windows to account for that. --- lib/spack/llnl/util/filesystem.py | 47 +++++++++++++++++++++++---------------- lib/spack/llnl/util/symlink.py | 14 ++++++++---- 2 files changed, 38 insertions(+), 23 deletions(-) diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index a23053df9c..bd203ef200 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -11,6 +11,7 @@ import hashlib import itertools import numbers import os +import pathlib import posixpath import re import shutil @@ -2426,7 +2427,7 @@ class WindowsSimulatedRPath: """ Set of directories where package binaries/libraries are located. """ - return set([self.pkg.prefix.bin]) | self._additional_library_dependents + return set([pathlib.Path(self.pkg.prefix.bin)]) | self._additional_library_dependents def add_library_dependent(self, *dest): """ @@ -2439,9 +2440,9 @@ class WindowsSimulatedRPath: """ for pth in dest: if os.path.isfile(pth): - self._additional_library_dependents.add(os.path.dirname) + self._additional_library_dependents.add(pathlib.Path(pth).parent) else: - self._additional_library_dependents.add(pth) + self._additional_library_dependents.add(pathlib.Path(pth)) @property def rpaths(self): @@ -2454,7 +2455,7 @@ class WindowsSimulatedRPath: dependent_libs.extend(list(find_all_shared_libraries(path, recursive=True))) for extra_path in self._addl_rpaths: dependent_libs.extend(list(find_all_shared_libraries(extra_path, recursive=True))) - return set(dependent_libs) + return set([pathlib.Path(x) for x in dependent_libs]) def add_rpath(self, *paths): """ @@ -2470,7 +2471,7 @@ class WindowsSimulatedRPath: """ self._addl_rpaths = self._addl_rpaths | set(paths) - def _link(self, path, dest_dir): + def _link(self, path: pathlib.Path, dest_dir: pathlib.Path): """Perform link step of simulated rpathing, installing simlinks of file in path to the dest_dir location. This method deliberately prevents @@ -2478,27 +2479,35 @@ class WindowsSimulatedRPath: This is because it is both meaningless from an rpath perspective, and will cause an error when Developer mode is not enabled""" - file_name = os.path.basename(path) - dest_file = os.path.join(dest_dir, file_name) - if os.path.exists(dest_dir) and not dest_file == path: + + def report_already_linked(): + # We have either already symlinked or we are encoutering a naming clash + # either way, we don't want to overwrite existing libraries + already_linked = islink(str(dest_file)) + tty.debug( + "Linking library %s to %s failed, " % (str(path), str(dest_file)) + + "already linked." + if already_linked + else "library with name %s already exists at location %s." + % (str(file_name), str(dest_dir)) + ) + + file_name = path.name + dest_file = dest_dir / file_name + if not dest_file.exists() and dest_dir.exists() and not dest_file == path: try: - symlink(path, dest_file) + symlink(str(path), str(dest_file)) # For py2 compatibility, we have to catch the specific Windows error code # associate with trying to create a file that already exists (winerror 183) + # Catch OSErrors missed by the SymlinkError checks except OSError as e: if sys.platform == "win32" and (e.winerror == 183 or e.errno == errno.EEXIST): - # We have either already symlinked or we are encoutering a naming clash - # either way, we don't want to overwrite existing libraries - already_linked = islink(dest_file) - tty.debug( - "Linking library %s to %s failed, " % (path, dest_file) + "already linked." - if already_linked - else "library with name %s already exists at location %s." - % (file_name, dest_dir) - ) - pass + report_already_linked() else: raise e + # catch errors we raise ourselves from Spack + except llnl.util.symlink.AlreadyExistsError: + report_already_linked() def establish_link(self): """ diff --git a/lib/spack/llnl/util/symlink.py b/lib/spack/llnl/util/symlink.py index d1084a13fe..ab6654e847 100644 --- a/lib/spack/llnl/util/symlink.py +++ b/lib/spack/llnl/util/symlink.py @@ -66,7 +66,9 @@ def symlink(source_path: str, link_path: str, allow_broken_symlinks: bool = not if not allow_broken_symlinks: # Perform basic checks to make sure symlinking will succeed if os.path.lexists(link_path): - raise SymlinkError(f"Link path ({link_path}) already exists. Cannot create link.") + raise AlreadyExistsError( + f"Link path ({link_path}) already exists. Cannot create link." + ) if not os.path.exists(source_path): if os.path.isabs(source_path) and not allow_broken_symlinks: @@ -78,7 +80,7 @@ def symlink(source_path: str, link_path: str, allow_broken_symlinks: bool = not else: # os.symlink can create a link when the given source path is relative to # the link path. Emulate this behavior and check to see if the source exists - # relative to the link patg ahead of link creation to prevent broken + # relative to the link path ahead of link creation to prevent broken # links from being made. link_parent_dir = os.path.dirname(link_path) relative_path = os.path.join(link_parent_dir, source_path) @@ -234,7 +236,7 @@ def _windows_create_junction(source: str, link: str): elif not os.path.exists(source): raise SymlinkError("Source path does not exist, cannot create a junction.") elif os.path.lexists(link): - raise SymlinkError("Link path already exists, cannot create a junction.") + raise AlreadyExistsError("Link path already exists, cannot create a junction.") elif not os.path.isdir(source): raise SymlinkError("Source path is not a directory, cannot create a junction.") @@ -259,7 +261,7 @@ def _windows_create_hard_link(path: str, link: str): elif not os.path.exists(path): raise SymlinkError(f"File path {path} does not exist. Cannot create hard link.") elif os.path.lexists(link): - raise SymlinkError(f"Link path ({link}) already exists. Cannot create hard link.") + raise AlreadyExistsError(f"Link path ({link}) already exists. Cannot create hard link.") elif not os.path.isfile(path): raise SymlinkError(f"File path ({link}) is not a file. Cannot create hard link.") else: @@ -340,3 +342,7 @@ class SymlinkError(SpackError): """Exception class for errors raised while creating symlinks, junctions and hard links """ + + +class AlreadyExistsError(SymlinkError): + """Link path already exists.""" -- cgit v1.2.3-60-g2f50