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.
This commit is contained in:
John W. Parent 2023-09-15 15:55:18 -04:00 committed by GitHub
parent 0280ac51ed
commit 060bc01273
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 23 deletions

View File

@ -11,6 +11,7 @@
import itertools import itertools
import numbers import numbers
import os import os
import pathlib
import posixpath import posixpath
import re import re
import shutil import shutil
@ -2426,7 +2427,7 @@ def library_dependents(self):
""" """
Set of directories where package binaries/libraries are located. 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): def add_library_dependent(self, *dest):
""" """
@ -2439,9 +2440,9 @@ def add_library_dependent(self, *dest):
""" """
for pth in dest: for pth in dest:
if os.path.isfile(pth): if os.path.isfile(pth):
self._additional_library_dependents.add(os.path.dirname) self._additional_library_dependents.add(pathlib.Path(pth).parent)
else: else:
self._additional_library_dependents.add(pth) self._additional_library_dependents.add(pathlib.Path(pth))
@property @property
def rpaths(self): def rpaths(self):
@ -2454,7 +2455,7 @@ def rpaths(self):
dependent_libs.extend(list(find_all_shared_libraries(path, recursive=True))) dependent_libs.extend(list(find_all_shared_libraries(path, recursive=True)))
for extra_path in self._addl_rpaths: for extra_path in self._addl_rpaths:
dependent_libs.extend(list(find_all_shared_libraries(extra_path, recursive=True))) 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): def add_rpath(self, *paths):
""" """
@ -2470,7 +2471,7 @@ def add_rpath(self, *paths):
""" """
self._addl_rpaths = self._addl_rpaths | set(paths) 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 """Perform link step of simulated rpathing, installing
simlinks of file in path to the dest_dir simlinks of file in path to the dest_dir
location. This method deliberately prevents location. This method deliberately prevents
@ -2478,27 +2479,35 @@ def _link(self, path, dest_dir):
This is because it is both meaningless from an rpath This is because it is both meaningless from an rpath
perspective, and will cause an error when Developer perspective, and will cause an error when Developer
mode is not enabled""" mode is not enabled"""
file_name = os.path.basename(path)
dest_file = os.path.join(dest_dir, file_name) def report_already_linked():
if os.path.exists(dest_dir) and not dest_file == path:
try:
symlink(path, 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)
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 # We have either already symlinked or we are encoutering a naming clash
# either way, we don't want to overwrite existing libraries # either way, we don't want to overwrite existing libraries
already_linked = islink(dest_file) already_linked = islink(str(dest_file))
tty.debug( tty.debug(
"Linking library %s to %s failed, " % (path, dest_file) + "already linked." "Linking library %s to %s failed, " % (str(path), str(dest_file))
+ "already linked."
if already_linked if already_linked
else "library with name %s already exists at location %s." else "library with name %s already exists at location %s."
% (file_name, dest_dir) % (str(file_name), str(dest_dir))
) )
pass
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(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):
report_already_linked()
else: else:
raise e raise e
# catch errors we raise ourselves from Spack
except llnl.util.symlink.AlreadyExistsError:
report_already_linked()
def establish_link(self): def establish_link(self):
""" """

View File

@ -66,7 +66,9 @@ def symlink(source_path: str, link_path: str, allow_broken_symlinks: bool = not
if not allow_broken_symlinks: if not allow_broken_symlinks:
# Perform basic checks to make sure symlinking will succeed # Perform basic checks to make sure symlinking will succeed
if os.path.lexists(link_path): 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 not os.path.exists(source_path):
if os.path.isabs(source_path) and not allow_broken_symlinks: 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: else:
# os.symlink can create a link when the given source path is relative to # 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 # 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. # links from being made.
link_parent_dir = os.path.dirname(link_path) link_parent_dir = os.path.dirname(link_path)
relative_path = os.path.join(link_parent_dir, source_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): elif not os.path.exists(source):
raise SymlinkError("Source path does not exist, cannot create a junction.") raise SymlinkError("Source path does not exist, cannot create a junction.")
elif os.path.lexists(link): 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): elif not os.path.isdir(source):
raise SymlinkError("Source path is not a directory, cannot create a junction.") 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): elif not os.path.exists(path):
raise SymlinkError(f"File path {path} does not exist. Cannot create hard link.") raise SymlinkError(f"File path {path} does not exist. Cannot create hard link.")
elif os.path.lexists(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): elif not os.path.isfile(path):
raise SymlinkError(f"File path ({link}) is not a file. Cannot create hard link.") raise SymlinkError(f"File path ({link}) is not a file. Cannot create hard link.")
else: else:
@ -340,3 +342,7 @@ class SymlinkError(SpackError):
"""Exception class for errors raised while creating symlinks, """Exception class for errors raised while creating symlinks,
junctions and hard links junctions and hard links
""" """
class AlreadyExistsError(SymlinkError):
"""Link path already exists."""