spack buildcache: symlink and relative RPATH fixes (#6140)

* Skip rewriting files that are links (this also avoids issues with parsing
  the output of the 'file' command for symlinks)
* Fail rather than warn for several gpg signing issues (e.g. if there is no
  key available to sign with)
* Installation with 'buildcache install' only retrieves link and run
  dependencies (this matches 'buildcache create' which only creates tarballs
  of link/run dependencies)
* Don't rewrite RPATHs for a binary if the binary cached package was created
  with relative RPATHs
* Refactor relocate_binary to operate on multiple binaries (given as a
  collection of paths). This was likewise done for relocate_text and
  make_binary_relative
This commit is contained in:
Patrick Gartung 2017-11-09 12:53:34 -06:00 committed by scheibelp
parent ee867db5de
commit 1484a94b1e
4 changed files with 152 additions and 105 deletions

View File

@ -95,7 +95,7 @@ def read_buildinfo_file(prefix):
return buildinfo return buildinfo
def write_buildinfo_file(prefix): def write_buildinfo_file(prefix, rel=False):
""" """
Create a cache file containing information Create a cache file containing information
required for the relocation required for the relocation
@ -119,6 +119,7 @@ def write_buildinfo_file(prefix):
# Create buildinfo data and write it to disk # Create buildinfo data and write it to disk
buildinfo = {} buildinfo = {}
buildinfo['relative_rpaths'] = rel
buildinfo['buildpath'] = spack.store.layout.root buildinfo['buildpath'] = spack.store.layout.root
buildinfo['relocate_textfiles'] = text_to_relocate buildinfo['relocate_textfiles'] = text_to_relocate
buildinfo['relocate_binaries'] = binary_to_relocate buildinfo['relocate_binaries'] = binary_to_relocate
@ -132,7 +133,7 @@ def tarball_directory_name(spec):
Return name of the tarball directory according to the convention Return name of the tarball directory according to the convention
<os>-<architecture>/<compiler>/<package>-<version>/ <os>-<architecture>/<compiler>/<package>-<version>/
""" """
return "%s/%s/%s-%s" % (spack.architecture.sys_type(), return "%s/%s/%s-%s" % (spec.architecture,
str(spec.compiler).replace("@", "-"), str(spec.compiler).replace("@", "-"),
spec.name, spec.version) spec.name, spec.version)
@ -142,7 +143,7 @@ def tarball_name(spec, ext):
Return the name of the tarfile according to the convention Return the name of the tarfile according to the convention
<os>-<architecture>-<package>-<dag_hash><ext> <os>-<architecture>-<package>-<dag_hash><ext>
""" """
return "%s-%s-%s-%s-%s%s" % (spack.architecture.sys_type(), return "%s-%s-%s-%s-%s%s" % (spec.architecture,
str(spec.compiler).replace("@", "-"), str(spec.compiler).replace("@", "-"),
spec.name, spec.name,
spec.version, spec.version,
@ -246,7 +247,7 @@ def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False,
install_tree(spec.prefix, workdir, symlinks=True) install_tree(spec.prefix, workdir, symlinks=True)
# create info for later relocation and create tar # create info for later relocation and create tar
write_buildinfo_file(workdir) write_buildinfo_file(workdir, rel=rel)
# optinally make the paths in the binaries relative to each other # optinally make the paths in the binaries relative to each other
# in the spack install tree before creating tarball # in the spack install tree before creating tarball
@ -333,10 +334,13 @@ def make_package_relative(workdir, prefix):
""" """
buildinfo = read_buildinfo_file(workdir) buildinfo = read_buildinfo_file(workdir)
old_path = buildinfo['buildpath'] old_path = buildinfo['buildpath']
orig_path_names = list()
cur_path_names = list()
for filename in buildinfo['relocate_binaries']: for filename in buildinfo['relocate_binaries']:
orig_path_name = os.path.join(prefix, filename) orig_path_names.append(os.path.join(prefix, filename))
cur_path_name = os.path.join(workdir, filename) cur_path_names.append(os.path.join(workdir, filename))
relocate.make_binary_relative(cur_path_name, orig_path_name, old_path) relocate.make_binary_relative(cur_path_names, orig_path_names,
old_path)
def relocate_package(prefix): def relocate_package(prefix):
@ -346,18 +350,25 @@ def relocate_package(prefix):
buildinfo = read_buildinfo_file(prefix) buildinfo = read_buildinfo_file(prefix)
new_path = spack.store.layout.root new_path = spack.store.layout.root
old_path = buildinfo['buildpath'] old_path = buildinfo['buildpath']
if new_path == old_path: rel = buildinfo.get('relative_rpaths', False)
if new_path == old_path and not rel:
return return
tty.msg("Relocating package from", tty.msg("Relocating package from",
"%s to %s." % (old_path, new_path)) "%s to %s." % (old_path, new_path))
for filename in buildinfo['relocate_binaries']: path_names = set()
path_name = os.path.join(prefix, filename)
relocate.relocate_binary(path_name, old_path, new_path)
for filename in buildinfo['relocate_textfiles']: for filename in buildinfo['relocate_textfiles']:
path_name = os.path.join(prefix, filename) path_name = os.path.join(prefix, filename)
relocate.relocate_text(path_name, old_path, new_path) 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 buildinfo['relocate_binaries']:
path_name = os.path.join(prefix, filename)
path_names.add(path_name)
relocate.relocate_binary(path_names, old_path, new_path)
def extract_tarball(spec, filename, yes_to_all=False, force=False): def extract_tarball(spec, filename, yes_to_all=False, force=False):
@ -391,17 +402,16 @@ def extract_tarball(spec, filename, yes_to_all=False, force=False):
# get the sha256 checksum of the tarball # get the sha256 checksum of the tarball
checksum = checksum_tarball(tarfile_path) checksum = checksum_tarball(tarfile_path)
if not yes_to_all: # get the sha256 checksum recorded at creation
# get the sha256 checksum recorded at creation spec_dict = {}
spec_dict = {} with open(specfile_path, 'r') as inputfile:
with open(specfile_path, 'r') as inputfile: content = inputfile.read()
content = inputfile.read() spec_dict = yaml.load(content)
spec_dict = yaml.load(content) bchecksum = spec_dict['binary_cache_checksum']
bchecksum = spec_dict['binary_cache_checksum']
# if the checksums don't match don't install # if the checksums don't match don't install
if bchecksum['hash'] != checksum: if bchecksum['hash'] != checksum:
raise NoChecksumException() raise NoChecksumException()
# delay creating installpath until verification is complete # delay creating installpath until verification is complete
mkdirp(installpath) mkdirp(installpath)

View File

@ -227,16 +227,16 @@ def createtarball(args):
except NoOverwriteException as e: except NoOverwriteException as e:
tty.warn("%s exists, use -f to force overwrite." % e) tty.warn("%s exists, use -f to force overwrite." % e)
except NoGpgException: except NoGpgException:
tty.warn("gpg2 is not available," tty.die("gpg2 is not available,"
" use -y to create unsigned build caches") " use -y to create unsigned build caches")
except NoKeyException: except NoKeyException:
tty.warn("no default key available for signing," tty.die("no default key available for signing,"
" use -y to create unsigned build caches" " use -y to create unsigned build caches"
" or spack gpg init to create a default key") " or spack gpg init to create a default key")
except PickKeyException: except PickKeyException:
tty.warn("multi keys available for signing," tty.die("multi keys available for signing,"
" use -y to create unsigned build caches" " use -y to create unsigned build caches"
" or -k <key hash> to pick a key") " or -k <key hash> to pick a key")
def installtarball(args): def installtarball(args):
@ -267,7 +267,7 @@ def install_tarball(spec, args):
force = False force = False
if args.force: if args.force:
force = True force = True
for d in s.dependencies(): for d in s.dependencies(deptype=('link', 'run')):
tty.msg("Installing buildcache for dependency spec %s" % d) tty.msg("Installing buildcache for dependency spec %s" % d)
install_tarball(d, args) install_tarball(d, args)
package = spack.repo.get(spec) package = spack.repo.get(spec)

View File

@ -91,7 +91,7 @@ def macho_get_paths(path_name):
otool = Executable('otool') otool = Executable('otool')
output = otool("-l", path_name, output=str, err=str) output = otool("-l", path_name, output=str, err=str)
last_cmd = None last_cmd = None
idpath = '' idpath = None
rpaths = [] rpaths = []
deps = [] deps = []
for line in output.split('\n'): for line in output.split('\n'):
@ -119,24 +119,24 @@ def macho_make_paths_relative(path_name, old_dir, rpaths, deps, idpath):
in rpaths and deps; idpaths are replaced with @rpath/basebane(path_name); in rpaths and deps; idpaths are replaced with @rpath/basebane(path_name);
replacement are returned. replacement are returned.
""" """
id = None new_idpath = None
nrpaths = []
ndeps = []
if idpath: if idpath:
id = '@rpath/%s' % os.path.basename(idpath) new_idpath = '@rpath/%s' % os.path.basename(idpath)
new_rpaths = list()
new_deps = list()
for rpath in rpaths: for rpath in rpaths:
if re.match(old_dir, rpath): if re.match(old_dir, rpath):
rel = os.path.relpath(rpath, start=os.path.dirname(path_name)) rel = os.path.relpath(rpath, start=os.path.dirname(path_name))
nrpaths.append('@loader_path/%s' % rel) new_rpaths.append('@loader_path/%s' % rel)
else: else:
nrpaths.append(rpath) new_rpaths.append(rpath)
for dep in deps: for dep in deps:
if re.match(old_dir, dep): if re.match(old_dir, dep):
rel = os.path.relpath(dep, start=os.path.dirname(path_name)) rel = os.path.relpath(dep, start=os.path.dirname(path_name))
ndeps.append('@loader_path/%s' % rel) new_deps.append('@loader_path/%s' % rel)
else: else:
ndeps.append(dep) new_deps.append(dep)
return nrpaths, ndeps, id return (new_rpaths, new_deps, new_idpath)
def macho_replace_paths(old_dir, new_dir, rpaths, deps, idpath): def macho_replace_paths(old_dir, new_dir, rpaths, deps, idpath):
@ -144,22 +144,22 @@ def macho_replace_paths(old_dir, new_dir, rpaths, deps, idpath):
Replace old_dir with new_dir in rpaths, deps and idpath Replace old_dir with new_dir in rpaths, deps and idpath
and return replacements and return replacements
""" """
id = None new_idpath = None
nrpaths = []
ndeps = []
if idpath: if idpath:
id = idpath.replace(old_dir, new_dir) new_idpath = idpath.replace(old_dir, new_dir)
new_rpaths = list()
new_deps = list()
for rpath in rpaths: for rpath in rpaths:
nrpath = rpath.replace(old_dir, new_dir) new_rpath = rpath.replace(old_dir, new_dir)
nrpaths.append(nrpath) new_rpaths.append(new_rpath)
for dep in deps: for dep in deps:
ndep = dep.replace(old_dir, new_dir) new_dep = dep.replace(old_dir, new_dir)
ndeps.append(ndep) new_deps.append(new_dep)
return nrpaths, ndeps, id return new_rpaths, new_deps, new_idpath
def modify_macho_object(cur_path_name, orig_path_name, old_dir, new_dir, def modify_macho_object(cur_path, rpaths, deps, idpath,
relative): new_rpaths, new_deps, new_idpath):
""" """
Modify MachO binary path_name by replacing old_dir with new_dir Modify MachO binary path_name by replacing old_dir with new_dir
or the relative path to spack install root. or the relative path to spack install root.
@ -171,28 +171,18 @@ def modify_macho_object(cur_path_name, orig_path_name, old_dir, new_dir,
install_name_tool -rpath old new binary install_name_tool -rpath old new binary
""" """
# avoid error message for libgcc_s # avoid error message for libgcc_s
if 'libgcc_' in cur_path_name: if 'libgcc_' in cur_path:
return return
rpaths, deps, idpath = macho_get_paths(cur_path_name)
id = None
nrpaths = []
ndeps = []
if relative:
nrpaths, ndeps, id = macho_make_paths_relative(orig_path_name,
old_dir, rpaths,
deps, idpath)
else:
nrpaths, ndeps, id = macho_replace_paths(old_dir, new_dir, rpaths,
deps, idpath)
install_name_tool = Executable('install_name_tool') install_name_tool = Executable('install_name_tool')
if id: if new_idpath:
install_name_tool('-id', id, cur_path_name, output=str, err=str) install_name_tool('-id', new_idpath, str(cur_path),
output=str, err=str)
for orig, new in zip(deps, ndeps): for orig, new in zip(deps, new_deps):
install_name_tool('-change', orig, new, cur_path_name) install_name_tool('-change', orig, new, str(cur_path))
for orig, new in zip(rpaths, nrpaths): for orig, new in zip(rpaths, new_rpaths):
install_name_tool('-rpath', orig, new, cur_path_name) install_name_tool('-rpath', orig, new, str(cur_path))
return return
@ -227,6 +217,8 @@ def needs_binary_relocation(filetype):
retval = False retval = False
if "relocatable" in filetype: if "relocatable" in filetype:
return False return False
if "link" in filetype:
return False
if platform.system() == 'Darwin': if platform.system() == 'Darwin':
return ('Mach-O' in filetype) return ('Mach-O' in filetype)
elif platform.system() == 'Linux': elif platform.system() == 'Linux':
@ -240,44 +232,65 @@ def needs_text_relocation(filetype):
""" """
Check whether the given filetype is text that may need relocation. Check whether the given filetype is text that may need relocation.
""" """
if "link" in filetype:
return False
return ("text" in filetype) return ("text" in filetype)
def relocate_binary(path_name, old_dir, new_dir): def relocate_binary(path_names, old_dir, new_dir):
""" """
Change old_dir to new_dir in RPATHs of elf or mach-o file path_name Change old_dir to new_dir in RPATHs of elf or mach-o files
""" """
if platform.system() == 'Darwin': if platform.system() == 'Darwin':
modify_macho_object(path_name, old_dir, new_dir, relative=False) for path_name in path_names:
rpaths, deps, idpath = macho_get_paths(path_name)
new_rpaths, new_deps, new_idpath = macho_replace_paths(old_dir,
new_dir,
rpaths,
deps,
idpath)
modify_macho_object(path_name,
rpaths, deps, idpath,
new_rpaths, new_deps, new_idpath)
elif platform.system() == 'Linux': elif platform.system() == 'Linux':
orig_rpaths = get_existing_elf_rpaths(path_name) for path_name in path_names:
new_rpaths = substitute_rpath(orig_rpaths, old_dir, new_dir) orig_rpaths = get_existing_elf_rpaths(path_name)
modify_elf_object(path_name, orig_rpaths, new_rpaths) new_rpaths = substitute_rpath(orig_rpaths, old_dir, new_dir)
modify_elf_object(path_name, orig_rpaths, new_rpaths)
else: else:
tty.die("Relocation not implemented for %s" % platform.system()) tty.die("Relocation not implemented for %s" % platform.system())
def make_binary_relative(cur_path_name, orig_path_name, old_dir): def make_binary_relative(cur_path_names, orig_path_names, old_dir):
""" """
Make RPATHs relative to old_dir in given elf or mach-o file path_name Make RPATHs relative to old_dir in given elf or mach-o files
""" """
if platform.system() == 'Darwin': if platform.system() == 'Darwin':
new_dir = '' for cur_path, orig_path in zip(cur_path_names, orig_path_names):
modify_macho_object(cur_path_name, orig_path_name, old_dir, new_dir, rpaths, deps, idpath = macho_get_paths(cur_path)
relative=True) (new_rpaths,
new_deps,
new_idpath) = macho_make_paths_relative(orig_path, old_dir,
rpaths, deps, idpath)
modify_macho_object(cur_path,
rpaths, deps, idpath,
new_rpaths, new_deps, new_idpath)
elif platform.system() == 'Linux': elif platform.system() == 'Linux':
orig_rpaths = get_existing_elf_rpaths(cur_path_name) for cur_path, orig_path in zip(cur_path_names, orig_path_names):
new_rpaths = get_relative_rpaths(orig_path_name, old_dir, orig_rpaths) orig_rpaths = get_existing_elf_rpaths(cur_path)
modify_elf_object(cur_path_name, orig_rpaths, new_rpaths) new_rpaths = get_relative_rpaths(orig_path, old_dir,
orig_rpaths)
modify_elf_object(cur_path, orig_rpaths, new_rpaths)
else: else:
tty.die("Prelocation not implemented for %s" % platform.system()) tty.die("Prelocation not implemented for %s" % platform.system())
def relocate_text(path_name, old_dir, new_dir): def relocate_text(path_names, old_dir, new_dir):
""" """
Replace old path with new path in text file path_name Replace old path with new path in text file path_name
""" """
filter_file("r'%s'" % old_dir, "r'%s'" % new_dir, path_name) filter_file("r'%s'" % old_dir, "r'%s'" % new_dir,
*path_names, backup=False)
def substitute_rpath(orig_rpath, topdir, new_root_path): def substitute_rpath(orig_rpath, topdir, new_root_path):

View File

@ -41,7 +41,8 @@
from spack.spec import Spec from spack.spec import Spec
from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite from spack.fetch_strategy import URLFetchStrategy, FetchStrategyComposite
from spack.util.executable import ProcessError from spack.util.executable import ProcessError
from spack.relocate import needs_binary_relocation, get_patchelf from spack.relocate import needs_binary_relocation, needs_text_relocation
from spack.relocate import get_patchelf
from spack.relocate import substitute_rpath, get_relative_rpaths from spack.relocate import substitute_rpath, get_relative_rpaths
from spack.relocate import macho_replace_paths, macho_make_paths_relative from spack.relocate import macho_replace_paths, macho_make_paths_relative
from spack.relocate import modify_macho_object, macho_get_paths from spack.relocate import modify_macho_object, macho_get_paths
@ -218,6 +219,8 @@ def test_packaging(mock_archive, tmpdir):
def test_relocate(): def test_relocate():
assert (needs_binary_relocation('relocatable') is False) assert (needs_binary_relocation('relocatable') is False)
assert (needs_binary_relocation('link') is False)
assert (needs_text_relocation('link') is False)
out = macho_make_paths_relative('/Users/Shares/spack/pkgC/lib/libC.dylib', out = macho_make_paths_relative('/Users/Shares/spack/pkgC/lib/libC.dylib',
'/Users/Shared/spack', '/Users/Shared/spack',
@ -225,8 +228,8 @@ def test_relocate():
'/Users/Shared/spack/pkgB/lib', '/Users/Shared/spack/pkgB/lib',
'/usr/local/lib'), '/usr/local/lib'),
('/Users/Shared/spack/pkgA/libA.dylib', ('/Users/Shared/spack/pkgA/libA.dylib',
'/Users/Shared/spack/pkgB/libB.dylib', '/Users/Shared/spack/pkgB/libB.dylib',
'/usr/local/lib/libloco.dylib'), '/usr/local/lib/libloco.dylib'),
'/Users/Shared/spack/pkgC/lib/libC.dylib') '/Users/Shared/spack/pkgC/lib/libC.dylib')
assert out == (['@loader_path/../../../../Shared/spack/pkgA/lib', assert out == (['@loader_path/../../../../Shared/spack/pkgA/lib',
'@loader_path/../../../../Shared/spack/pkgB/lib', '@loader_path/../../../../Shared/spack/pkgB/lib',
@ -242,8 +245,8 @@ def test_relocate():
'/Users/Shared/spack/pkgB/lib', '/Users/Shared/spack/pkgB/lib',
'/usr/local/lib'), '/usr/local/lib'),
('/Users/Shared/spack/pkgA/libA.dylib', ('/Users/Shared/spack/pkgA/libA.dylib',
'/Users/Shared/spack/pkgB/libB.dylib', '/Users/Shared/spack/pkgB/libB.dylib',
'/usr/local/lib/libloco.dylib'), None) '/usr/local/lib/libloco.dylib'), None)
assert out == (['@loader_path/../../pkgA/lib', assert out == (['@loader_path/../../pkgA/lib',
'@loader_path/../../pkgB/lib', '@loader_path/../../pkgB/lib',
@ -258,8 +261,8 @@ def test_relocate():
'/Users/Shared/spack/pkgB/lib', '/Users/Shared/spack/pkgB/lib',
'/usr/local/lib'), '/usr/local/lib'),
('/Users/Shared/spack/pkgA/libA.dylib', ('/Users/Shared/spack/pkgA/libA.dylib',
'/Users/Shared/spack/pkgB/libB.dylib', '/Users/Shared/spack/pkgB/libB.dylib',
'/usr/local/lib/libloco.dylib'), '/usr/local/lib/libloco.dylib'),
'/Users/Shared/spack/pkgC/lib/libC.dylib') '/Users/Shared/spack/pkgC/lib/libC.dylib')
assert out == (['/Applications/spack/pkgA/lib', assert out == (['/Applications/spack/pkgA/lib',
'/Applications/spack/pkgB/lib', '/Applications/spack/pkgB/lib',
@ -275,8 +278,8 @@ def test_relocate():
'/Users/Shared/spack/pkgB/lib', '/Users/Shared/spack/pkgB/lib',
'/usr/local/lib'), '/usr/local/lib'),
('/Users/Shared/spack/pkgA/libA.dylib', ('/Users/Shared/spack/pkgA/libA.dylib',
'/Users/Shared/spack/pkgB/libB.dylib', '/Users/Shared/spack/pkgB/libB.dylib',
'/usr/local/lib/libloco.dylib'), '/usr/local/lib/libloco.dylib'),
None) None)
assert out == (['/Applications/spack/pkgA/lib', assert out == (['/Applications/spack/pkgA/lib',
'/Applications/spack/pkgB/lib', '/Applications/spack/pkgB/lib',
@ -301,25 +304,46 @@ def test_relocate():
def test_relocate_macho(tmpdir): def test_relocate_macho(tmpdir):
with tmpdir.as_cwd(): with tmpdir.as_cwd():
get_patchelf() get_patchelf()
assert (needs_binary_relocation('Mach-O') is True) assert (needs_binary_relocation('Mach-O'))
macho_get_paths('/bin/bash') rpaths, deps, idpath = macho_get_paths('/bin/bash')
nrpaths, ndeps, nid = macho_make_paths_relative('/bin/bash', '/usr',
rpaths, deps, idpath)
shutil.copyfile('/bin/bash', 'bash') shutil.copyfile('/bin/bash', 'bash')
modify_macho_object('bash',
rpaths, deps, idpath,
nrpaths, ndeps, nid)
modify_macho_object('bash', '/bin/bash', '/usr', '/opt', False) rpaths, deps, idpath = macho_get_paths('/bin/bash')
modify_macho_object('bash', '/bin/bash', '/usr', '/opt', True) nrpaths, ndeps, nid = macho_replace_paths('/usr', '/opt',
rpaths, deps, idpath)
shutil.copyfile('/bin/bash', 'bash')
modify_macho_object('bash',
rpaths, deps, idpath,
nrpaths, ndeps, nid)
path = '/usr/lib/libncurses.5.4.dylib'
rpaths, deps, idpath = macho_get_paths(path)
nrpaths, ndeps, nid = macho_make_paths_relative(path, '/usr',
rpaths, deps, idpath)
shutil.copyfile(
'/usr/lib/libncurses.5.4.dylib', 'libncurses.5.4.dylib')
modify_macho_object('libncurses.5.4.dylib',
rpaths, deps, idpath,
nrpaths, ndeps, nid)
rpaths, deps, idpath = macho_get_paths(path)
nrpaths, ndeps, nid = macho_replace_paths('/usr', '/opt',
rpaths, deps, idpath)
shutil.copyfile( shutil.copyfile(
'/usr/lib/libncurses.5.4.dylib', 'libncurses.5.4.dylib') '/usr/lib/libncurses.5.4.dylib', 'libncurses.5.4.dylib')
modify_macho_object( modify_macho_object(
'libncurses.5.4.dylib', 'libncurses.5.4.dylib',
'/usr/lib/libncurses.5.4.dylib', '/usr', '/opt', False) rpaths, deps, idpath,
modify_macho_object( nrpaths, ndeps, nid)
'libncurses.5.4.dylib',
'/usr/lib/libncurses.5.4.dylib', '/usr', '/opt', True)
@pytest.mark.skipif(sys.platform != 'linux2', @pytest.mark.skipif(sys.platform != 'linux2',
reason="only works with Elf objects") reason="only works with Elf objects")
def test_relocate_elf(): def test_relocate_elf():
assert (needs_binary_relocation('ELF') is True) assert (needs_binary_relocation('ELF'))