relocate.py, binary_distribution.py: cleanup (#48651)

lib/spack/spack/binary_distribution.py

@@ -2166,7 +2166,8 @@ def dedupe_hardlinks_if_necessary(root, buildinfo):
 
 def relocate_package(spec: spack.spec.Spec) -> None:
     """Relocate binaries and text files in the given spec prefix, based on its buildinfo file."""
-    buildinfo = read_buildinfo_file(spec.prefix)
+    spec_prefix = str(spec.prefix)
+    buildinfo = read_buildinfo_file(spec_prefix)
     old_layout_root = str(buildinfo["buildpath"])
 
     # Warn about old style tarballs created with the --rel flag (removed in Spack v0.20)
@@ -2187,7 +2188,7 @@ def relocate_package(spec: spack.spec.Spec) -> None:
             "and an older buildcache create implementation. It cannot be relocated."
         )
 
-    prefix_to_prefix = {}
+    prefix_to_prefix: Dict[str, str] = {}
 
     if "sbang_install_path" in buildinfo:
         old_sbang_install_path = str(buildinfo["sbang_install_path"])
@@ -2239,12 +2240,12 @@ def relocate_package(spec: spack.spec.Spec) -> None:
         tty.debug(f"Relocating: {old} => {new}.")
 
     # Old archives may have hardlinks repeated.
-    dedupe_hardlinks_if_necessary(spec.prefix, buildinfo)
+    dedupe_hardlinks_if_necessary(spec_prefix, buildinfo)
 
     # Text files containing the prefix text
-    textfiles = [os.path.join(spec.prefix, f) for f in buildinfo["relocate_textfiles"]]
+    textfiles = [os.path.join(spec_prefix, f) for f in buildinfo["relocate_textfiles"]]
-    binaries = [os.path.join(spec.prefix, f) for f in buildinfo.get("relocate_binaries")]
+    binaries = [os.path.join(spec_prefix, f) for f in buildinfo.get("relocate_binaries")]
-    links = [os.path.join(spec.prefix, f) for f in buildinfo.get("relocate_links", [])]
+    links = [os.path.join(spec_prefix, f) for f in buildinfo.get("relocate_links", [])]
 
     platform = spack.platforms.by_name(spec.platform)
     if "macho" in platform.binary_formats:

lib/spack/spack/filesystem_view.py

