diff options
| -rw-r--r-- | lib/spack/llnl/util/filesystem.py | 20 | ||||
| -rw-r--r-- | lib/spack/spack/binary_distribution.py | 4 | ||||
| -rw-r--r-- | lib/spack/spack/relocate.py | 11 | ||||
| -rw-r--r-- | lib/spack/spack/test/llnl/util/filesystem.py | 11 | ||||
| -rw-r--r-- | lib/spack/spack/util/filesystem.py | 16 |
5 files changed, 49 insertions, 13 deletions
diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index a876a76c27..4732924572 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -24,6 +24,7 @@ from typing import ( Callable, Deque, Dict, + Generator, Iterable, List, Match, @@ -2838,6 +2839,25 @@ def temporary_dir( remove_directory_contents(tmp_dir) +@contextmanager +def edit_in_place_through_temporary_file(file_path: str) -> Generator[str, None, None]: + """Context manager for modifying ``file_path`` in place, preserving its inode and hardlinks, + for functions or external tools that do not support in-place editing. Notice that this function + is unsafe in that it works with paths instead of a file descriptors, but this is by design, + since we assume the call site will create a new inode at the same path.""" + tmp_fd, tmp_path = tempfile.mkstemp( + dir=os.path.dirname(file_path), prefix=f"{os.path.basename(file_path)}." + ) + # windows cannot replace a file with open fds, so close since the call site needs to replace. + os.close(tmp_fd) + try: + shutil.copyfile(file_path, tmp_path, follow_symlinks=True) + yield tmp_path + shutil.copyfile(tmp_path, file_path, follow_symlinks=True) + finally: + os.unlink(tmp_path) + + def filesummary(path, print_bytes=16) -> Tuple[int, bytes]: """Create a small summary of the given file. Does not error when file does not exist. diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index db26f32c00..da8f7d0364 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -2334,7 +2334,9 @@ def relocate_package(spec): if not codesign: return for binary in changed_files: - codesign("-fs-", binary) + # preserve the original inode by running codesign on a copy + with fsys.edit_in_place_through_temporary_file(binary) as tmp_binary: + codesign("-fs-", tmp_binary) # If we are installing back to the same location # relocate the sbang location if the spack directory changed diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 627c9e2b05..dda17a128e 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -13,6 +13,7 @@ from typing import List, Optional import macholib.mach_o import macholib.MachO +import llnl.util.filesystem as fs import llnl.util.lang import llnl.util.tty as tty from llnl.util.lang import memoized @@ -275,10 +276,10 @@ def modify_macho_object(cur_path, rpaths, deps, idpath, paths_to_paths): # Deduplicate and flatten args = list(itertools.chain.from_iterable(llnl.util.lang.dedupe(args))) + install_name_tool = executable.Executable("install_name_tool") if args: - args.append(str(cur_path)) - install_name_tool = executable.Executable("install_name_tool") - install_name_tool(*args) + with fs.edit_in_place_through_temporary_file(cur_path) as temp_path: + install_name_tool(*args, temp_path) def macholib_get_paths(cur_path): @@ -717,8 +718,8 @@ def fixup_macos_rpath(root, filename): # No fixes needed return False - args.append(abspath) - executable.Executable("install_name_tool")(*args) + with fs.edit_in_place_through_temporary_file(abspath) as temp_path: + executable.Executable("install_name_tool")(*args, temp_path) return True diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index 1a32e5707c..8b512a2c5f 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -1249,3 +1249,14 @@ def test_find_input_types(tmp_path: pathlib.Path): with pytest.raises(TypeError): fs.find(1, "file.txt") # type: ignore + + +def test_edit_in_place_through_temporary_file(tmp_path): + (tmp_path / "example.txt").write_text("Hello") + current_ino = os.stat(tmp_path / "example.txt").st_ino + with fs.edit_in_place_through_temporary_file(tmp_path / "example.txt") as temporary: + os.unlink(temporary) + with open(temporary, "w") as f: + f.write("World") + assert (tmp_path / "example.txt").read_text() == "World" + assert os.stat(tmp_path / "example.txt").st_ino == current_ino diff --git a/lib/spack/spack/util/filesystem.py b/lib/spack/spack/util/filesystem.py index b296438fe8..eac5c58918 100644 --- a/lib/spack/spack/util/filesystem.py +++ b/lib/spack/spack/util/filesystem.py @@ -13,7 +13,7 @@ import os import sys from llnl.util import tty -from llnl.util.filesystem import join_path +from llnl.util.filesystem import edit_in_place_through_temporary_file from llnl.util.lang import memoized from spack.util.executable import Executable, which @@ -81,12 +81,11 @@ def fix_darwin_install_name(path): Parameters: path (str): directory in which .dylib files are located """ - libs = glob.glob(join_path(path, "*.dylib")) + libs = glob.glob(os.path.join(path, "*.dylib")) + install_name_tool = Executable("install_name_tool") + otool = Executable("otool") for lib in libs: - # fix install name first: - install_name_tool = Executable("install_name_tool") - install_name_tool("-id", lib, lib) - otool = Executable("otool") + args = ["-id", lib] long_deps = otool("-L", lib, output=str).split("\n") deps = [dep.partition(" ")[0][1::] for dep in long_deps[2:-1]] # fix all dependencies: @@ -98,5 +97,8 @@ def fix_darwin_install_name(path): # but we don't know builddir (nor how symbolic links look # in builddir). We thus only compare the basenames. if os.path.basename(dep) == os.path.basename(loc): - install_name_tool("-change", dep, loc, lib) + args.extend(("-change", dep, loc)) break + + with edit_in_place_through_temporary_file(lib) as tmp: + install_name_tool(*args, tmp) |