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.

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