@@ -89,10 +89,10 @@ def view_copy(
     if stat.S_ISLNK(src_stat.st_mode):
         spack.relocate.relocate_links(links=[dst], prefix_to_prefix=prefix_to_projection)
     elif spack.relocate.is_binary(dst):
-        spack.relocate.relocate_text_bin(binaries=[dst], prefixes=prefix_to_projection)
+        spack.relocate.relocate_text_bin(binaries=[dst], prefix_to_prefix=prefix_to_projection)
     else:
         prefix_to_projection[spack.store.STORE.layout.root] = view._root
-        spack.relocate.relocate_text(files=[dst], prefixes=prefix_to_projection)
+        spack.relocate.relocate_text(files=[dst], prefix_to_prefix=prefix_to_projection)
 
     # The os module on Windows does not have a chown function.
     if sys.platform != "win32":

lib/spack/spack/relocate.py

@@ -6,8 +6,7 @@
 import os
 import re
 import sys
-from collections import OrderedDict
+from typing import Dict, Iterable, List, Optional
-from typing import List, Optional
 
 import macholib.mach_o
 import macholib.MachO
@@ -18,28 +17,11 @@
 from llnl.util.lang import memoized
 from llnl.util.symlink import readlink, symlink
 
-import spack.error
 import spack.store
 import spack.util.elf as elf
 import spack.util.executable as executable
 
-from .relocate_text import BinaryFilePrefixReplacer, TextFilePrefixReplacer
+from .relocate_text import BinaryFilePrefixReplacer, PrefixToPrefix, TextFilePrefixReplacer
-
-
-class InstallRootStringError(spack.error.SpackError):
-    def __init__(self, file_path, root_path):
-        """Signal that the relocated binary still has the original
-        Spack's store root string
-
-        Args:
-            file_path (str): path of the binary
-            root_path (str): original Spack's store root string
-        """
-        super().__init__(
-            "\n %s \ncontains string\n %s \n"
-            "after replacing it in rpaths.\n"
-            "Package should not be relocated.\n Use -a to override." % (file_path, root_path)
-        )
 
 
 @memoized
@@ -58,7 +40,7 @@ def _decode_macho_data(bytestring):
     return bytestring.rstrip(b"\x00").decode("ascii")
 
 
-def macho_find_paths(orig_rpaths, deps, idpath, prefix_to_prefix):
+def _macho_find_paths(orig_rpaths, deps, idpath, prefix_to_prefix):
     """
     Inputs
     original rpaths from mach-o binaries
@@ -103,7 +85,7 @@ def macho_find_paths(orig_rpaths, deps, idpath, prefix_to_prefix):
     return paths_to_paths
 
 
-def modify_macho_object(cur_path, rpaths, deps, idpath, paths_to_paths):
+def _modify_macho_object(cur_path, rpaths, deps, idpath, paths_to_paths):
     """
     This function is used to make machO buildcaches on macOS by
     replacing old paths with new paths using install_name_tool
@@ -146,7 +128,7 @@ def modify_macho_object(cur_path, rpaths, deps, idpath, paths_to_paths):
             install_name_tool(*args, temp_path)
 
 
-def macholib_get_paths(cur_path):
+def _macholib_get_paths(cur_path):
     """Get rpaths, dependent libraries, and library id of mach-o objects."""
     headers = []
     try:
@@ -228,25 +210,25 @@ def relocate_macho_binaries(path_names, prefix_to_prefix):
         if path_name.endswith(".o"):
             continue
         # get the paths in the old prefix
-        rpaths, deps, idpath = macholib_get_paths(path_name)
+        rpaths, deps, idpath = _macholib_get_paths(path_name)
         # get the mapping of paths in the old prerix to the new prefix
-        paths_to_paths = macho_find_paths(rpaths, deps, idpath, prefix_to_prefix)
+        paths_to_paths = _macho_find_paths(rpaths, deps, idpath, prefix_to_prefix)
         # replace the old paths with new paths
-        modify_macho_object(path_name, rpaths, deps, idpath, paths_to_paths)
+        _modify_macho_object(path_name, rpaths, deps, idpath, paths_to_paths)
 
 
-def relocate_elf_binaries(binaries, prefix_to_prefix):
+def relocate_elf_binaries(binaries: Iterable[str], prefix_to_prefix: Dict[str, str]) -> None:
-    """Take a list of binaries, and an ordered dictionary of
+    """Take a list of binaries, and an ordered prefix to prefix mapping, and update the rpaths
-    prefix to prefix mapping, and update the rpaths accordingly."""
+    accordingly."""
 
     # Transform to binary string
-    prefix_to_prefix = OrderedDict(
+    prefix_to_prefix_bin = {
-        (k.encode("utf-8"), v.encode("utf-8")) for (k, v) in prefix_to_prefix.items()
+        k.encode("utf-8"): v.encode("utf-8") for k, v in prefix_to_prefix.items()
-    )
+    }
 
     for path in binaries:
         try:
-            elf.substitute_rpath_and_pt_interp_in_place_or_raise(path, prefix_to_prefix)
+            elf.substitute_rpath_and_pt_interp_in_place_or_raise(path, prefix_to_prefix_bin)
         except elf.ElfCStringUpdatesFailed as e:
             # Fall back to `patchelf --set-rpath ... --set-interpreter ...`
             rpaths = e.rpath.new_value.decode("utf-8").split(":") if e.rpath else []
@@ -254,13 +236,13 @@ def relocate_elf_binaries(binaries, prefix_to_prefix):
             _set_elf_rpaths_and_interpreter(path, rpaths=rpaths, interpreter=interpreter)
 
 
-def warn_if_link_cant_be_relocated(link, target):
+def _warn_if_link_cant_be_relocated(link: str, target: str):
     if not os.path.isabs(target):
         return
-    tty.warn('Symbolic link at "{}" to "{}" cannot be relocated'.format(link, target))
+    tty.warn(f'Symbolic link at "{link}" to "{target}" cannot be relocated')
 
 
-def relocate_links(links, prefix_to_prefix):
+def relocate_links(links: Iterable[str], prefix_to_prefix: Dict[str, str]) -> None:
     """Relocate links to a new install prefix."""
     regex = re.compile("|".join(re.escape(p) for p in prefix_to_prefix.keys()))
     for link in links:
@@ -269,7 +251,7 @@ def relocate_links(links, prefix_to_prefix):
 
         # No match.
         if match is None:
-            warn_if_link_cant_be_relocated(link, old_target)
+            _warn_if_link_cant_be_relocated(link, old_target)
             continue
 
         new_target = prefix_to_prefix[match.group()] + old_target[match.end() :]
@@ -277,32 +259,32 @@ def relocate_links(links, prefix_to_prefix):
         symlink(new_target, link)
 
 
-def relocate_text(files, prefixes):
+def relocate_text(files: Iterable[str], prefix_to_prefix: PrefixToPrefix) -> None:
     """Relocate text file from the original installation prefix to the
     new prefix.
 
     Relocation also affects the the path in Spack's sbang script.
 
     Args:
-        files (list): Text files to be relocated
+        files: Text files to be relocated
-        prefixes (OrderedDict): String prefixes which need to be changed
+        prefix_to_prefix: ordered prefix to prefix mapping
     """
-    TextFilePrefixReplacer.from_strings_or_bytes(prefixes).apply(files)
+    TextFilePrefixReplacer.from_strings_or_bytes(prefix_to_prefix).apply(files)
 
 
-def relocate_text_bin(binaries, prefixes):
+def relocate_text_bin(binaries: Iterable[str], prefix_to_prefix: PrefixToPrefix) -> List[str]:
     """Replace null terminated path strings hard-coded into binaries.
 
     The new install prefix must be shorter than the original one.
 
     Args:
-        binaries (list): binaries to be relocated
+        binaries: paths to binaries to be relocated
-        prefixes (OrderedDict): String prefixes which need to be changed.
+        prefix_to_prefix: ordered prefix to prefix mapping
 
     Raises:
       spack.relocate_text.BinaryTextReplaceError: when the new path is longer than the old path
     """
-    return BinaryFilePrefixReplacer.from_strings_or_bytes(prefixes).apply(binaries)
+    return BinaryFilePrefixReplacer.from_strings_or_bytes(prefix_to_prefix).apply(binaries)
 
 
 def is_macho_magic(magic: bytes) -> bool:
@@ -339,7 +321,7 @@ def _exists_dir(dirname):
     return os.path.isdir(dirname)
 
 
-def is_macho_binary(path):
+def is_macho_binary(path: str) -> bool:
     try:
         with open(path, "rb") as f:
             return is_macho_magic(f.read(4))
@@ -363,7 +345,7 @@ def fixup_macos_rpath(root, filename):
         return False
 
     # Get Mach-O header commands
-    (rpath_list, deps, id_dylib) = macholib_get_paths(abspath)
+    (rpath_list, deps, id_dylib) = _macholib_get_paths(abspath)
 
     # Convert rpaths list to (name -> number of occurrences)
     add_rpaths = set()

lib/spack/spack/relocate_text.py

@@ -6,64 +6,61 @@
 paths inside text files and binaries."""
 
 import re
-from collections import OrderedDict
+from typing import IO, Dict, Iterable, List, Union
-from typing import Dict, Union
+
+from llnl.util.lang import PatternBytes
 
 import spack.error
 
 Prefix = Union[str, bytes]
+PrefixToPrefix = Union[Dict[str, str], Dict[bytes, bytes]]
 
 
 def encode_path(p: Prefix) -> bytes:
     return p if isinstance(p, bytes) else p.encode("utf-8")
 
 
-def _prefix_to_prefix_as_bytes(prefix_to_prefix) -> Dict[bytes, bytes]:
+def _prefix_to_prefix_as_bytes(prefix_to_prefix: PrefixToPrefix) -> Dict[bytes, bytes]:
-    return OrderedDict((encode_path(k), encode_path(v)) for (k, v) in prefix_to_prefix.items())
+    return {encode_path(k): encode_path(v) for (k, v) in prefix_to_prefix.items()}
 
 
-def utf8_path_to_binary_regex(prefix: str):
+def utf8_path_to_binary_regex(prefix: str) -> PatternBytes:
     """Create a binary regex that matches the input path in utf8"""
     prefix_bytes = re.escape(prefix).encode("utf-8")
     return re.compile(b"(?<![\\w\\-_/])([\\w\\-_]*?)%s([\\w\\-_/]*)" % prefix_bytes)
 
 
-def _byte_strings_to_single_binary_regex(prefixes):
+def _byte_strings_to_single_binary_regex(prefixes: Iterable[bytes]) -> PatternBytes:
     all_prefixes = b"|".join(re.escape(p) for p in prefixes)
     return re.compile(b"(?<![\\w\\-_/])([\\w\\-_]*?)(%s)([\\w\\-_/]*)" % all_prefixes)
 
 
-def utf8_paths_to_single_binary_regex(prefixes):
+def utf8_paths_to_single_binary_regex(prefixes: Iterable[str]) -> PatternBytes:
     """Create a (binary) regex that matches any input path in utf8"""
     return _byte_strings_to_single_binary_regex(p.encode("utf-8") for p in prefixes)
 
 
-def filter_identity_mappings(prefix_to_prefix):
+def filter_identity_mappings(prefix_to_prefix: Dict[bytes, bytes]) -> Dict[bytes, bytes]:
     """Drop mappings that are not changed."""
     # NOTE: we don't guard against the following case:
     # [/abc/def -> /abc/def, /abc -> /x] *will* be simplified to
     # [/abc -> /x], meaning that after this simplification /abc/def will be
     # mapped to /x/def instead of /abc/def. This should not be a problem.
-    return OrderedDict((k, v) for (k, v) in prefix_to_prefix.items() if k != v)
+    return {k: v for k, v in prefix_to_prefix.items() if k != v}
 
 
 class PrefixReplacer:
-    """Base class for applying a prefix to prefix map
+    """Base class for applying a prefix to prefix map to a list of binaries or text files. Derived
-    to a list of binaries or text files.
+    classes implement _apply_to_file to do the actual work, which is different when it comes to
-    Child classes implement _apply_to_file to do the
-    actual work, which is different when it comes to
     binaries and text files."""
 
-    def __init__(self, prefix_to_prefix: Dict[bytes, bytes]):
+    def __init__(self, prefix_to_prefix: Dict[bytes, bytes]) -> None:
         """
         Arguments:
-
+            prefix_to_prefix: An ordered mapping from prefix to prefix. The order is relevant to
-            prefix_to_prefix (OrderedDict):
+                support substring fallbacks, for example
-
+                ``[("/first/sub", "/x"), ("/first", "/y")]`` will ensure /first/sub is matched and
-                A ordered mapping from prefix to prefix. The order is
+                replaced before /first.
-                relevant to support substring fallbacks, for example
-                [("/first/sub", "/x"), ("/first", "/y")] will ensure
-                /first/sub is matched and replaced before /first.
         """
         self.prefix_to_prefix = filter_identity_mappings(prefix_to_prefix)
 
@@ -74,7 +71,7 @@ def is_noop(self) -> bool:
         or there are no prefixes to replace."""
         return not self.prefix_to_prefix
 
-    def apply(self, filenames: list):
+    def apply(self, filenames: Iterable[str]) -> List[str]:
         """Returns a list of files that were modified"""
         changed_files = []
         if self.is_noop:
@@ -84,17 +81,20 @@ def apply(self, filenames: list):
                 changed_files.append(filename)
         return changed_files
 
-    def apply_to_filename(self, filename):
+    def apply_to_filename(self, filename: str) -> bool:
         if self.is_noop:
             return False
         with open(filename, "rb+") as f:
             return self.apply_to_file(f)
 
-    def apply_to_file(self, f):
+    def apply_to_file(self, f: IO[bytes]) -> bool:
         if self.is_noop:
             return False
         return self._apply_to_file(f)
 
+    def _apply_to_file(self, f: IO) -> bool:
+        raise NotImplementedError("Derived classes must implement this method")
+
 
 class TextFilePrefixReplacer(PrefixReplacer):
     """This class applies prefix to prefix mappings for relocation
@@ -112,13 +112,11 @@ def __init__(self, prefix_to_prefix: Dict[bytes, bytes]):
         self.regex = _byte_strings_to_single_binary_regex(self.prefix_to_prefix.keys())
 
     @classmethod
-    def from_strings_or_bytes(
+    def from_strings_or_bytes(cls, prefix_to_prefix: PrefixToPrefix) -> "TextFilePrefixReplacer":
-        cls, prefix_to_prefix: Dict[Prefix, Prefix]
-    ) -> "TextFilePrefixReplacer":
         """Create a TextFilePrefixReplacer from an ordered prefix to prefix map."""
         return cls(_prefix_to_prefix_as_bytes(prefix_to_prefix))
 
-    def _apply_to_file(self, f):
+    def _apply_to_file(self, f: IO) -> bool:
         """Text replacement implementation simply reads the entire file
         in memory and applies the combined regex."""
         replacement = lambda m: m.group(1) + self.prefix_to_prefix[m.group(2)] + m.group(3)
@@ -133,12 +131,12 @@ def _apply_to_file(self, f):
 
 
 class BinaryFilePrefixReplacer(PrefixReplacer):
-    def __init__(self, prefix_to_prefix, suffix_safety_size=7):
+    def __init__(self, prefix_to_prefix: Dict[bytes, bytes], suffix_safety_size: int = 7) -> None:
         """
-        prefix_to_prefix (OrderedDict): OrderedDictionary where the keys are
+        prefix_to_prefix: Ordered dictionary where the keys are bytes representing the old prefixes
-            bytes representing the old prefixes and the values are the new
+            and the values are the new
-        suffix_safety_size (int): in case of null terminated strings, what size
+        suffix_safety_size: in case of null terminated strings, what size of the suffix should
-            of the suffix should remain to avoid aliasing issues?
+            remain to avoid aliasing issues?
         """
         assert suffix_safety_size >= 0
         super().__init__(prefix_to_prefix)
@@ -146,17 +144,18 @@ def __init__(self, prefix_to_prefix, suffix_safety_size=7):
         self.regex = self.binary_text_regex(self.prefix_to_prefix.keys(), suffix_safety_size)
 
     @classmethod
-    def binary_text_regex(cls, binary_prefixes, suffix_safety_size=7):
+    def binary_text_regex(
-        """
+        cls, binary_prefixes: Iterable[bytes], suffix_safety_size: int = 7
-        Create a regex that looks for exact matches of prefixes, and also tries to
+    ) -> PatternBytes:
-        match a C-string type null terminator in a small lookahead window.
+        """Create a regex that looks for exact matches of prefixes, and also tries to match a
+        C-string type null terminator in a small lookahead window.
 
         Arguments:
-            binary_prefixes (list): List of byte strings of prefixes to match
+            binary_prefixes: Iterable of byte strings of prefixes to match
-            suffix_safety_size (int): Sizeof the lookahed for null-terminated string.
+            suffix_safety_size: Sizeof the lookahed for null-terminated string.
-
-        Returns: compiled regex
         """
+        # Note: it's important not to use capture groups for the prefix, since it destroys
+        # performance due to common prefix optimization.
         return re.compile(
             b"("
             + b"|".join(re.escape(p) for p in binary_prefixes)
@@ -165,36 +164,34 @@ def binary_text_regex(cls, binary_prefixes, suffix_safety_size=7):
 
     @classmethod
     def from_strings_or_bytes(
-        cls, prefix_to_prefix: Dict[Prefix, Prefix], suffix_safety_size: int = 7
+        cls, prefix_to_prefix: PrefixToPrefix, suffix_safety_size: int = 7
     ) -> "BinaryFilePrefixReplacer":
         """Create a BinaryFilePrefixReplacer from an ordered prefix to prefix map.
 
         Arguments:
-            prefix_to_prefix (OrderedDict): Ordered mapping of prefix to prefix.
+            prefix_to_prefix: Ordered mapping of prefix to prefix.
-            suffix_safety_size (int): Number of bytes to retain at the end of a C-string
+            suffix_safety_size: Number of bytes to retain at the end of a C-string to avoid binary
-                to avoid binary string-aliasing issues.
+                string-aliasing issues.
         """
         return cls(_prefix_to_prefix_as_bytes(prefix_to_prefix), suffix_safety_size)
 
-    def _apply_to_file(self, f):
+    def _apply_to_file(self, f: IO[bytes]) -> bool:
         """
-        Given a file opened in rb+ mode, apply the string replacements as
+        Given a file opened in rb+ mode, apply the string replacements as specified by an ordered
-        specified by an ordered dictionary of prefix to prefix mappings. This
+        dictionary of prefix to prefix mappings. This method takes special care of null-terminated
-        method takes special care of null-terminated C-strings. C-string constants
+        C-strings. C-string constants are problematic because compilers and linkers optimize
-        are problematic because compilers and linkers optimize readonly strings for
+        readonly strings for space by aliasing those that share a common suffix (only suffix since
-        space by aliasing those that share a common suffix (only suffix since all
+        all of them are null terminated). See https://github.com/spack/spack/pull/31739 and
-        of them are null terminated). See https://github.com/spack/spack/pull/31739
+        https://github.com/spack/spack/pull/32253 for details. Our logic matches the original
-        and https://github.com/spack/spack/pull/32253 for details. Our logic matches
+        prefix with a ``suffix_safety_size + 1`` lookahead for null bytes. If no null terminator
-        the original prefix with a ``suffix_safety_size + 1`` lookahead for null bytes.
+        is found, we simply pad with leading /, assuming that it's a long C-string; the full
-        If no null terminator is found, we simply pad with leading /, assuming that
+        C-string after replacement has a large suffix in common with its original value. If there
-        it's a long C-string; the full C-string after replacement has a large suffix
+        *is* a null terminator we can do the same as long as the replacement has a sufficiently
-        in common with its original value.
+        long common suffix with the original prefix. As a last resort when the replacement does
-        If there *is* a null terminator we can do the same as long as the replacement
+        not have a long enough common suffix, we can try to shorten the string, but this only
-        has a sufficiently long common suffix with the original prefix.
+        works if the new length is sufficiently short (typically the case when going from large
-        As a last resort when the replacement does not have a long enough common suffix,
+        padding -> normal path) If the replacement string is longer, or all of the above fails,
-        we can try to shorten the string, but this only works if the new length is
+        we error out.
-        sufficiently short (typically the case when going from large padding -> normal path)
-        If the replacement string is longer, or all of the above fails, we error out.
 
         Arguments:
             f: file opened in rb+ mode
@@ -204,9 +201,8 @@ def _apply_to_file(self, f):
         """
         assert f.tell() == 0
 
-        # We *could* read binary data in chunks to avoid loading all in memory,
+        # We *could* read binary data in chunks to avoid loading all in memory, but it's nasty to
-        # but it's nasty to deal with matches across boundaries, so let's stick to
+        # deal with matches across boundaries, so let's stick to something simple.
-        # something simple.
 
         modified = False
 
@@ -218,8 +214,7 @@ def _apply_to_file(self, f):
             # Did we find a trailing null within a N + 1 bytes window after the prefix?
             null_terminated = match.end(0) > match.end(1)
 
-            # Suffix string length, excluding the null byte
+            # Suffix string length, excluding the null byte. Only makes sense if null_terminated
-            # Only makes sense if null_terminated
             suffix_strlen = match.end(0) - match.end(1) - 1
 
             # How many bytes are we shrinking our string?
@@ -229,9 +224,9 @@ def _apply_to_file(self, f):
             if bytes_shorter < 0:
                 raise CannotGrowString(old, new)
 
-            # If we don't know whether this is a null terminated C-string (we're looking
+            # If we don't know whether this is a null terminated C-string (we're looking only N + 1
-            # only N + 1 bytes ahead), or if it is and we have a common suffix, we can
+            # bytes ahead), or if it is and we have a common suffix, we can simply pad with leading
-            # simply pad with leading dir separators.
+            # dir separators.
             elif (
                 not null_terminated
                 or suffix_strlen >= self.suffix_safety_size  # == is enough, but let's be defensive
@@ -240,9 +235,9 @@ def _apply_to_file(self, f):
             ):
                 replacement = b"/" * bytes_shorter + new
 
-            # If it *was* null terminated, all that matters is that we can leave N bytes
+            # If it *was* null terminated, all that matters is that we can leave N bytes of old
-            # of old suffix in place. Note that > is required since we also insert an
+            # suffix in place. Note that > is required since we also insert an additional null
-            # additional null terminator.
+            # terminator.
             elif bytes_shorter > self.suffix_safety_size:
                 replacement = new + match.group(2)  # includes the trailing null
 
@@ -257,22 +252,6 @@ def _apply_to_file(self, f):
         return modified
 
 
-class BinaryStringReplacementError(spack.error.SpackError):
-    def __init__(self, file_path, old_len, new_len):
-        """The size of the file changed after binary path substitution
-
-        Args:
-            file_path (str): file with changing size
-            old_len (str): original length of the file
-            new_len (str): length of the file after substitution
-        """
-        super().__init__(
-            "Doing a binary string replacement in %s failed.\n"
-            "The size of the file changed from %s to %s\n"
-            "when it should have remanined the same." % (file_path, old_len, new_len)
-        )
-
-
 class BinaryTextReplaceError(spack.error.SpackError):
     def __init__(self, msg):
         msg += (
@@ -284,17 +263,16 @@ def __init__(self, msg):
 
 class CannotGrowString(BinaryTextReplaceError):
     def __init__(self, old, new):
-        msg = "Cannot replace {!r} with {!r} because the new prefix is longer.".format(old, new)
+        return super().__init__(
-        super().__init__(msg)
+            f"Cannot replace {old!r} with {new!r} because the new prefix is longer."
+        )
 
 
 class CannotShrinkCString(BinaryTextReplaceError):
     def __init__(self, old, new, full_old_string):
-        # Just interpolate binary string to not risk issues with invalid
+        # Just interpolate binary string to not risk issues with invalid unicode, which would be
-        # unicode, which would be really bad user experience: error in error.
+        # really bad user experience: error in error. We have no clue if we actually deal with a
-        # We have no clue if we actually deal with a real C-string nor what
+        # real C-string nor what encoding it has.
-        # encoding it has.
+        super().__init__(
-        msg = "Cannot replace {!r} with {!r} in the C-string {!r}.".format(
+            f"Cannot replace {old!r} with {new!r} in the C-string {full_old_string!r}."
-            old, new, full_old_string
         )
-        super().__init__(msg)

lib/spack/spack/rewiring.py

@@ -69,7 +69,7 @@ def rewire_node(spec, explicit):
         os.path.join(spec.prefix, rel_path) for rel_path in buildinfo["relocate_textfiles"]
     ]
     if text_to_relocate:
-        relocate.relocate_text(files=text_to_relocate, prefixes=prefix_to_prefix)
+        relocate.relocate_text(files=text_to_relocate, prefix_to_prefix=prefix_to_prefix)
     links = [os.path.join(spec.prefix, f) for f in buildinfo["relocate_links"]]
     relocate.relocate_links(links, prefix_to_prefix)
     bins_to_relocate = [
@@ -80,7 +80,7 @@ def rewire_node(spec, explicit):
             relocate.relocate_macho_binaries(bins_to_relocate, prefix_to_prefix)
         if "elf" in platform.binary_formats:
             relocate.relocate_elf_binaries(bins_to_relocate, prefix_to_prefix)
-        relocate.relocate_text_bin(binaries=bins_to_relocate, prefixes=prefix_to_prefix)
+        relocate.relocate_text_bin(binaries=bins_to_relocate, prefix_to_prefix=prefix_to_prefix)
     shutil.rmtree(tempdir)
     install_manifest = os.path.join(
         spec.prefix,

lib/spack/spack/test/packaging.py

@@ -32,7 +32,7 @@
 from spack.fetch_strategy import URLFetchStrategy
 from spack.installer import PackageInstaller
 from spack.paths import mock_gpg_keys_path
-from spack.relocate import macho_find_paths, relocate_links, relocate_text
+from spack.relocate import _macho_find_paths, relocate_links, relocate_text
 
 pytestmark = pytest.mark.not_on_windows("does not run on windows")
 
@@ -287,7 +287,7 @@ def test_replace_paths(tmpdir):
         for prefix, hash in prefix2hash.items():
             prefix2prefix[prefix] = hash2prefix[hash]
 
-        out_dict = macho_find_paths(
+        out_dict = _macho_find_paths(
             [oldlibdir_a, oldlibdir_b, oldlibdir_c, oldlibdir_cc, oldlibdir_local],
             [
                 os.path.join(oldlibdir_a, libfile_a),
@@ -309,7 +309,7 @@ def test_replace_paths(tmpdir):
             os.path.join(oldlibdir_cc, libfile_c): os.path.join(libdir_cc, libfile_c),
         }
 
-        out_dict = macho_find_paths(
+        out_dict = _macho_find_paths(
             [oldlibdir_a, oldlibdir_b, oldlibdir_c, oldlibdir_cc, oldlibdir_local],
             [
                 os.path.join(oldlibdir_a, libfile_a),
@@ -332,7 +332,7 @@ def test_replace_paths(tmpdir):
             os.path.join(oldlibdir_cc, libfile_c): os.path.join(libdir_cc, libfile_c),
         }
 
-        out_dict = macho_find_paths(
+        out_dict = _macho_find_paths(
             [oldlibdir_a, oldlibdir_b, oldlibdir_c, oldlibdir_cc, oldlibdir_local],
             [
                 f"@rpath/{libfile_a}",
@@ -356,7 +356,7 @@ def test_replace_paths(tmpdir):
             libdir_local: libdir_local,
         }
 
-        out_dict = macho_find_paths(
+        out_dict = _macho_find_paths(
             [oldlibdir_a, oldlibdir_b, oldlibdir_d, oldlibdir_local],
             [f"@rpath/{libfile_a}", f"@rpath/{libfile_b}", f"@rpath/{libfile_loco}"],
             None,
@@ -465,7 +465,7 @@ def test_macho_relocation_with_changing_projection(relocation_dict):
     the two schemes, like /a/b/baz.
     """
     original_rpath = "/foo/bar/baz/abcdef"
-    result = macho_find_paths(
+    result = _macho_find_paths(
         [original_rpath], deps=[], idpath=None, prefix_to_prefix=relocation_dict
     )
     assert result[original_rpath] == "/a/b/c/abcdef"