From e5d6f28b4e6763bf5e3f899be2f584eeff7f0d9e Mon Sep 17 00:00:00 2001 From: Patrick Gartung Date: Wed, 20 Dec 2017 20:21:41 -0600 Subject: binary caching: handle files misidentified as needing relocation (#6679) * Only specify a file as needing relocation if it contains the spack root as a text string (this constraint also applies to binaries) * Don't fail if there is an error retrieving RPATH information from a binary (even if it is specified as requiring relocation) --- lib/spack/spack/binary_distribution.py | 18 +++++++----- lib/spack/spack/relocate.py | 52 ++++++++++++++++++++++++---------- lib/spack/spack/test/packaging.py | 9 ++++-- 3 files changed, 54 insertions(+), 25 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 5a2db06424..e62e9aaf14 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -111,13 +111,17 @@ def write_buildinfo_file(prefix, workdir, rel=False): dirs[:] = [d for d in dirs if d not in blacklist] for filename in files: path_name = os.path.join(root, filename) - filetype = relocate.get_filetype(path_name) - if relocate.needs_binary_relocation(filetype, os_id): - rel_path_name = os.path.relpath(path_name, prefix) - binary_to_relocate.append(rel_path_name) - elif relocate.needs_text_relocation(filetype): - rel_path_name = os.path.relpath(path_name, prefix) - text_to_relocate.append(rel_path_name) + # Check if the file contains a string with the installroot. + # This cuts down on the number of files added to the list + # of files potentially needing relocation + if relocate.strings_contains_installroot(path_name): + filetype = relocate.get_filetype(path_name) + if relocate.needs_binary_relocation(filetype, os_id): + rel_path_name = os.path.relpath(path_name, prefix) + binary_to_relocate.append(rel_path_name) + elif relocate.needs_text_relocation(filetype): + rel_path_name = os.path.relpath(path_name, prefix) + text_to_relocate.append(rel_path_name) # Create buildinfo data and write it to disk buildinfo = {} diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 1f62c70231..325ec22509 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -28,7 +28,7 @@ import platform import re import spack import spack.cmd -from spack.util.executable import Executable +from spack.util.executable import Executable, ProcessError from llnl.util.filesystem import filter_file import llnl.util.tty as tty @@ -56,10 +56,15 @@ def get_existing_elf_rpaths(path_name): as a list of strings. """ if platform.system() == 'Linux': - command = Executable(get_patchelf()) - output = command('--print-rpath', '%s' % - path_name, output=str, err=str) - return output.rstrip('\n').split(':') + patchelf = Executable(get_patchelf()) + try: + output = patchelf('--print-rpath', '%s' % + path_name, output=str, error=str) + return output.rstrip('\n').split(':') + except ProcessError as e: + tty.debug('patchelf --print-rpath produced an error on %s' % + path_name, e) + return [] else: tty.die('relocation not supported for this platform') return @@ -193,19 +198,34 @@ def get_filetype(path_name): file = Executable('file') file.add_default_env('LC_ALL', 'C') output = file('-b', '-h', '%s' % path_name, - output=str, err=str) + output=str, error=str) return output.strip() -def modify_elf_object(path_name, orig_rpath, new_rpath): +def strings_contains_installroot(path_name): + """ + Check if the file contain the install root string. + """ + strings = Executable('strings') + output = strings('%s' % path_name, + output=str, err=str) + return (spack.store.layout.root in output) + + +def modify_elf_object(path_name, new_rpaths): """ Replace orig_rpath with new_rpath in RPATH of elf object path_name """ if platform.system() == 'Linux': - new_joined = ':'.join(new_rpath) + new_joined = ':'.join(new_rpaths) patchelf = Executable(get_patchelf()) - patchelf('--force-rpath', '--set-rpath', '%s' % new_joined, - '%s' % path_name, output=str, cmd=str) + try: + patchelf('--force-rpath', '--set-rpath', '%s' % new_joined, + '%s' % path_name, output=str, error=str) + except ProcessError as e: + tty.die('patchelf --set-rpath %s failed' % + path_name, e) + pass else: tty.die('relocation not supported for this platform') @@ -255,8 +275,9 @@ def relocate_binary(path_names, old_dir, new_dir): elif platform.system() == 'Linux': for path_name in path_names: orig_rpaths = get_existing_elf_rpaths(path_name) - new_rpaths = substitute_rpath(orig_rpaths, old_dir, new_dir) - modify_elf_object(path_name, orig_rpaths, new_rpaths) + if orig_rpaths: + new_rpaths = substitute_rpath(orig_rpaths, old_dir, new_dir) + modify_elf_object(path_name, new_rpaths) else: tty.die("Relocation not implemented for %s" % platform.system()) @@ -278,9 +299,10 @@ def make_binary_relative(cur_path_names, orig_path_names, old_dir): elif platform.system() == 'Linux': for cur_path, orig_path in zip(cur_path_names, orig_path_names): orig_rpaths = get_existing_elf_rpaths(cur_path) - new_rpaths = get_relative_rpaths(orig_path, old_dir, - orig_rpaths) - modify_elf_object(cur_path, orig_rpaths, new_rpaths) + if orig_rpaths: + new_rpaths = get_relative_rpaths(orig_path, old_dir, + orig_rpaths) + modify_elf_object(cur_path, new_rpaths) else: tty.die("Prelocation not implemented for %s" % platform.system()) diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index 49e5138c89..7cb8dfc1fe 100644 --- a/lib/spack/spack/test/packaging.py +++ b/lib/spack/spack/test/packaging.py @@ -42,6 +42,7 @@ from spack.spec import Spec from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite from spack.util.executable import ProcessError from spack.relocate import needs_binary_relocation, needs_text_relocation +from spack.relocate import strings_contains_installroot from spack.relocate import get_patchelf, relocate_text from spack.relocate import substitute_rpath, get_relative_rpaths from spack.relocate import macho_replace_paths, macho_make_paths_relative @@ -217,10 +218,10 @@ echo $PATH""" stage.destroy() -def test_relocate_text(): +def test_relocate_text(tmpdir): # Validate the text path replacement old_dir = '/home/spack/opt/spack' - filename = 'dummy.txt' + filename = str(tmpdir) + '/dummy.txt' with open(filename, "w") as script: script.write(old_dir) script.close() @@ -229,7 +230,9 @@ def test_relocate_text(): new_dir = '/opt/rh/devtoolset/' relocate_text(filenames, old_dir, new_dir) - with open(filename, "r")as script: + assert(strings_contains_installroot(filename) is False) + + with open(filename, "r") as script: for line in script: assert(new_dir in line) -- cgit v1.2.3-70-g09d2