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>
This commit is contained in:
		@@ -42,6 +42,7 @@
 | 
			
		||||
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 @@ def before_visit_symlinked_dir(self, root, rel_path, depth):
 | 
			
		||||
        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,6 +762,7 @@ def get_buildfile_manifest(spec):
 | 
			
		||||
                continue
 | 
			
		||||
 | 
			
		||||
        elif relocate.needs_text_relocation(m_type, m_subtype):
 | 
			
		||||
            if file_matches_any_binary_regex(abs_path, compiled_prefixes):
 | 
			
		||||
                data["text_to_relocate"].append(rel_path)
 | 
			
		||||
                continue
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -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
 | 
			
		||||
 
 | 
			
		||||
@@ -11,7 +11,7 @@
 | 
			
		||||
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.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"]
 | 
			
		||||
 
 | 
			
		||||
@@ -20,6 +20,7 @@
 | 
			
		||||
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]/"
 | 
			
		||||
    )
 | 
			
		||||
 
 | 
			
		||||
@@ -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)
 | 
			
		||||
		Reference in New Issue
	
	Block a user