From 7c0c31361a37332f69ea19e47f1474a1f6f1cc77 Mon Sep 17 00:00:00 2001 From: Patrick Gartung Date: Wed, 18 Sep 2019 07:24:45 -0500 Subject: Update buildcache creation and installation to allow mach-o binary relocation using py-machotools on linux or macos. (#12858) Update py-machotools dependencies and versions. --- lib/spack/spack/binary_distribution.py | 52 ++-- lib/spack/spack/relocate.py | 282 +++++++++++++-------- lib/spack/spack/test/packaging.py | 4 +- .../repos/builtin/packages/py-altgraph/package.py | 21 ++ .../repos/builtin/packages/py-macholib/package.py | 5 +- 5 files changed, 224 insertions(+), 140 deletions(-) create mode 100644 var/spack/repos/builtin/packages/py-altgraph/package.py diff --git a/lib/spack/spack/binary_distribution.py b/lib/spack/spack/binary_distribution.py index 8dcacb6df3..564aa3a535 100644 --- a/lib/spack/spack/binary_distribution.py +++ b/lib/spack/spack/binary_distribution.py @@ -154,18 +154,12 @@ def write_buildinfo_file(prefix, workdir, rel=False): 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 - elif relocate.strings_contains_installroot( - path_name, - spack.store.layout.root): - 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) - elif relocate.needs_text_relocation(m_type, m_subtype): - rel_path_name = os.path.relpath(path_name, prefix) - text_to_relocate.append(rel_path_name) + 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 relocate.needs_text_relocation(m_type, m_subtype): + rel_path_name = os.path.relpath(path_name, prefix) + text_to_relocate.append(rel_path_name) # Create buildinfo data and write it to disk buildinfo = {} @@ -325,14 +319,14 @@ def build_tarball(spec, outdir, force=False, rel=False, unsigned=False, # in the spack install tree before creating tarball if rel: try: - make_package_relative(workdir, spec.prefix, allow_root) + make_package_relative(workdir, spec, allow_root) except Exception as e: shutil.rmtree(workdir) shutil.rmtree(tarfile_dir) tty.die(e) else: try: - make_package_placeholder(workdir, spec.prefix, allow_root) + make_package_placeholder(workdir, spec, allow_root) except Exception as e: shutil.rmtree(workdir) shutil.rmtree(tarfile_dir) @@ -416,11 +410,12 @@ def download_tarball(spec): return None -def make_package_relative(workdir, prefix, allow_root): +def make_package_relative(workdir, spec, allow_root): """ Change paths in binaries to relative paths. Change absolute symlinks to relative symlinks. """ + prefix = spec.prefix buildinfo = read_buildinfo_file(workdir) old_path = buildinfo['buildpath'] orig_path_names = list() @@ -428,8 +423,12 @@ def make_package_relative(workdir, prefix, allow_root): 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, allow_root) + if spec.architecture.platform == 'darwin': + relocate.make_macho_binary_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) orig_path_names = list() cur_path_names = list() for filename in buildinfo.get('relocate_links', []): @@ -438,15 +437,17 @@ def make_package_relative(workdir, prefix, allow_root): relocate.make_link_relative(cur_path_names, orig_path_names) -def make_package_placeholder(workdir, prefix, allow_root): +def make_package_placeholder(workdir, spec, allow_root): """ - Change paths in binaries to placeholder paths + Check if package binaries are relocatable + Change links to placeholder links """ + prefix = spec.prefix 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) + relocate.check_files_relocatable(cur_path_names, allow_root) cur_path_names = list() for filename in buildinfo.get('relocate_links', []): @@ -454,7 +455,7 @@ def make_package_placeholder(workdir, prefix, allow_root): relocate.make_link_placeholder(cur_path_names, workdir, prefix) -def relocate_package(workdir, allow_root): +def relocate_package(workdir, spec, allow_root): """ Relocate the given package """ @@ -485,7 +486,12 @@ def relocate_package(workdir, allow_root): for filename in buildinfo['relocate_binaries']: path_name = os.path.join(workdir, filename) path_names.add(path_name) - relocate.relocate_binary(path_names, old_path, new_path, allow_root) + if spec.architecture.platform == 'darwin': + 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) path_names = set() for filename in buildinfo.get('relocate_links', []): path_name = os.path.join(workdir, filename) @@ -572,7 +578,7 @@ def extract_tarball(spec, filename, allow_root=False, unsigned=False, os.remove(specfile_path) try: - relocate_package(workdir, allow_root) + relocate_package(workdir, spec, allow_root) except Exception as e: shutil.rmtree(workdir) tty.die(e) diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 2a41682147..f396837708 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -5,8 +5,8 @@ import os -import platform import re +import platform import spack.repo import spack.cmd import llnl.util.lang @@ -41,6 +41,24 @@ class BinaryStringReplacementException(spack.error.SpackError): (file_path, old_len, new_len)) +class MissingMachotoolsException(spack.error.SpackError): + """ + Raised when the size of the file changes after binary path substitution. + """ + + def __init__(self, error): + super(MissingMachotoolsException, self).__init__( + "%s\n" + "Python package machotools needs to be avaiable to list\n" + "and modify a mach-o binary's rpaths, deps and id.\n" + "Use virtualenv with pip install machotools or\n" + "use spack to install the py-machotools package\n" + "spack install py-machotools\n" + "spack activate py-machotools\n" + "spack load python\n" + % error) + + def get_patchelf(): """ Builds and installs spack patchelf package on linux platforms @@ -48,8 +66,6 @@ def get_patchelf(): Returns the full patchelf binary path. """ # as we may need patchelf, find out where it is - if platform.system() == 'Darwin': - return None patchelf = spack.util.executable.which('patchelf') if patchelf is None: patchelf_spec = spack.cmd.parse_specs("patchelf", concretize=True)[0] @@ -67,18 +83,15 @@ def get_existing_elf_rpaths(path_name): Return the RPATHS returned by patchelf --print-rpath path_name as a list of strings. """ - if platform.system() == 'Linux': - patchelf = Executable(get_patchelf()) - try: - output = patchelf('--print-rpath', '%s' % - path_name, output=str, error=str) - return output.rstrip('\n').split(':') - except ProcessError as e: - tty.debug('patchelf --print-rpath produced an error on %s' % - path_name, e) - return [] - else: - tty.die('relocation not supported for this platform') + patchelf = Executable(get_patchelf()) + try: + output = patchelf('--print-rpath', '%s' % + path_name, output=str, error=str) + return output.rstrip('\n').split(':') + except ProcessError as e: + tty.debug('patchelf --print-rpath produced an error on %s' % + path_name, e) + return [] return @@ -256,6 +269,55 @@ def modify_macho_object(cur_path, rpaths, deps, idpath, return +def modify_object_machotools(cur_path, rpaths, deps, idpath, + new_rpaths, new_deps, new_idpath): + """ + Modify MachO binary path_name by replacing old_dir with new_dir + or the relative path to spack install root. + The old install dir in LC_ID_DYLIB is replaced with the new install dir + using py-machotools + The old install dir in LC_LOAD_DYLIB is replaced with the new install dir + using py-machotools + The old install dir in LC_RPATH is replaced with the new install dir using + using py-machotools + """ + 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 + 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) + rewriter.commit() + return + + +def machotools_get_paths(path_name): + """ + Examines the output of otool -l path_name for these three fields: + LC_ID_DYLIB, LC_LOAD_DYLIB, LC_RPATH and parses out the rpaths, + dependiencies and library id. + Returns these values. + """ + try: + import machotools + except ImportError as e: + raise MissingMachotoolsException(e) + idpath = None + rpaths = list() + deps = list() + rewriter = machotools.rewriter_factory(path_name) + if machotools.detect.is_dylib(path_name): + idpath = rewriter.install_name + rpaths = rewriter.rpaths + deps = rewriter.dependencies + return rpaths, deps, idpath + + def strings_contains_installroot(path_name, root_dir): """ Check if the file contain the install root string. @@ -270,18 +332,15 @@ def modify_elf_object(path_name, new_rpaths): """ Replace orig_rpath with new_rpath in RPATH of elf object path_name """ - if platform.system() == 'Linux': - new_joined = ':'.join(new_rpaths) - patchelf = Executable(get_patchelf()) - try: - patchelf('--force-rpath', '--set-rpath', '%s' % new_joined, - '%s' % path_name, output=str, error=str) - except ProcessError as e: - tty.die('patchelf --set-rpath %s failed' % - path_name, e) - pass - else: - tty.die('relocation not supported for this platform') + new_joined = ':'.join(new_rpaths) + patchelf = Executable(get_patchelf()) + try: + patchelf('--force-rpath', '--set-rpath', '%s' % new_joined, + '%s' % path_name, output=str, error=str) + except ProcessError as e: + tty.die('patchelf --set-rpath %s failed' % + path_name, e) + pass def needs_binary_relocation(m_type, m_subtype): @@ -330,61 +389,65 @@ def replace_prefix_bin(path_name, old_dir, new_dir): f.truncate() -def relocate_binary(path_names, old_dir, new_dir, allow_root): +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 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) + for path_name in path_names: + (rpaths, deps, idpath) = machotools_get_paths(path_name) + # one pass to replace placeholder + (n_rpaths, + n_deps, + n_idpath) = macho_replace_paths(placeholder, + new_dir, + rpaths, + deps, + idpath) + # another pass to replace old_dir + (new_rpaths, + new_deps, + new_idpath) = macho_replace_paths(old_dir, + new_dir, + 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) + 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): + """ + Change old_dir to new_dir in RPATHs of elf binaries + Account for the case where old_dir is now a placeholder + """ + placeholder = set_placeholder(old_dir) + for path_name in path_names: + orig_rpaths = get_existing_elf_rpaths(path_name) + if orig_rpaths: # one pass to replace placeholder - (n_rpaths, - n_deps, - n_idpath) = macho_replace_paths(placeholder, - new_dir, - rpaths, - deps, - idpath) - # another pass to replace old_dir - (new_rpaths, - new_deps, - new_idpath) = macho_replace_paths(old_dir, - new_dir, - n_rpaths, - n_deps, - n_idpath) - modify_macho_object(path_name, - rpaths, deps, idpath, - new_rpaths, new_deps, new_idpath) + n_rpaths = substitute_rpath(orig_rpaths, + placeholder, new_dir) + # one pass to replace old_dir + 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' % + ' because %s is longer than %s.' % (path_name, new_dir, old_dir)) - elif platform.system() == 'Linux': - for path_name in path_names: - orig_rpaths = get_existing_elf_rpaths(path_name) - if orig_rpaths: - # one pass to replace placeholder - n_rpaths = substitute_rpath(orig_rpaths, - placeholder, new_dir) - # one pass to replace old_dir - 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)) - else: - tty.die("Relocation not implemented for %s" % platform.system()) def make_link_relative(cur_path_names, orig_path_names): @@ -399,55 +462,50 @@ def make_link_relative(cur_path_names, orig_path_names): os.symlink(new_src, cur_path) -def make_binary_relative(cur_path_names, orig_path_names, old_dir, allow_root): +def make_macho_binary_relative(cur_path_names, orig_path_names, old_dir, + allow_root): """ 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): - rpaths, deps, idpath = macho_get_paths(cur_path) - (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) - if (not allow_root and - not file_is_relocatable(cur_path, old_dir)): - raise InstallRootStringException(cur_path, old_dir) - 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) - if orig_rpaths: - new_rpaths = get_relative_rpaths(orig_path, old_dir, - orig_rpaths) - modify_elf_object(cur_path, new_rpaths) - if (not allow_root and - not file_is_relocatable(cur_path, old_dir)): - raise InstallRootStringException(cur_path, old_dir) - else: - tty.die("Prelocation not implemented for %s" % platform.system()) + for cur_path, orig_path in zip(cur_path_names, orig_path_names): + 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 (not allow_root and + not file_is_relocatable(cur_path, old_dir)): + raise InstallRootStringException(cur_path, old_dir) + + +def make_elf_binary_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): + orig_rpaths = get_existing_elf_rpaths(cur_path) + if orig_rpaths: + new_rpaths = get_relative_rpaths(orig_path, old_dir, + orig_rpaths) + modify_elf_object(cur_path, new_rpaths) + if (not allow_root and + not file_is_relocatable(cur_path, old_dir)): + raise InstallRootStringException(cur_path, old_dir) -def make_binary_placeholder(cur_path_names, allow_root): +def check_files_relocatable(cur_path_names, allow_root): """ - Replace old install root in RPATHs with placeholder in binary files + Check binary files for the current install root """ - if platform.system() == 'Darwin': - for cur_path in cur_path_names: - if (not allow_root and - not file_is_relocatable(cur_path)): - raise InstallRootStringException( - cur_path, spack.store.layout.root) - elif platform.system() == 'Linux': - for cur_path in cur_path_names: - if (not allow_root and - not file_is_relocatable(cur_path)): - raise InstallRootStringException( - cur_path, spack.store.layout.root) - else: - tty.die("Placeholder not implemented for %s" % platform.system()) + for cur_path in cur_path_names: + if (not allow_root and + not file_is_relocatable(cur_path)): + raise InstallRootStringException( + cur_path, spack.store.layout.root) def make_link_placeholder(cur_path_names, cur_dir, old_dir): diff --git a/lib/spack/spack/test/packaging.py b/lib/spack/spack/test/packaging.py index e865cac32e..cff60e3127 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 get_patchelf, relocate_text, relocate_links +from spack.relocate import 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,8 +364,6 @@ 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) diff --git a/var/spack/repos/builtin/packages/py-altgraph/package.py b/var/spack/repos/builtin/packages/py-altgraph/package.py new file mode 100644 index 0000000000..db27953995 --- /dev/null +++ b/var/spack/repos/builtin/packages/py-altgraph/package.py @@ -0,0 +1,21 @@ +# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +from spack import * + + +class PyAltgraph(PythonPackage): + """ + altgraph is a fork of graphlib: a graph (network) + package for constructing graphs, BFS and DFS traversals, + topological sort, shortest paths, etc. with graphviz output. + """ + + homepage = "https://pypi.python.org/pypi/altgraph" + url = "https://pypi.io/packages/source/a/altgraph/altgraph-0.16.1.tar.gz" + + version('0.16.1', "ddf5320017147ba7b810198e0b6619bd7b5563aa034da388cea8546b877f9b0c") + + depends_on('py-setuptools', type='build') diff --git a/var/spack/repos/builtin/packages/py-macholib/package.py b/var/spack/repos/builtin/packages/py-macholib/package.py index 160ebdd5e4..4f730733c8 100644 --- a/var/spack/repos/builtin/packages/py-macholib/package.py +++ b/var/spack/repos/builtin/packages/py-macholib/package.py @@ -12,6 +12,7 @@ class PyMacholib(PythonPackage): homepage = "https://pypi.python.org/pypi/macholib" url = "https://pypi.io/packages/source/m/macholib/macholib-1.8.tar.gz" - version('1.8', '65af8f20dada7bdb2a142afbec51330e') + version('1.11', 'c4180ffc6f909bf8db6cd81cff4b6f601d575568f4d5dee148c830e9851eb9db') - depends_on('py-setuptools', type='build') + depends_on('py-setuptools', type=('build', 'run')) + depends_on('py-altgraph', type=('build', 'run')) -- cgit v1.2.3-70-g09d2