zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Binary caching bugfix: symlink relocation (#10073)

Browse Source 
Binary caches of packages with absolute symlinks had broken symlinks.
As a stopgap measure, #9747 addressed this by replacing symlinks with
copies of files when creating binary cached packages.

This reverts #9747 and instead, either relative-izes the symlink or
rewrites the target. If the binary cache is created using '--rel' (as
in "spack buildcache create --rel...") then absolute symlinks will be
replaced with relative symlinks (in addition to making RPATHs relative
as before); otherwise they are rewritten (when the binary cache is
unpacked and installed).
This commit is contained in:
Greg Becker
2019-01-11 14:52:01 -08:00
committed by Peter Scheibel
parent 3c6d484150
commit c63c4a048c
3 changed files with 131 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
lib/spack/spack/binary_distribution.py
View File

@@ -116,6 +116,7 @@ def write_buildinfo_file(prefix, workdir, rel=False):
"""
text_to_relocate = []
binary_to_relocate = []
link_to_relocate = []
blacklist = (".spack", "man")
os_id = platform.system()
# Do this at during tarball creation to save time when tarball unpacked.
@@ -124,10 +125,23 @@ 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)
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 stage %s ' % prefix
msg += 'cannot be relocated.'
tty.warn(msg)
# 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(
elif relocate.strings_contains_installroot(
path_name, spack.store.layout.root):
filetype = get_filetype(path_name)
if relocate.needs_binary_relocation(filetype, os_id):
@@ -145,6 +159,7 @@ def write_buildinfo_file(prefix, workdir, 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
filename = buildinfo_file_name(workdir)
with open(filename, 'w') as outfile:
outfile.write(yaml.dump(buildinfo, default_flow_style=True))
@@ -269,9 +284,7 @@ def build_tarball(spec, outdir, force=False, rel=False, unsigned=False,
raise NoOverwriteException(str(specfile_path))
# make a copy of the install directory to work with
workdir = os.path.join(tempfile.mkdtemp(), os.path.basename(spec.prefix))
# set symlinks=False here to avoid broken symlinks when archiving and
# moving the package
install_tree(spec.prefix, workdir, symlinks=False)
install_tree(spec.prefix, workdir, symlinks=True)
# create info for later relocation and create tar
write_buildinfo_file(spec.prefix, workdir, rel=rel)
@@ -287,7 +300,7 @@ def build_tarball(spec, outdir, force=False, rel=False, unsigned=False,
tty.die(str(e))
else:
try:
make_package_placeholder(workdir, allow_root)
make_package_placeholder(workdir, spec.prefix, allow_root)
except Exception as e:
shutil.rmtree(workdir)
shutil.rmtree(tarfile_dir)
@@ -366,7 +379,8 @@ def download_tarball(spec):
def make_package_relative(workdir, prefix, allow_root):
"""
Change paths in binaries to relative paths
Change paths in binaries to relative paths. Change absolute symlinks
to relative symlinks.
"""
buildinfo = read_buildinfo_file(workdir)
old_path = buildinfo['buildpath']
@@ -377,9 +391,15 @@ def make_package_relative(workdir, prefix, allow_root):
cur_path_names.append(os.path.join(workdir, filename))
relocate.make_binary_relative(cur_path_names, orig_path_names,
old_path, allow_root)
orig_path_names = list()
cur_path_names = list()
for filename in buildinfo.get('relocate_links', []):
orig_path_names.append(os.path.join(prefix, filename))
cur_path_names.append(os.path.join(workdir, filename))
relocate.make_link_relative(cur_path_names, orig_path_names)
def make_package_placeholder(workdir, allow_root):
def make_package_placeholder(workdir, prefix, allow_root):
"""
Change paths in binaries to placeholder paths
"""
@@ -389,6 +409,11 @@ def make_package_placeholder(workdir, allow_root):
cur_path_names.append(os.path.join(workdir, filename))
relocate.make_binary_placeholder(cur_path_names, allow_root)
cur_path_names = list()
for filename in buildinfo.get('relocate_links', []):
cur_path_names.append(os.path.join(workdir, filename))
relocate.make_link_placeholder(cur_path_names, workdir, prefix)
def relocate_package(workdir, allow_root):
"""
@@ -419,6 +444,11 @@ def relocate_package(workdir, allow_root):
path_names.add(path_name)
relocate.relocate_binary(path_names, old_path, new_path,
allow_root)
path_names = set()
for filename in buildinfo.get('relocate_links', []):
path_name = os.path.join(workdir, filename)
path_names.add(path_name)
relocate.relocate_links(path_names, old_path, new_path)
def extract_tarball(spec, filename, allow_root=False, unsigned=False,

47
lib/spack/spack/relocate.py
View File

@@ -351,6 +351,18 @@ def relocate_binary(path_names, old_dir, new_dir, allow_root):
tty.die("Relocation not implemented for %s" % platform.system())
def make_link_relative(cur_path_names, orig_path_names):
"""
Change absolute links to be relative.
"""
for cur_path, orig_path in zip(cur_path_names, orig_path_names):
old_src = os.readlink(orig_path)
new_src = os.path.relpath(old_src, orig_path)
os.unlink(cur_path)
os.symlink(new_src, cur_path)
def make_binary_relative(cur_path_names, orig_path_names, old_dir, allow_root):
"""
Replace old RPATHs with paths relative to old_dir in binary files
@@ -415,6 +427,41 @@ def make_binary_placeholder(cur_path_names, allow_root):
tty.die("Placeholder not implemented for %s" % platform.system())
def make_link_placeholder(cur_path_names, cur_dir, old_dir):
"""
Replace old install path with placeholder in absolute links.
Links in ``cur_path_names`` must link to absolute paths.
"""
for cur_path in cur_path_names:
placeholder = set_placeholder(spack.store.layout.root)
placeholder_prefix = old_dir.replace(spack.store.layout.root,
placeholder)
cur_src = os.readlink(cur_path)
rel_src = os.path.relpath(cur_src, cur_dir)
new_src = os.path.join(placeholder_prefix, rel_src)
os.unlink(cur_path)
os.symlink(new_src, cur_path)
def relocate_links(path_names, old_dir, new_dir):
"""
Replace old path with new path in link sources.
Links in ``path_names`` must link to absolute paths or placeholders.
"""
placeholder = set_placeholder(old_dir)
for path_name in path_names:
old_src = os.readlink(path_name)
# replace either placeholder or old_dir
new_src = old_src.replace(placeholder, new_dir, 1)
new_src = new_src.replace(old_dir, new_dir, 1)
os.unlink(path_name)
os.symlink(new_src, path_name)
def relocate_text(path_names, old_dir, new_dir):
"""
Replace old path with new path in text file path_name

49
lib/spack/spack/test/packaging.py
View File

@@ -25,7 +25,7 @@
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 get_patchelf, relocate_text, relocate_links
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
@@ -85,9 +85,12 @@ def test_buildcache(mock_archive, tmpdir):
with open(filename, "w") as script:
script.write(spec.prefix)
# Create an absolute symlink
linkname = os.path.join(spec.prefix, "link_to_dummy.txt")
os.symlink(filename, linkname)
# Create the build cache and
# put it directly into the mirror
mirror_path = os.path.join(str(tmpdir), 'test-mirror')
spack.mirror.create(
mirror_path, specs=[], no_checksum=True
@@ -100,6 +103,7 @@ def test_buildcache(mock_archive, tmpdir):
stage = spack.stage.Stage(
mirrors['spack-mirror-test'], name="build_cache", keep=True)
stage.create()
# setup argument parser
parser = argparse.ArgumentParser()
buildcache.setup_parser(parser)
@@ -120,6 +124,13 @@ def test_buildcache(mock_archive, tmpdir):
args = parser.parse_args(['install', '-f', str(pkghash)])
buildcache.buildcache(parser, args)
files = os.listdir(spec.prefix)
assert 'link_to_dummy.txt' in files
assert 'dummy.txt' in files
assert os.path.realpath(
os.path.join(spec.prefix, 'link_to_dummy.txt')
) == os.path.realpath(os.path.join(spec.prefix, 'dummy.txt'))
# create build cache with relative path and signing
args = parser.parse_args(
['create', '-d', mirror_path, '-f', '-r', str(spec)])
@@ -136,6 +147,13 @@ def test_buildcache(mock_archive, tmpdir):
args = parser.parse_args(['install', '-f', str(pkghash)])
buildcache.buildcache(parser, args)
files = os.listdir(spec.prefix)
assert 'link_to_dummy.txt' in files
assert 'dummy.txt' in files
assert os.path.realpath(
os.path.join(spec.prefix, 'link_to_dummy.txt')
) == os.path.realpath(os.path.join(spec.prefix, 'dummy.txt'))
else:
# create build cache without signing
args = parser.parse_args(
@@ -149,6 +167,13 @@ def test_buildcache(mock_archive, tmpdir):
args = parser.parse_args(['install', '-u', str(spec)])
buildcache.install_tarball(spec, args)
files = os.listdir(spec.prefix)
assert 'link_to_dummy.txt' in files
assert 'dummy.txt' in files
assert os.path.realpath(
os.path.join(spec.prefix, 'link_to_dummy.txt')
) == os.path.realpath(os.path.join(spec.prefix, 'dummy.txt'))
# test overwrite install without verification
args = parser.parse_args(['install', '-f', '-u', str(pkghash)])
buildcache.buildcache(parser, args)
@@ -169,9 +194,17 @@ def test_buildcache(mock_archive, tmpdir):
args = parser.parse_args(['install', '-f', '-u', str(pkghash)])
buildcache.buildcache(parser, args)
files = os.listdir(spec.prefix)
assert 'link_to_dummy.txt' in files
assert 'dummy.txt' in files
assert os.path.realpath(
os.path.join(spec.prefix, 'link_to_dummy.txt')
) == os.path.realpath(os.path.join(spec.prefix, 'dummy.txt'))
# Validate the relocation information
buildinfo = bindist.read_buildinfo_file(spec.prefix)
assert(buildinfo['relocate_textfiles'] == ['dummy.txt'])
assert(buildinfo['relocate_links'] == ['link_to_dummy.txt'])
args = parser.parse_args(['list'])
buildcache.buildcache(parser, args)
@@ -219,6 +252,18 @@ def test_relocate_text(tmpdir):
assert(strings_contains_installroot(filename, old_dir) is False)
def test_relocate_links(tmpdir):
with tmpdir.as_cwd():
old_dir = '/home/spack/opt/spack'
filename = 'link.ln'
old_src = os.path.join(old_dir, filename)
os.symlink(old_src, filename)
filenames = [filename]
new_dir = '/opt/rh/devtoolset/'
relocate_links(filenames, old_dir, new_dir)
assert os.path.realpath(filename) == os.path.join(new_dir, filename)
def test_needs_relocation():
binary_type = (
'ELF 64-bit LSB executable, x86-64, version 1 (SYSV),'