From a940ff34d745e0830f9e40305698bfd85d65fbd3 Mon Sep 17 00:00:00 2001 From: Patrick Gartung Date: Thu, 19 Sep 2019 17:16:26 -0500 Subject: Put back the use of otool and install_name_tool when running on macOS. Only use machotools on linux. (#12867) Move verbose messages to debug level get_patchelf should return None for test platform as well because create_buildinfo invokes patchelf to get rpaths. --- lib/spack/spack/binary_distribution.py | 27 ++++---- lib/spack/spack/cmd/buildcache.py | 30 +++++---- lib/spack/spack/relocate.py | 119 +++++++++++++++++++++------------ lib/spack/spack/test/packaging.py | 4 +- 4 files changed, 112 insertions(+), 68 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 564aa3a535..40e3a7c17a 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -29,7 +29,6 @@ from spack.util.gpg import Gpg from spack.util.web import spider, read_from_url from spack.util.executable import ProcessError - _build_cache_relative_path = 'build_cache' @@ -155,8 +154,9 @@ def write_buildinfo_file(prefix, workdir, rel=False): tty.warn(msg) if relocate.needs_binary_relocation(m_type, m_subtype): - rel_path_name = os.path.relpath(path_name, prefix) - binary_to_relocate.append(rel_path_name) + 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): rel_path_name = os.path.relpath(path_name, prefix) text_to_relocate.append(rel_path_name) @@ -308,6 +308,7 @@ def build_tarball(spec, outdir, force=False, rel=False, unsigned=False, os.remove(specfile_path) else: 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)) install_tree(spec.prefix, workdir, symlinks=True) @@ -424,11 +425,11 @@ def make_package_relative(workdir, spec, allow_root): orig_path_names.append(os.path.join(prefix, filename)) cur_path_names.append(os.path.join(workdir, filename)) if spec.architecture.platform == 'darwin': - relocate.make_macho_binary_relative(cur_path_names, orig_path_names, - old_path, allow_root) + relocate.make_macho_binaries_relative(cur_path_names, orig_path_names, + old_path, allow_root) else: - relocate.make_elf_binary_relative(cur_path_names, orig_path_names, - old_path, allow_root) + relocate.make_elf_binaries_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', []): @@ -439,8 +440,8 @@ def make_package_relative(workdir, spec, allow_root): def make_package_placeholder(workdir, spec, allow_root): """ - Check if package binaries are relocatable - Change links to placeholder links + Check if package binaries are relocatable. + Change links to placeholder links. """ prefix = spec.prefix buildinfo = read_buildinfo_file(workdir) @@ -487,11 +488,11 @@ def relocate_package(workdir, spec, allow_root): path_name = os.path.join(workdir, filename) path_names.add(path_name) if spec.architecture.platform == 'darwin': - relocate.relocate_macho_binaries(path_names, old_path, new_path, - allow_root) + relocate.relocate_macho_binaries(path_names, old_path, + new_path, allow_root) else: - relocate.relocate_elf_binaries(path_names, old_path, new_path, - allow_root) + relocate.relocate_elf_binaries(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) diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py index b3d73053e4..5d5e693319 100644 --- a/lib/spack/spack/cmd/buildcache.py +++ b/lib/spack/spack/cmd/buildcache.py @@ -323,34 +323,38 @@ def createtarball(args): matches = find_matching_specs(pkgs, env=env) if matches: - tty.msg('Found at least one matching spec') + tty.debug('Found at least one matching spec') for match in matches: - tty.msg('examining match {0}'.format(match.format())) + tty.debug('examining match {0}'.format(match.format())) if match.external or match.virtual: - tty.msg('skipping external or virtual spec %s' % - match.format()) + tty.debug('skipping external or virtual spec %s' % + match.format()) else: - tty.msg('adding matching spec %s' % match.format()) + tty.debug('adding matching spec %s' % match.format()) specs.add(match) - tty.msg('recursing dependencies') + tty.debug('recursing dependencies') for d, node in match.traverse(order='post', depth=True, deptype=('link', 'run')): if node.external or node.virtual: - tty.msg('skipping external or virtual dependency %s' % - node.format()) + tty.debug('skipping external or virtual dependency %s' % + node.format()) else: - tty.msg('adding dependency %s' % node.format()) + tty.debug('adding dependency %s' % node.format()) specs.add(node) - tty.msg('writing tarballs to %s/build_cache' % outdir) + tty.debug('writing tarballs to %s/build_cache' % outdir) for spec in specs: tty.msg('creating binary cache file for package %s ' % spec.format()) - bindist.build_tarball(spec, outdir, args.force, args.rel, - args.unsigned, args.allow_root, signkey, - not args.no_rebuild_index) + try: + bindist.build_tarball(spec, outdir, args.force, args.rel, + args.unsigned, args.allow_root, signkey, + not args.no_rebuild_index) + except Exception as e: + tty.warn('%s' % e) + pass def installtarball(args): diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index f396837708..01fc913a0e 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -66,6 +66,10 @@ def get_patchelf(): Returns the full patchelf binary path. """ # as we may need patchelf, find out where it is + if str(spack.architecture.platform()) == 'test': + return None + if str(spack.architecture.platform()) == 'darwin': + return None patchelf = spack.util.executable.which('patchelf') if patchelf is None: patchelf_spec = spack.cmd.parse_specs("patchelf", concretize=True)[0] @@ -255,17 +259,19 @@ 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: - args.extend(['-id', new_idpath]) + if new_idpath and not idpath == new_idpath: + install_name_tool('-id', new_idpath, str(cur_path)) - for orig, new in zip(deps, new_deps): - args.extend(['-change', orig, new]) + if len(deps) == len(new_deps): + for orig, new in zip(deps, new_deps): + if not orig == new: + install_name_tool('-change', orig, new, str(cur_path)) + + if len(rpaths) == len(new_rpaths): + for orig, new in zip(rpaths, new_rpaths): + if not orig == new: + install_name_tool('-rpath', orig, new, str(cur_path)) - for orig, new in zip(rpaths, new_rpaths): - args.extend(['-rpath', orig, new]) - args.append(str(cur_path)) - install_name_tool(*args) return @@ -281,17 +287,19 @@ def modify_object_machotools(cur_path, rpaths, deps, idpath, The old install dir in LC_RPATH is replaced with the new install dir using using py-machotools """ + if cur_path.endswith('.o'): + return try: import machotools except ImportError as e: raise MissingMachotoolsException(e) rewriter = machotools.rewriter_factory(cur_path) if machotools.detect.is_dylib(cur_path): - rewriter.install_name = new_idpath + if not new_idpath == idpath: + rewriter.install_name = new_idpath for orig, new in zip(deps, new_deps): - rewriter.change_dependency(orig, new) - for orig, new in zip(rpaths, new_rpaths): - rewriter.append_rpath(new) + if not orig == new: + rewriter.change_dependency(orig, new) rewriter.commit() return @@ -391,12 +399,21 @@ def replace_prefix_bin(path_name, old_dir, new_dir): def relocate_macho_binaries(path_names, old_dir, new_dir, allow_root): """ - Change old_dir to new_dir in RPATHs of elf or mach-o files + Change old_dir to new_dir in LC_RPATH of mach-o files (on macOS) + Change old_dir to new_dir in LC_ID and LC_DEP of mach-o files Account for the case where old_dir is now a placeholder """ placeholder = set_placeholder(old_dir) for path_name in path_names: - (rpaths, deps, idpath) = machotools_get_paths(path_name) + deps = set() + idpath = None + if platform.system().lower() == 'darwin': + if path_name.endswith('.o'): + continue + else: + rpaths, deps, idpath = macho_get_paths(path_name) + else: + rpaths, deps, idpath = machotools_get_paths(path_name) # one pass to replace placeholder (n_rpaths, n_deps, @@ -413,16 +430,23 @@ def relocate_macho_binaries(path_names, old_dir, new_dir, allow_root): n_rpaths, n_deps, n_idpath) - modify_object_machotools(path_name, - rpaths, deps, idpath, - new_rpaths, new_deps, new_idpath) - if len(new_dir) <= len(old_dir): - replace_prefix_bin(path_name, old_dir, new_dir) + if platform.system().lower() == 'darwin': + modify_macho_object(path_name, + rpaths, deps, idpath, + new_rpaths, new_deps, new_idpath) else: - tty.warn('Cannot do a binary string replacement' - ' with padding for %s' - ' because %s is longer than %s' % - (path_name, new_dir, old_dir)) + modify_object_machotools(path_name, + rpaths, deps, idpath, + new_rpaths, new_deps, new_idpath) + + if not new_dir == old_dir: + if len(new_dir) <= len(old_dir): + replace_prefix_bin(path_name, old_dir, new_dir) + else: + tty.warn('Cannot do a binary string replacement' + ' with padding for %s' + ' because %s is longer than %s' % + (path_name, new_dir, old_dir)) def relocate_elf_binaries(path_names, old_dir, new_dir, allow_root): @@ -441,13 +465,14 @@ def relocate_elf_binaries(path_names, old_dir, new_dir, allow_root): new_rpaths = substitute_rpath(n_rpaths, old_dir, new_dir) modify_elf_object(path_name, new_rpaths) - if len(new_dir) <= len(old_dir): - replace_prefix_bin(path_name, old_dir, new_dir) - else: - tty.warn('Cannot do a binary string replacement' - ' with padding for %s' - ' because %s is longer than %s.' % - (path_name, new_dir, old_dir)) + if not new_dir == old_dir: + if len(new_dir) <= len(old_dir): + replace_prefix_bin(path_name, old_dir, new_dir) + else: + tty.warn('Cannot do a binary string replacement' + ' with padding for %s' + ' because %s is longer than %s.' % + (path_name, new_dir, old_dir)) def make_link_relative(cur_path_names, orig_path_names): @@ -462,27 +487,39 @@ def make_link_relative(cur_path_names, orig_path_names): os.symlink(new_src, cur_path) -def make_macho_binary_relative(cur_path_names, orig_path_names, old_dir, - allow_root): +def make_macho_binaries_relative(cur_path_names, orig_path_names, old_dir, + allow_root): """ Replace old RPATHs with paths relative to old_dir in binary files """ for cur_path, orig_path in zip(cur_path_names, orig_path_names): - rpaths, deps, idpath = machotools_get_paths(cur_path) + rpaths = set() + deps = set() + idpath = None + if platform.system().lower() == 'darwin': + (rpaths, deps, idpath) = macho_get_paths(cur_path) + else: + (rpaths, deps, idpath) = machotools_get_paths(cur_path) (new_rpaths, new_deps, new_idpath) = macho_make_paths_relative(orig_path, old_dir, rpaths, deps, idpath) - modify_object_machotools(cur_path, - rpaths, deps, idpath, - new_rpaths, new_deps, new_idpath) + if platform.system().lower() == 'darwin': + modify_macho_object(cur_path, + rpaths, deps, idpath, + new_rpaths, new_deps, new_idpath) + else: + modify_object_machotools(cur_path, + rpaths, deps, idpath, + new_rpaths, new_deps, new_idpath) + if (not allow_root and - not file_is_relocatable(cur_path, old_dir)): + not file_is_relocatable(cur_path)): raise InstallRootStringException(cur_path, old_dir) -def make_elf_binary_relative(cur_path_names, orig_path_names, old_dir, - allow_root): +def make_elf_binaries_relative(cur_path_names, orig_path_names, old_dir, + allow_root): """ Replace old RPATHs with paths relative to old_dir in binary files """ @@ -493,7 +530,7 @@ def make_elf_binary_relative(cur_path_names, orig_path_names, old_dir, orig_rpaths) modify_elf_object(cur_path, new_rpaths) if (not allow_root and - not file_is_relocatable(cur_path, old_dir)): + not file_is_relocatable(cur_path)): raise InstallRootStringException(cur_path, old_dir) diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index cff60e3127..e865cac32e 100644 --- a/lib/spack/spack/test/packaging.py +++ b/lib/spack/spack/test/packaging.py @@ -25,7 +25,7 @@ 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 strings_contains_installroot -from spack.relocate import relocate_text, relocate_links +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 @@ -364,6 +364,8 @@ def test_elf_paths(): def test_relocate_macho(tmpdir): with tmpdir.as_cwd(): + 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', rpaths, deps, idpath) -- cgit v1.2.3-70-g09d2