Add filename to text_to_relocate only if it needs to be relocated (#31074)

Scan the text files for relocatable prefixes *before* creating a tarball,
to reduce the amount of work to be done during install from binary
cache. 

Co-authored-by: Harmen Stoppels <harmenstoppels@gmail.com>

4 files changed, 54 insertions, 8 deletions

diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py
index 7ae91dd6bf..3f74d91a9e 100644
--- a/lib/spack/spack/binary_distribution.py
+++ b/lib/spack/spack/binary_distribution.py
@@ -42,6 +42,7 @@ import spack.util.spack_yaml as syaml
 import spack.util.url as url_util
 import spack.util.web as web_util
 from spack.caches import misc_cache_location
+from spack.relocate import utf8_path_to_binary_regex
 from spack.spec import Spec
 from spack.stage import Stage
 
@@ -687,6 +688,15 @@ class BuildManifestVisitor(BaseDirectoryVisitor):
         return False
 
 
+def file_matches_any_binary_regex(path, regexes):
+    with open(path, "rb") as f:
+        contents = f.read()
+    for regex in regexes:
+        if regex.search(contents):
+            return True
+    return False
+
+
 def get_buildfile_manifest(spec):
     """
     Return a data structure with information about a build, including
@@ -712,6 +722,15 @@ def get_buildfile_manifest(spec):
     root = spec.prefix
     visit_directory_tree(root, visitor)
 
+    # Collect a list of prefixes for this package and it's dependencies, Spack will
+    # look for them to decide if text file needs to be relocated or not
+    prefixes = [d.prefix for d in spec.traverse(root=True, deptype="all") if not d.external]
+    prefixes.append(spack.hooks.sbang.sbang_install_path())
+    prefixes.append(str(spack.store.layout.root))
+
+    # Create a list regexes matching collected prefixes
+    compiled_prefixes = [utf8_path_to_binary_regex(prefix) for prefix in prefixes]
+
     # Symlinks.
 
     # Obvious bugs:
@@ -743,8 +762,9 @@ def get_buildfile_manifest(spec):
                 continue
 
         elif relocate.needs_text_relocation(m_type, m_subtype):
-            data["text_to_relocate"].append(rel_path)
-            continue
+            if file_matches_any_binary_regex(abs_path, compiled_prefixes):
+                data["text_to_relocate"].append(rel_path)
+                continue
 
         data["other"].append(abs_path)
 
diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py
index 3297965b6e..667e8de4e0 100644
--- a/lib/spack/spack/relocate.py
+++ b/lib/spack/spack/relocate.py
@@ -744,6 +744,14 @@ def relocate_links(links, orig_layout_root, orig_install_prefix, new_install_pre
             tty.warn(msg.format(link_target, abs_link, new_install_prefix))
 
 
+def utf8_path_to_binary_regex(prefix):
+    """Create a (binary) regex that matches the input path in utf8"""
+    prefix_bytes = re.escape(prefix).encode("utf-8")
+    prefix_rexp = re.compile(b"(?<![\\w\\-_/])([\\w\\-_]*?)%s([\\w\\-_/]*)" % prefix_bytes)
+
+    return prefix_rexp
+
+
 def unsafe_relocate_text(files, prefixes, concurrency=32):
     """Relocate text file from the original installation prefix to the
     new prefix.
@@ -767,11 +775,8 @@ def unsafe_relocate_text(files, prefixes, concurrency=32):
 
     for orig_prefix, new_prefix in prefixes.items():
         if orig_prefix != new_prefix:
-            orig_bytes = orig_prefix.encode("utf-8")
-            orig_prefix_rexp = re.compile(
-                b"(?<![\\w\\-_/])([\\w\\-_]*?)%s([\\w\\-_/]*)" % orig_bytes
-            )
-            new_bytes = b"\\1%s\\2" % new_prefix.encode("utf-8")
+            orig_prefix_rexp = utf8_path_to_binary_regex(orig_prefix)
+            new_bytes = b"\\1%s\\2" % new_prefix.replace("\\", r"\\").encode("utf-8")
             compiled_prefixes[orig_prefix_rexp] = new_bytes
 
     # Do relocations on text that refers to the install tree
diff --git a/lib/spack/spack/test/bindist.py b/lib/spack/spack/test/bindist.py
index d0a8a0a4f0..c73612f101 100644
--- a/lib/spack/spack/test/bindist.py
+++ b/lib/spack/spack/test/bindist.py
@@ -11,7 +11,7 @@ import sys
 import py
 import pytest
 
-from llnl.util.filesystem import visit_directory_tree
+from llnl.util.filesystem import join_path, visit_directory_tree
 
 import spack.binary_distribution as bindist
 import spack.config
@@ -22,6 +22,7 @@ import spack.repo
 import spack.store
 import spack.util.gpg
 import spack.util.web as web_util
+from spack.binary_distribution import get_buildfile_manifest
 from spack.directory_layout import DirectoryLayout
 from spack.paths import test_path
 from spack.spec import Spec
@@ -679,3 +680,13 @@ def test_build_manifest_visitor(tmpdir):
     with tmpdir.as_cwd():
         assert not any(os.path.islink(f) or os.path.isdir(f) for f in visitor.files)
         assert all(os.path.islink(f) for f in visitor.symlinks)
+
+
+def test_text_relocate_if_needed(install_mockery, mock_fetch, monkeypatch, capfd):
+    spec = Spec("needs-text-relocation").concretized()
+    install_cmd(str(spec))
+
+    manifest = get_buildfile_manifest(spec)
+    assert join_path("bin", "exe") in manifest["text_to_relocate"]
+    assert join_path("bin", "otherexe") not in manifest["text_to_relocate"]
+    assert join_path("bin", "secretexe") not in manifest["text_to_relocate"]
diff --git a/lib/spack/spack/test/relocate.py b/lib/spack/spack/test/relocate.py
index ab306759cd..42e392c1ba 100644
--- a/lib/spack/spack/test/relocate.py
+++ b/lib/spack/spack/test/relocate.py
@@ -20,6 +20,7 @@ import spack.spec
 import spack.store
 import spack.tengine
 import spack.util.executable
+from spack.relocate import utf8_path_to_binary_regex
 
 pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="Tests fail on Windows")
 
@@ -476,3 +477,12 @@ def test_fixup_macos_rpaths(make_dylib, make_object_file):
     # (this is a corner case for GCC installation)
     (root, filename) = make_object_file()
     assert not fixup_rpath(root, filename)
+
+
+def test_text_relocation_regex_is_safe():
+    assert (
+        utf8_path_to_binary_regex("/[a-z]/")
+        .search(b"This does not match /a/, but this does: /[a-z]/.")
+        .group(0)
+        == b"/[a-z]/"
+    )