Skip to content

Commit

Permalink
elf relocation fix: cherry-picked from develop branch (spack#6889)
Browse files Browse the repository at this point in the history 
* Revert "Quick fix for relocation issues."

This reverts commit 57608a6.

* Buildcache: relocate fixes (spack#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.
  • Loading branch information
@scheibelp @JBlaschke
scheibelp authored and JBlaschke committed Jun 1, 2020
1 parent 1eabd32 commit 369e4bf
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 188 deletions.
80 changes: 19 additions & 61 deletions lib/spack/spack/binary_distribution.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,8 @@
import re
import tarfile
import shutil
import tempfile
import hashlib
import glob
import platform

from contextlib import closing
import ruamel.yaml as yaml

import json

from six.moves.urllib.error import URLError

import llnl.util.tty as tty
from llnl.util.filesystem import mkdirp

Expand All @@ -38,9 +28,6 @@
from spack.util.gpg import Gpg
import spack.architecture as architecture

_relocation_blacklist = (".spack", "man")


class NoOverwriteException(Exception):
pass

Expand Down Expand Up @@ -150,48 +137,29 @@ 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)
m_type, m_subtype = relocate.mime_type(path_name)
if os.path.islink(path_name):
link = os.readlink(path_name)
if os.path.isabs(link):
# Relocate absolute links into the spack tree
if link.startswith(spack.store.layout.root):
rel_path_name = os.path.relpath(path_name, prefix)
link_to_relocate.append(rel_path_name)
else:
msg = 'Absolute link %s to %s ' % (path_name, link)
msg += 'outside of prefix %s ' % prefix
msg += 'should not be relocated.'
tty.warn(msg)

if relocate.needs_binary_relocation(m_type, m_subtype):
if not filename.endswith('.o'):
rel_path_name = os.path.relpath(path_name, prefix)
binary_to_relocate.append(rel_path_name)
if relocate.needs_text_relocation(m_type, m_subtype):
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)

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
Expand All @@ -201,8 +169,6 @@ def write_buildinfo_file(prefix, rel=False):
prefix, spack.store.layout.root)
buildinfo['relocate_textfiles'] = text_to_relocate
buildinfo['relocate_binaries'] = binary_to_relocate
buildinfo['relocate_links'] = link_to_relocate
buildinfo['prefix_to_hash'] = prefix_to_hash
filename = buildinfo_file_name(workdir)
with open(filename, 'w') as outfile:
outfile.write(syaml.dump(buildinfo, default_flow_style=True))
Expand Down Expand Up @@ -375,7 +341,7 @@ def build_tarball(spec, outdir, force=False, rel=False, unsigned=False,
os.remove(temp_tarfile_path)

# create info for later relocation and create tar
write_buildinfo_file(spec, workdir, rel)
write_buildinfo_file(spec.prefix, workdir, rel=rel)

# optionally make the paths in the binaries relative to each other
# in the spack install tree before creating tarball
Expand Down Expand Up @@ -538,28 +504,20 @@ def check_package_relocatable(workdir, spec, allow_root):
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)
Expand Down
103 changes: 12 additions & 91 deletions lib/spack/spack/relocate.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,76 +449,7 @@ def needs_text_relocation(m_type, m_subtype):
return m_type == 'text'


def _replace_prefix_text(filename, old_dir, new_dir):
"""Replace all the occurrences of the old install prefix with a
new install prefix in text files that are utf-8 encoded.
Args:
filename (str): target text file (utf-8 encoded)
old_dir (str): directory to be searched in the file
new_dir (str): substitute for the old directory
"""
# TODO: cache regexes globally to speedup computation
with open(filename, 'rb+') as f:
data = f.read()
f.seek(0)
# Replace old_dir with new_dir if it appears at the beginning of a path
# Negative lookbehind for a character legal in a path
# Then a match group for any characters legal in a compiler flag
# Then old_dir
# Then characters legal in a path
# Ensures we only match the old_dir if it's precedeed by a flag or by
# characters not legal in a path, but not if it's preceeded by other
# components of a path.
old_bytes = old_dir.encode('utf-8')
pat = b'(?<![\\w\\-_/])([\\w\\-_]*?)%s([\\w\\-_/]*)' % old_bytes
repl = b'\\1%s\\2' % new_dir.encode('utf-8')
ndata = re.sub(pat, repl, data)
f.write(ndata)
f.truncate()


def _replace_prefix_bin(filename, old_dir, new_dir):
"""Replace all the occurrences of the old install prefix with a
new install prefix in binary files.
The new install prefix is prefixed with ``os.sep`` until the
lengths of the prefixes are the same.
Args:
filename (str): target binary file
old_dir (str): directory to be searched in the file
new_dir (str): substitute for the old directory
"""
def replace(match):
occurances = match.group().count(old_dir.encode('utf-8'))
olen = len(old_dir.encode('utf-8'))
nlen = len(new_dir.encode('utf-8'))
padding = (olen - nlen) * occurances
if padding < 0:
return data
return match.group().replace(
old_dir.encode('utf-8'),
os.sep.encode('utf-8') * padding + new_dir.encode('utf-8')
)

with open(filename, 'rb+') as f:
data = f.read()
f.seek(0)
original_data_len = len(data)
pat = re.compile(old_dir.encode('utf-8'))
if not pat.search(data):
return
ndata = pat.sub(replace, data)
if not len(ndata) == original_data_len:
raise BinaryStringReplacementError(
filename, original_data_len, len(ndata))
f.write(ndata)
f.truncate()


def relocate_macho_binaries(path_names, old_layout_root, new_layout_root,
prefix_to_prefix, rel, old_prefix, new_prefix):
def needs_binary_relocation(filetype, os_id=None):
"""
Use macholib python package to get the rpaths, depedent libraries
and library identity for libraries from the MachO object. Modify them
Expand All @@ -529,14 +460,14 @@ def relocate_macho_binaries(path_names, old_layout_root, new_layout_root,
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

for path_name in path_names:
Expand Down Expand Up @@ -696,11 +627,9 @@ def make_link_relative(new_links, orig_links):
new_links (list): new links to be made relative
orig_links (list): original links
"""
for new_link, orig_link in zip(new_links, orig_links):
target = os.readlink(orig_link)
relative_target = os.path.relpath(target, os.path.dirname(orig_link))
os.unlink(new_link)
os.symlink(relative_target, new_link)
if "link to" in filetype:
return False
return ("text" in filetype)


def make_macho_binaries_relative(cur_path_names, orig_path_names,
Expand Down Expand Up @@ -907,16 +836,8 @@ def file_is_relocatable(file, paths_to_relocate=None):
ValueError: if the file does not exist or the path is not absolute
"""
default_paths_to_relocate = [spack.store.layout.root, spack.paths.prefix]
paths_to_relocate = paths_to_relocate or default_paths_to_relocate

if not (platform.system().lower() == 'darwin'
or platform.system().lower() == 'linux'):
msg = 'function currently implemented only for linux and macOS'
raise NotImplementedError(msg)

if not os.path.exists(file):
raise ValueError('{0} does not exist'.format(file))
filter_file('%s' % old_dir, '%s' % new_dir,
*path_names, backup=False)

if not os.path.isabs(file):
raise ValueError('{0} is not an absolute path'.format(file))
Expand Down

0 comments on commit 369e4bf

Please sign in to comment.