From 7a0a907b5c24cb7b52d749206a444130a03514b3 Mon Sep 17 00:00:00 2001 From: scheibelp Date: Tue, 16 Jan 2018 21:33:28 -0800 Subject: elf relocation fix: cherry-picked from develop branch (#6889) * Revert "Quick fix for relocation issues." This reverts commit 57608a6dc495af6c042945767ac3bd0c009fadd5. * Buildcache: relocate fixes (#6512) * Updated function which checks if a binary file needs relocation. Previously this was incorrectly identifying ELF binaries as symbolic links (so they were being excluded from relocation). Added test to check that ELF binaries are not considered symlinks. * relocate_text was not replacing paths in text files. Added test to check that text files are relocated properly (i.e. paths in the file are converted to the new prefix). * Exclude backup files created by filter_file when installing from binary cache. * Update write_buildinfo_file method signature to distinguish between the spec prefix and the working directory for the binary cache package. --- lib/spack/spack/binary_distribution.py | 49 +++++++++++------------------- lib/spack/spack/relocate.py | 18 +++++------ lib/spack/spack/test/packaging.py | 55 ++++++++++++++++++++++++++-------- 3 files changed, 68 insertions(+), 54 deletions(-) diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index fb43f252cf..5a2db06424 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -28,6 +28,7 @@ import re import tarfile import yaml import shutil +import platform import llnl.util.tty as tty from spack.util.gpg import Gpg @@ -44,9 +45,6 @@ from spack.util.executable import ProcessError import spack.relocate as relocate -_relocation_blacklist = (".spack", "man") - - class NoOverwriteException(Exception): pass @@ -98,41 +96,36 @@ def read_buildinfo_file(prefix): return buildinfo -def _find_relocations(prefix): +def write_buildinfo_file(prefix, workdir, rel=False): + """ + Create a cache file containing information + required for the relocation + """ text_to_relocate = [] binary_to_relocate = [] + blacklist = (".spack", "man") + os_id = platform.system() # Do this at during tarball creation to save time when tarball unpacked. # Used by make_package_relative to determine binaries to change. for root, dirs, files in os.walk(prefix, topdown=True): - dirs[:] = [d for d in dirs if d not in _relocation_blacklist] - + 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): + 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) - return text_to_relocate, binary_to_relocate - - -def write_buildinfo_file(prefix, rel=False): - """ - Create a cache file containing information - required for the relocation - """ - text_to_relocate, binary_to_relocate = _find_relocations(prefix) - # Create buildinfo data and write it to disk buildinfo = {} buildinfo['relative_rpaths'] = rel buildinfo['buildpath'] = spack.store.layout.root buildinfo['relocate_textfiles'] = text_to_relocate buildinfo['relocate_binaries'] = binary_to_relocate - filename = buildinfo_file_name(prefix) + filename = buildinfo_file_name(workdir) with open(filename, 'w') as outfile: outfile.write(yaml.dump(buildinfo, default_flow_style=True)) @@ -256,7 +249,7 @@ def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False, install_tree(spec.prefix, workdir, symlinks=True) # create info for later relocation and create tar - write_buildinfo_file(workdir, rel=rel) + write_buildinfo_file(spec.prefix, workdir, rel=rel) # optinally make the paths in the binaries relative to each other # in the spack install tree before creating tarball @@ -363,28 +356,20 @@ def relocate_package(prefix): if new_path == old_path and not rel: return - text_relocs = buildinfo['relocate_textfiles'] - binary_relocs = buildinfo['relocate_binaries'] - - # if there are no relocations, search for them instead - # TODO: revisit this in a 0.11 point release - if not text_relocs or not binary_relocs: - text_relocs, binary_relocs = _find_relocations(prefix) - rel = False - tty.msg("Relocating package from", "%s to %s." % (old_path, new_path)) path_names = set() - for filename in text_relocs: + for filename in buildinfo['relocate_textfiles']: path_name = os.path.join(prefix, filename) - path_names.add(path_name) + # Don't add backup files generated by filter_file during install step. + if not path_name.endswith('~'): + path_names.add(path_name) relocate.relocate_text(path_names, old_path, new_path) - # If the binary files in the package were not edited to use # relative RPATHs, then the RPATHs need to be relocated if not rel: path_names = set() - for filename in binary_relocs: + for filename in buildinfo['relocate_binaries']: path_name = os.path.join(prefix, filename) path_names.add(path_name) relocate.relocate_binary(path_names, old_path, new_path) diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 246ed7510c..1f62c70231 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -210,21 +210,21 @@ def modify_elf_object(path_name, orig_rpath, new_rpath): tty.die('relocation not supported for this platform') -def needs_binary_relocation(filetype): +def needs_binary_relocation(filetype, os_id=None): """ Check whether the given filetype is a binary that may need relocation. """ retval = False if "relocatable" in filetype: return False - if "symbolic link" in filetype: + if "link to" in filetype: return False - if platform.system() == 'Darwin': - return ('Mach-O' in filetype) - elif platform.system() == 'Linux': - return ('ELF' in filetype) + if os_id == 'Darwin': + return ("Mach-O" in filetype) + elif os_id == 'Linux': + return ("ELF" in filetype) else: - tty.die("Relocation not implemented for %s" % platform.system()) + tty.die("Relocation not implemented for %s" % os_id) return retval @@ -232,7 +232,7 @@ def needs_text_relocation(filetype): """ Check whether the given filetype is text that may need relocation. """ - if "link" in filetype: + if "link to" in filetype: return False return ("text" in filetype) @@ -289,7 +289,7 @@ def relocate_text(path_names, old_dir, new_dir): """ Replace old path with new path in text file path_name """ - filter_file("r'%s'" % old_dir, "r'%s'" % new_dir, + filter_file('%s' % old_dir, '%s' % new_dir, *path_names, backup=False) diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index f9c77c530f..49e5138c89 100644 --- a/lib/spack/spack/test/packaging.py +++ b/lib/spack/spack/test/packaging.py @@ -42,7 +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 get_patchelf +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 from spack.relocate import modify_macho_object, macho_get_paths @@ -217,10 +217,43 @@ echo $PATH""" stage.destroy() -def test_relocate(): - assert (needs_binary_relocation('relocatable') is False) - assert (needs_binary_relocation('link') is False) - assert (needs_text_relocation('link') is False) +def test_relocate_text(): + # Validate the text path replacement + old_dir = '/home/spack/opt/spack' + filename = 'dummy.txt' + with open(filename, "w") as script: + script.write(old_dir) + script.close() + + filenames = [filename] + new_dir = '/opt/rh/devtoolset/' + relocate_text(filenames, old_dir, new_dir) + + with open(filename, "r")as script: + for line in script: + assert(new_dir in line) + + +def test_needs_relocation(): + binary_type = ( + 'ELF 64-bit LSB executable, x86-64, version 1 (SYSV),' + ' dynamically linked (uses shared libs),' + ' for GNU/Linux x.y.z, stripped') + + assert needs_binary_relocation(binary_type, os_id='Linux') + assert not needs_binary_relocation('relocatable', + os_id='Linux') + assert not needs_binary_relocation('symbolic link to `foo\'', + os_id='Linux') + + assert needs_text_relocation('ASCII text') + assert not needs_text_relocation('symbolic link to `foo.text\'') + + macho_type = 'Mach-O 64-bit executable x86_64' + assert needs_binary_relocation(macho_type, os_id='Darwin') + + +def test_macho_paths(): out = macho_make_paths_relative('/Users/Shares/spack/pkgC/lib/libC.dylib', '/Users/Shared/spack', @@ -289,6 +322,8 @@ def test_relocate(): '/usr/local/lib/libloco.dylib'], None) + +def test_elf_paths(): out = get_relative_rpaths( '/usr/bin/test', '/usr', ('/usr/lib', '/usr/lib64', '/opt/local/lib')) @@ -303,8 +338,8 @@ def test_relocate(): reason="only works with Mach-o objects") def test_relocate_macho(tmpdir): with tmpdir.as_cwd(): - get_patchelf() - assert (needs_binary_relocation('Mach-O')) + + get_patchelf() # this does nothing on Darwin rpaths, deps, idpath = macho_get_paths('/bin/bash') nrpaths, ndeps, nid = macho_make_paths_relative('/bin/bash', '/usr', @@ -341,9 +376,3 @@ def test_relocate_macho(tmpdir): 'libncurses.5.4.dylib', rpaths, deps, idpath, nrpaths, ndeps, nid) - - -@pytest.mark.skipif(sys.platform != 'linux2', - reason="only works with Elf objects") -def test_relocate_elf(): - assert (needs_binary_relocation('ELF')) -- cgit v1.2.3-70-g09d2