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>

5 files changed, 81 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]/"
+    )
diff --git a/var/spack/repos/builtin.mock/packages/needs-text-relocation/package.py b/var/spack/repos/builtin.mock/packages/needs-text-relocation/package.py
new file mode 100644
index 0000000000..9b2654dd83
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/needs-text-relocation/package.py
@@ -0,0 +1,27 @@
+# Copyright 2013-2022 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+from spack.package import *
+
+
+class NeedsTextRelocation(Package):
+    """A dumy package that encodes its prefix."""
+
+    homepage = "https://www.cmake.org"
+    url = "https://cmake.org/files/v3.4/cmake-3.4.3.tar.gz"
+
+    version("0.0.0", "12345678qwertyuiasdfghjkzxcvbnm0")
+
+    def install(self, spec, prefix):
+        mkdirp(prefix.bin)
+
+        exe = join_path(prefix.bin, "exe")
+        with open(exe, "w") as f:
+            f.write(prefix)
+        set_executable(exe)
+
+        otherexe = join_path(prefix.bin, "otherexe")
+        with open(otherexe, "w") as f:
+            f.write("Lorem Ipsum")
+        set_executable(otherexe)