From f5467957bca49ca612cfc32710ed2ca8a943583d Mon Sep 17 00:00:00 2001 From: Patrick Gartung Date: Tue, 6 Mar 2018 20:02:53 -0600 Subject: Improved binary relocation: error checking and reporting (#7314) Fixes #7237 Fixes #6404 Fixes #6418 Fixes #6369 Identify when binary relocation fails to remove all instances of the source prefix (and report an error to the user unless they specify -a to allow the old root to appear). This check occurs at two stages: during "bincache create" all instances of the root are replaced with a special placeholder string ("@@@@..."), and a failure occurs if the root is detected at this point; when the binary package is extracted there is a second check. This addresses #7237 and #6418. This is intended to be compatible with previously-created binary packages. This also adds: * Better error messages for "spack install --use-cache" (#6404) * Faster relocation on Mac OS (using a single call to install_name_tool for all files rather than a call for each file) * Clean up when "buildcache create" fails (addresses #6369) * Explicit error message when the spack instance extracting the binary package uses a different install layout than the spack instance that created the binary package (since this is currently not supported) * Remove the option to create unsigned binary packages with -y --- lib/spack/spack/binary_distribution.py | 218 +++++++++++++++++++++++---------- lib/spack/spack/cmd/buildcache.py | 79 ++++-------- lib/spack/spack/package.py | 2 +- lib/spack/spack/relocate.py | 170 ++++++++++++++++++++++--- lib/spack/spack/test/packaging.py | 32 +++-- 5 files changed, 352 insertions(+), 149 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index e62e9aaf14..903e020b58 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -29,6 +29,7 @@ import tarfile import yaml import shutil import platform +import tempfile import llnl.util.tty as tty from spack.util.gpg import Gpg @@ -46,26 +47,57 @@ import spack.relocate as relocate class NoOverwriteException(Exception): - pass + """ + Raised when a file exists and must be overwritten. + """ + def __init__(self, file_path): + err_msg = "\n%s\nexists\n" % file_path + err_msg += "Use -f option to overwrite." + super(NoOverwriteException, self).__init__(err_msg) -class NoGpgException(Exception): +class NoGpgException(spack.error.SpackError): + """ + Raised when gpg2 is not in PATH + """ pass -class PickKeyException(Exception): +class NoKeyException(spack.error.SpackError): + """ + Raised when gpg has no default key added. + """ pass -class NoKeyException(Exception): +class PickKeyException(spack.error.SpackError): + """ + Raised when multiple keys can be used to sign. + """ + def __init__(self, keys): + err_msg = "Multi keys available for signing\n%s\n" % keys + err_msg += "Use spack buildcache create -k to pick a key." + super(PickKeyException, self).__init__(err_msg) + + +class NoVerifyException(spack.error.SpackError): + """ + Raised if file fails signature verification. + """ pass -class NoVerifyException(Exception): +class NoChecksumException(spack.error.SpackError): + """ + Raised if file fails checksum verification. + """ pass -class NoChecksumException(Exception): +class NewLayoutException(spack.error.SpackError): + """ + Raised if directory layout is different from buildcache. + """ pass @@ -114,7 +146,8 @@ def write_buildinfo_file(prefix, workdir, rel=False): # 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): + if relocate.strings_contains_installroot(path_name, + spack.store.layout.root): filetype = relocate.get_filetype(path_name) if relocate.needs_binary_relocation(filetype, os_id): rel_path_name = os.path.relpath(path_name, prefix) @@ -127,6 +160,8 @@ def write_buildinfo_file(prefix, workdir, rel=False): buildinfo = {} buildinfo['relative_rpaths'] = rel buildinfo['buildpath'] = spack.store.layout.root + buildinfo['relative_prefix'] = os.path.relpath(prefix, + spack.store.layout.root) buildinfo['relocate_textfiles'] = text_to_relocate buildinfo['relocate_binaries'] = binary_to_relocate filename = buildinfo_file_name(workdir) @@ -178,19 +213,24 @@ def checksum_tarball(file): return hasher.hexdigest() -def sign_tarball(yes_to_all, key, force, specfile_path): +def sign_tarball(key, force, specfile_path): # Sign the packages if keys available if not has_gnupg2(): - raise NoGpgException() + raise NoGpgException( + "gpg2 is not available in $PATH .\n" + "Use spack install gnupg and spack load gnupg.") else: if key is None: keys = Gpg.signing_keys() if len(keys) == 1: key = keys[0] if len(keys) > 1: - raise PickKeyException() + raise PickKeyException(str(keys)) if len(keys) == 0: - raise NoKeyException() + msg = "No default key available for signing.\n" + msg += "Use spack gpg init and spack gpg create" + msg += " to create a default key." + raise NoKeyException(msg) if os.path.exists('%s.asc' % specfile_path): if force: os.remove('%s.asc' % specfile_path) @@ -214,7 +254,7 @@ def generate_index(outdir, indexfile_path): f.close() -def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False, +def build_tarball(spec, outdir, force=False, rel=False, allow_root=False, key=None): """ Build a tarball from given spec and put it into the directory structure @@ -247,9 +287,7 @@ def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False, else: raise NoOverwriteException(str(specfile_path)) # make a copy of the install directory to work with - workdir = join_path(outdir, os.path.basename(spec.prefix)) - if os.path.exists(workdir): - shutil.rmtree(workdir) + workdir = join_path(tempfile.mkdtemp(), os.path.basename(spec.prefix)) install_tree(spec.prefix, workdir, symlinks=True) # create info for later relocation and create tar @@ -258,11 +296,23 @@ def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False, # optinally make the paths in the binaries relative to each other # in the spack install tree before creating tarball if rel: - make_package_relative(workdir, spec.prefix) + try: + make_package_relative(workdir, spec.prefix, allow_root) + except Exception as e: + shutil.rmtree(workdir) + shutil.rmtree(tarfile_dir) + tty.die(str(e)) + else: + try: + make_package_placeholder(workdir, allow_root) + except Exception as e: + shutil.rmtree(workdir) + shutil.rmtree(tarfile_dir) + tty.die(str(e)) # create compressed tarball of the install prefix with closing(tarfile.open(tarfile_path, 'w:gz')) as tar: tar.add(name='%s' % workdir, - arcname='%s' % os.path.basename(workdir)) + arcname='%s' % os.path.basename(spec.prefix)) # remove copy of install directory shutil.rmtree(workdir) @@ -278,32 +328,26 @@ def build_tarball(spec, outdir, force=False, rel=False, yes_to_all=False, bchecksum['hash_algorithm'] = 'sha256' bchecksum['hash'] = checksum spec_dict['binary_cache_checksum'] = bchecksum + # Add original install prefix relative to layout root to spec.yaml. + # This will be used to determine is the directory layout has changed. + buildinfo = {} + buildinfo['relative_prefix'] = os.path.relpath(spec.prefix, + spack.store.layout.root) + spec_dict['buildinfo'] = buildinfo with open(specfile_path, 'w') as outfile: outfile.write(yaml.dump(spec_dict)) - signed = False - if not yes_to_all: - # sign the tarball and spec file with gpg - try: - sign_tarball(yes_to_all, key, force, specfile_path) - signed = True - except NoGpgException: - raise NoGpgException() - except PickKeyException: - raise PickKeyException() - except NoKeyException(): - raise NoKeyException() + # sign the tarball and spec file with gpg + sign_tarball(key, force, specfile_path) # put tarball, spec and signature files in .spack archive with closing(tarfile.open(spackfile_path, 'w')) as tar: tar.add(name='%s' % tarfile_path, arcname='%s' % tarfile_name) tar.add(name='%s' % specfile_path, arcname='%s' % specfile_name) - if signed: - tar.add(name='%s.asc' % specfile_path, - arcname='%s.asc' % specfile_name) + tar.add(name='%s.asc' % specfile_path, + arcname='%s.asc' % specfile_name) # cleanup file moved to archive os.remove(tarfile_path) - if signed: - os.remove('%s.asc' % specfile_path) + os.remove('%s.asc' % specfile_path) # create an index.html for the build_cache directory so specs can be found if os.path.exists(indexfile_path): @@ -334,7 +378,7 @@ def download_tarball(spec): return None -def make_package_relative(workdir, prefix): +def make_package_relative(workdir, prefix, allow_root): """ Change paths in binaries to relative paths """ @@ -345,15 +389,26 @@ def make_package_relative(workdir, prefix): for filename in buildinfo['relocate_binaries']: orig_path_names.append(os.path.join(prefix, filename)) cur_path_names.append(os.path.join(workdir, filename)) - relocate.make_binary_relative(cur_path_names, orig_path_names, - old_path) + relocate.make_binary_relative(cur_path_names, orig_path_names, + old_path, allow_root) -def relocate_package(prefix): +def make_package_placeholder(workdir, allow_root): + """ + Change paths in binaries to placeholder paths + """ + buildinfo = read_buildinfo_file(workdir) + cur_path_names = list() + for filename in buildinfo['relocate_binaries']: + cur_path_names.append(os.path.join(workdir, filename)) + relocate.make_binary_placeholder(cur_path_names, allow_root) + + +def relocate_package(workdir, allow_root): """ Relocate the given package """ - buildinfo = read_buildinfo_file(prefix) + buildinfo = read_buildinfo_file(workdir) new_path = spack.store.layout.root old_path = buildinfo['buildpath'] rel = buildinfo.get('relative_rpaths', False) @@ -364,7 +419,7 @@ def relocate_package(prefix): "%s to %s." % (old_path, new_path)) path_names = set() for filename in buildinfo['relocate_textfiles']: - path_name = os.path.join(prefix, filename) + path_name = os.path.join(workdir, filename) # Don't add backup files generated by filter_file during install step. if not path_name.endswith('~'): path_names.add(path_name) @@ -374,39 +429,46 @@ def relocate_package(prefix): if not rel: path_names = set() for filename in buildinfo['relocate_binaries']: - path_name = os.path.join(prefix, filename) + path_name = os.path.join(workdir, filename) path_names.add(path_name) - relocate.relocate_binary(path_names, old_path, new_path) + relocate.relocate_binary(path_names, old_path, new_path, + allow_root) -def extract_tarball(spec, filename, yes_to_all=False, force=False): +def extract_tarball(spec, filename, allow_root=False, force=False): """ extract binary tarball for given package into install area """ - installpath = spec.prefix - if os.path.exists(installpath): + if os.path.exists(spec.prefix): if force: - shutil.rmtree(installpath) + shutil.rmtree(spec.prefix) else: - raise NoOverwriteException(str(installpath)) + raise NoOverwriteException(str(spec.prefix)) + + tmpdir = tempfile.mkdtemp() stagepath = os.path.dirname(filename) spackfile_name = tarball_name(spec, '.spack') spackfile_path = os.path.join(stagepath, spackfile_name) tarfile_name = tarball_name(spec, '.tar.gz') - tarfile_path = os.path.join(stagepath, tarfile_name) + tarfile_path = os.path.join(tmpdir, tarfile_name) specfile_name = tarball_name(spec, '.spec.yaml') - specfile_path = os.path.join(stagepath, specfile_name) + specfile_path = os.path.join(tmpdir, specfile_name) with closing(tarfile.open(spackfile_path, 'r')) as tar: - tar.extractall(stagepath) + tar.extractall(tmpdir) - if not yes_to_all: - if os.path.exists('%s.asc' % specfile_path): + if os.path.exists('%s.asc' % specfile_path): + try: Gpg.verify('%s.asc' % specfile_path, specfile_path) - os.remove(specfile_path + '.asc') - else: - raise NoVerifyException() - + except Exception as e: + shutil.rmtree(tmpdir) + tty.die(str(e)) + else: + shutil.rmtree(tmpdir) + raise NoVerifyException( + "Package spec file failed signature verification.\n" + "Use spack buildcache keys to download " + "and install a key for verification from the mirror.") # get the sha256 checksum of the tarball checksum = checksum_tarball(tarfile_path) @@ -419,16 +481,48 @@ def extract_tarball(spec, filename, yes_to_all=False, force=False): # if the checksums don't match don't install if bchecksum['hash'] != checksum: - raise NoChecksumException() - - # delay creating installpath until verification is complete - mkdirp(installpath) + shutil.rmtree(tmpdir) + raise NoChecksumException( + "Package tarball failed checksum verification.\n" + "It cannot be installed.") + + new_relative_prefix = str(os.path.relpath(spec.prefix, + spack.store.layout.root)) + # if the original relative prefix is in the spec file use it + buildinfo = spec_dict.get('buildinfo', {}) + old_relative_prefix = buildinfo.get('relative_prefix', new_relative_prefix) + # if the original relative prefix and new relative prefix differ the + # directory layout has changed and the buildcache cannot be installed + if old_relative_prefix != new_relative_prefix: + shutil.rmtree(tmpdir) + msg = "Package tarball was created from an install " + msg += "prefix with a different directory layout.\n" + msg += "It cannot be relocated." + raise NewLayoutException(msg) + + # extract the tarball in a temp directory with closing(tarfile.open(tarfile_path, 'r')) as tar: - tar.extractall(path=join_path(installpath, '..')) + tar.extractall(path=tmpdir) + # the base of the install prefix is used when creating the tarball + # so the pathname should be the same now that the directory layout + # is confirmed + workdir = join_path(tmpdir, os.path.basename(spec.prefix)) + # cleanup os.remove(tarfile_path) os.remove(specfile_path) - relocate_package(installpath) + + try: + relocate_package(workdir, allow_root) + except Exception as e: + shutil.rmtree(workdir) + tty.die(str(e)) + # Delay creating spec.prefix until verification is complete + # and any relocation has been done. + else: + install_tree(workdir, spec.prefix, symlinks=True) + finally: + shutil.rmtree(tmpdir) def get_specs(force=False): diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py index 7dc48cd220..cc22fb33fd 100644 --- a/lib/spack/spack/cmd/buildcache.py +++ b/lib/spack/spack/cmd/buildcache.py @@ -24,15 +24,11 @@ ############################################################################## import argparse -import os import llnl.util.tty as tty import spack import spack.cmd import spack.binary_distribution as bindist -from spack.binary_distribution import NoOverwriteException, NoGpgException -from spack.binary_distribution import NoKeyException, PickKeyException -from spack.binary_distribution import NoVerifyException, NoChecksumException description = "create, download and install binary packages" section = "packaging" @@ -49,9 +45,9 @@ def setup_parser(subparser): " before creating tarballs.") create.add_argument('-f', '--force', action='store_true', help="overwrite tarball if it exists.") - create.add_argument('-y', '--yes-to-all', action='store_true', - help="answer yes to all create unsigned " + - "buildcache questions") + create.add_argument('-a', '--allow_root', action='store_true', + help="allow install root string in binary files " + + "after RPATH substitution") create.add_argument('-k', '--key', metavar='key', type=str, default=None, help="Key for signing.") @@ -66,9 +62,11 @@ def setup_parser(subparser): install = subparsers.add_parser('install', help=installtarball.__doc__) install.add_argument('-f', '--force', action='store_true', help="overwrite install directory if it exists.") - install.add_argument('-y', '--yes-to-all', action='store_true', - help="answer yes to all install unsigned " + - "buildcache questions") + install.add_argument('-m', '--multiple', action='store_true', + help="allow all matching packages ") + install.add_argument('-a', '--allow_root', action='store_true', + help="allow install root string in binary files " + + "after RPATH substitution") install.add_argument( 'packages', nargs=argparse.REMAINDER, help="specs of packages to install biuldache for") @@ -87,7 +85,7 @@ def setup_parser(subparser): '-i', '--install', action='store_true', help="install Keys pulled from mirror") dlkeys.add_argument( - '-y', '--yes-to-all', action='store_true', + '-y', '--yes_to_all', action='store_true', help="answer yes to all trust questions") dlkeys.add_argument('-f', '--force', action='store_true', help="force new download of keys") @@ -185,17 +183,17 @@ def createtarball(args): " installed package argument") pkgs = set(args.packages) specs = set() - outdir = os.getcwd() + outdir = '.' if args.directory: outdir = args.directory signkey = None if args.key: signkey = args.key - yes_to_all = False + allow_root = False force = False relative = False - if args.yes_to_all: - yes_to_all = True + if args.allow_root: + allow_root = True if args.force: force = True if args.rel: @@ -220,24 +218,12 @@ def createtarball(args): tty.msg('adding dependency %s' % node.format()) specs.add(node) + tty.msg('writing tarballs to %s/build_cache' % outdir) + for spec in specs: tty.msg('creating binary cache file for package %s ' % spec.format()) - try: - bindist.build_tarball(spec, outdir, force, - relative, yes_to_all, signkey) - except NoOverwriteException as e: - tty.warn("%s exists, use -f to force overwrite." % e) - except NoGpgException: - tty.die("gpg2 is not available," - " use -y to create unsigned build caches") - except NoKeyException: - tty.die("no default key available for signing," - " use -y to create unsigned build caches" - " or spack gpg init to create a default key") - except PickKeyException: - tty.die("multi keys available for signing," - " use -y to create unsigned build caches" - " or -k to pick a key") + bindist.build_tarball(spec, outdir, force, + relative, allow_root, signkey) def installtarball(args): @@ -246,13 +232,13 @@ def installtarball(args): tty.die("build cache file installation requires" + " at least one package spec argument") pkgs = set(args.packages) - yes_to_all = False - if args.yes_to_all: - yes_to_all = True + multiple = False + if args.multiple: + multiple = True force = False if args.force: force = True - matches = match_downloaded_specs(pkgs, yes_to_all, force) + matches = match_downloaded_specs(pkgs, multiple, force) for match in matches: install_tarball(match, args) @@ -263,9 +249,9 @@ def install_tarball(spec, args): if s.external or s.virtual: tty.warn("Skipping external or virtual package %s" % spec.format()) return - yes_to_all = False - if args.yes_to_all: - yes_to_all = True + allow_root = False + if args.allow_root: + allow_root = True force = False if args.force: force = True @@ -274,24 +260,13 @@ def install_tarball(spec, args): install_tarball(d, args) package = spack.repo.get(spec) if s.concrete and package.installed and not force: - tty.warn("Package for spec %s already installed." % spec.format(), - " Use -f flag to overwrite.") + tty.warn("Package for spec %s already installed." % spec.format()) else: tarball = bindist.download_tarball(spec) if tarball: tty.msg('Installing buildcache for spec %s' % spec.format()) - try: - bindist.extract_tarball(spec, tarball, yes_to_all, force) - except NoOverwriteException as e: - tty.warn("%s exists. use -f to force overwrite." % e.args) - except NoVerifyException: - tty.die("Package spec file failed signature verification," - " use -y flag to install build cache") - except NoChecksumException: - tty.die("Package tarball failed checksum verification," - " use -y flag to install build cache") - finally: - spack.store.db.reindex(spack.store.layout) + bindist.extract_tarball(spec, tarball, allow_root, force) + spack.store.db.reindex(spack.store.layout) else: tty.die('Download of binary cache file for spec %s failed.' % spec.format()) diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 82af2375af..91382f2efb 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1288,7 +1288,7 @@ class PackageBase(with_metaclass(PackageMeta, object)): tty.msg('Installing %s from binary cache' % self.name) tarball = binary_distribution.download_tarball(binary_spec) binary_distribution.extract_tarball( - binary_spec, tarball, yes_to_all=False, force=False) + binary_spec, tarball, allow_root=False, force=False) spack.store.db.add(self.spec, spack.store.layout, explicit=explicit) return True diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 325ec22509..29c062a713 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -33,6 +33,18 @@ from llnl.util.filesystem import filter_file import llnl.util.tty as tty +class InstallRootStringException(spack.error.SpackError): + """ + Raised when the relocated binary still has the install root string. + """ + def __init__(self, file_path, root_path): + super(InstallRootStringException, self).__init__( + "\n %s \ncontains string\n %s \n" + "after replacing it in rpaths.\n" + "Package should not be relocated.\n Use -a to override." % + (file_path, root_path)) + + def get_patchelf(): """ Builds and installs spack patchelf package on linux platforms @@ -86,6 +98,29 @@ def get_relative_rpaths(path_name, orig_dir, orig_rpaths): return rel_rpaths +def set_placeholder(dirname): + """ + return string of @'s with same length + """ + return '@' * len(dirname) + + +def get_placeholder_rpaths(path_name, orig_rpaths): + """ + Replaces original layout root dir with a placeholder string in all rpaths. + """ + rel_rpaths = [] + orig_dir = spack.store.layout.root + for rpath in orig_rpaths: + if re.match(orig_dir, rpath): + placeholder = set_placeholder(orig_dir) + rel = re.sub(orig_dir, placeholder, rpath) + rel_rpaths.append('%s' % rel) + else: + rel_rpaths.append(rpath) + return rel_rpaths + + def macho_get_paths(path_name): """ Examines the output of otool -l path_name for these three fields: @@ -121,7 +156,7 @@ def macho_get_paths(path_name): def macho_make_paths_relative(path_name, old_dir, rpaths, deps, idpath): """ Replace old_dir with relative path from dirname(path_name) - in rpaths and deps; idpaths are replaced with @rpath/basebane(path_name); + in rpaths and deps; idpaths are replaced with @rpath/libname as needed; replacement are returned. """ new_idpath = None @@ -144,6 +179,34 @@ def macho_make_paths_relative(path_name, old_dir, rpaths, deps, idpath): return (new_rpaths, new_deps, new_idpath) +def macho_make_paths_placeholder(rpaths, deps, idpath): + """ + Replace old_dir with a placeholder of the same length + in rpaths and deps and idpaths is needed. + replacement are returned. + """ + new_idpath = None + old_dir = spack.store.layout.root + placeholder = set_placeholder(old_dir) + if idpath: + new_idpath = re.sub(old_dir, placeholder, idpath) + new_rpaths = list() + new_deps = list() + for rpath in rpaths: + if re.match(old_dir, rpath): + ph = re.sub(old_dir, placeholder, rpath) + new_rpaths.append('%s' % ph) + else: + new_rpaths.append(rpath) + for dep in deps: + if re.match(old_dir, dep): + ph = re.sub(old_dir, placeholder, dep) + new_deps.append('%s' % ph) + else: + new_deps.append(dep) + return (new_rpaths, new_deps, new_idpath) + + def macho_replace_paths(old_dir, new_dir, rpaths, deps, idpath): """ Replace old_dir with new_dir in rpaths, deps and idpath @@ -179,15 +242,17 @@ def modify_macho_object(cur_path, rpaths, deps, idpath, if 'libgcc_' in cur_path: return install_name_tool = Executable('install_name_tool') + args = [] if new_idpath: - install_name_tool('-id', new_idpath, str(cur_path), - output=str, err=str) + args.extend(['-id', new_idpath]) for orig, new in zip(deps, new_deps): - install_name_tool('-change', orig, new, str(cur_path)) + args.extend(['-change', orig, new]) for orig, new in zip(rpaths, new_rpaths): - install_name_tool('-rpath', orig, new, str(cur_path)) + args.extend(['-rpath', orig, new]) + args.append(str(cur_path)) + install_name_tool(*args) return @@ -202,14 +267,14 @@ def get_filetype(path_name): return output.strip() -def strings_contains_installroot(path_name): +def strings_contains_installroot(path_name, root_dir): """ 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) + return (root_dir in output) def modify_elf_object(path_name, new_rpaths): @@ -257,34 +322,66 @@ def needs_text_relocation(filetype): return ("text" in filetype) -def relocate_binary(path_names, old_dir, new_dir): +def relocate_binary(path_names, old_dir, new_dir, allow_root): """ Change old_dir to new_dir in RPATHs of elf or mach-o files + Account for the case where old_dir is now a placeholder """ + placeholder = set_placeholder(old_dir) if platform.system() == 'Darwin': 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) + (rpaths, deps, idpath) = macho_get_paths(path_name) + # new style buildaches with placeholder in binaries + if (deps[0].startswith(placeholder) or + rpaths[0].startswith(placeholder) or + (idpath and idpath.startswith(placeholder))): + (new_rpaths, + new_deps, + new_idpath) = macho_replace_paths(placeholder, + new_dir, + rpaths, + deps, + idpath) + # old style buildcaches with original install root in binaries + else: + (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) + if (not allow_root and + strings_contains_installroot(path_name, old_dir)): + raise InstallRootStringException(path_name, old_dir) + elif platform.system() == 'Linux': for path_name in path_names: orig_rpaths = get_existing_elf_rpaths(path_name) if orig_rpaths: - new_rpaths = substitute_rpath(orig_rpaths, old_dir, new_dir) + if orig_rpaths[0].startswith(placeholder): + # new style buildaches with placeholder in binaries + new_rpaths = substitute_rpath(orig_rpaths, + placeholder, new_dir) + else: + # old style buildcaches with original install + # root in binaries + new_rpaths = substitute_rpath(orig_rpaths, + old_dir, new_dir) modify_elf_object(path_name, new_rpaths) + if (not allow_root and + strings_contains_installroot(path_name, old_dir)): + raise InstallRootStringException(path_name, old_dir) else: tty.die("Relocation not implemented for %s" % platform.system()) -def make_binary_relative(cur_path_names, orig_path_names, old_dir): +def make_binary_relative(cur_path_names, orig_path_names, old_dir, allow_root): """ - Make RPATHs relative to old_dir in given elf or mach-o files + Replace old RPATHs with paths relative to old_dir in binary files """ if platform.system() == 'Darwin': for cur_path, orig_path in zip(cur_path_names, orig_path_names): @@ -296,6 +393,9 @@ def make_binary_relative(cur_path_names, orig_path_names, old_dir): modify_macho_object(cur_path, rpaths, deps, idpath, new_rpaths, new_deps, new_idpath) + if (not allow_root and + strings_contains_installroot(cur_path)): + raise InstallRootStringException(cur_path) 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) @@ -303,10 +403,46 @@ def make_binary_relative(cur_path_names, orig_path_names, old_dir): new_rpaths = get_relative_rpaths(orig_path, old_dir, orig_rpaths) modify_elf_object(cur_path, new_rpaths) + if (not allow_root and + strings_contains_installroot(cur_path, old_dir)): + raise InstallRootStringException(cur_path, old_dir) else: tty.die("Prelocation not implemented for %s" % platform.system()) +def make_binary_placeholder(cur_path_names, allow_root): + """ + Replace old install root in RPATHs with placeholder in binary files + """ + if platform.system() == 'Darwin': + for cur_path in cur_path_names: + rpaths, deps, idpath = macho_get_paths(cur_path) + (new_rpaths, + new_deps, + new_idpath) = macho_make_paths_placeholder(rpaths, deps, idpath) + modify_macho_object(cur_path, + rpaths, deps, idpath, + new_rpaths, new_deps, new_idpath) + if (not allow_root and + strings_contains_installroot(cur_path, + spack.store.layout.root)): + raise InstallRootStringException(cur_path, + spack.store.layout.root) + elif platform.system() == 'Linux': + for cur_path in cur_path_names: + orig_rpaths = get_existing_elf_rpaths(cur_path) + if orig_rpaths: + new_rpaths = get_placeholder_rpaths(cur_path, orig_rpaths) + modify_elf_object(cur_path, new_rpaths) + if (not allow_root and + strings_contains_installroot(cur_path, + spack.store.layout.root)): + raise InstallRootStringException(cur_path, + spack.store.layout.root) + else: + tty.die("Placeholder not implemented for %s" % platform.system()) + + def relocate_text(path_names, old_dir, new_dir): """ Replace old path with new path in text file path_name diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index df45d96189..e3095ee19d 100644 --- a/lib/spack/spack/test/packaging.py +++ b/lib/spack/spack/test/packaging.py @@ -73,7 +73,7 @@ def fake_fetchify(url, pkg): @pytest.mark.usefixtures('install_mockery', 'testing_gpg_directory') -def test_packaging(mock_archive, tmpdir): +def test_buildcache(mock_archive, tmpdir): # tweak patchelf to only do a download spec = Spec("patchelf") spec.concretize() @@ -218,22 +218,20 @@ echo $PATH""" def test_relocate_text(tmpdir): - # Validate the text path replacement - old_dir = '/home/spack/opt/spack' - filename = str(tmpdir) + '/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) - - assert(strings_contains_installroot(filename) is False) - - with open(filename, "r") as script: - for line in script: - assert(new_dir in line) + with tmpdir.as_cwd(): + # 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) + assert(strings_contains_installroot(filename, old_dir) is False) def test_needs_relocation(): -- cgit v1.2.3-70-g09d2