From e3af8ed4546df3438cfcb6fd581ae07660ec5b64 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 28 Feb 2019 22:36:47 +0100 Subject: [PATCH] Added a sub-command to show if packages are relocatable (#9199) * Added the `spack buildcache preview` sub-command This is similar to `spack spec -I` but highlights which nodes in a DAG are relocatable and which are not. spec.tree has been generalized a little to accept a status function, instead of always showing the install status The current implementation works only for ELF, and needs to be generalized to other platforms. * Added a test to check if an executable is relocatable or not This test requires a few commands to be present in the environment. Currently it will run only under python 3.7 (which uses Xenial instead of Trusty). * Added tests for the 'buildcache preview' command. * Fixed codebase after rebase * Fixed the list of apt addons for Python 3.7 in travis.yaml * Only check ELF executables and shared libraries. Skip checking virtual or external packages. (#229) * Fixed flake8 issues * Add handling for macOS mach binaries (#231) --- .travis.yml | 18 +++ lib/spack/llnl/util/lang.py | 2 +- lib/spack/spack/cmd/buildcache.py | 40 ++++- lib/spack/spack/cmd/spec.py | 14 +- lib/spack/spack/environment.py | 6 +- lib/spack/spack/relocate.py | 138 +++++++++++++++++- lib/spack/spack/spec.py | 8 +- lib/spack/spack/test/cmd/buildcache.py | 21 +++ .../test/data/templates/non_relocatable.c | 5 + .../spack/test/data/templates/relocatable.c | 5 + lib/spack/spack/test/relocate.py | 95 ++++++++++++ 11 files changed, 327 insertions(+), 25 deletions(-) create mode 100644 lib/spack/spack/test/cmd/buildcache.py create mode 100644 lib/spack/spack/test/data/templates/non_relocatable.c create mode 100644 lib/spack/spack/test/data/templates/relocatable.c create mode 100644 lib/spack/spack/test/relocate.py diff --git a/.travis.yml b/.travis.yml index bc3639fca0..d8f955a332 100644 --- a/.travis.yml +++ b/.travis.yml @@ -55,6 +55,24 @@ jobs: os: linux language: python env: TEST_SUITE=unit + addons: + apt: + packages: + - cmake + - gfortran + - graphviz + - gnupg2 + - kcov + - mercurial + - ninja-build + - perl + - perl-base + - realpath + - patchelf + - r-base + - r-base-core + - r-base-dev + - python: '3.6' sudo: required os: linux diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py index 03783265dd..21eb9ee3ba 100644 --- a/lib/spack/llnl/util/lang.py +++ b/lib/spack/llnl/util/lang.py @@ -324,7 +324,7 @@ def foo(self, **kwargs): if kwargs: raise TypeError( "'%s' is an invalid keyword argument for function %s()." - % (next(kwargs.iterkeys()), fun.__name__)) + % (next(iter(kwargs)), fun.__name__)) def match_predicate(*args): diff --git a/lib/spack/spack/cmd/buildcache.py b/lib/spack/spack/cmd/buildcache.py index fe91312c42..9824f96cb4 100644 --- a/lib/spack/spack/cmd/buildcache.py +++ b/lib/spack/spack/cmd/buildcache.py @@ -8,9 +8,15 @@ import sys import llnl.util.tty as tty - +import spack.binary_distribution as bindist import spack.cmd +import spack.cmd.common.arguments as arguments import spack.environment as ev +import spack.relocate +import spack.repo +import spack.spec +import spack.store + from spack.error import SpecError import spack.config import spack.repo @@ -19,8 +25,6 @@ from spack.spec import Spec, save_dependency_spec_yamls from spack.spec_set import CombinatorialSpecSet -import spack.binary_distribution as bindist -import spack.cmd.common.arguments as arguments from spack.cmd import display_specs description = "create, download and install binary packages" @@ -100,6 +104,15 @@ def setup_parser(subparser): help="force new download of keys") dlkeys.set_defaults(func=getkeys) + preview_parser = subparsers.add_parser( + 'preview', + help='analyzes an installed spec and reports whether ' + 'executables and libraries are relocatable' + ) + preview_parser.add_argument( + 'packages', nargs='+', help='list of installed packages' + ) + preview_parser.set_defaults(func=preview) # Check if binaries need to be rebuilt on remote mirror check = subparsers.add_parser('check', help=check_binaries.__doc__) check.add_argument( @@ -176,8 +189,7 @@ def setup_parser(subparser): saveyaml.set_defaults(func=save_spec_yamls) -def find_matching_specs( - pkgs, allow_multiple_matches=False, force=False, env=None): +def find_matching_specs(pkgs, allow_multiple_matches=False, env=None): """Returns a list of specs matching the not necessarily concretized specs given from cli @@ -293,7 +305,7 @@ def createtarball(args): # restrict matching to current environment if one is active env = ev.get_env(args, 'buildcache create') - matches = find_matching_specs(pkgs, False, False, env=env) + matches = find_matching_specs(pkgs, env=env) if matches: tty.msg('Found at least one matching spec') @@ -378,6 +390,22 @@ def getkeys(args): bindist.get_keys(args.install, args.trust, args.force) +def preview(args): + """Print a status tree of the selected specs that shows which nodes are + relocatable and which might not be. + + Args: + args: command line arguments + """ + specs = find_matching_specs(args.packages, allow_multiple_matches=True) + + # Cycle over the specs that match + for spec in specs: + print("Relocatable nodes") + print("--------------------------------") + print(spec.tree(status_fn=spack.relocate.is_relocatable)) + + def check_binaries(args): """Check specs (either a single spec from --spec, or else the full set of release specs) against remote binary mirror(s) to see if any need diff --git a/lib/spack/spack/cmd/spec.py b/lib/spack/spack/cmd/spec.py index 64510df23e..f60e5f1283 100644 --- a/lib/spack/spack/cmd/spec.py +++ b/lib/spack/spack/cmd/spec.py @@ -13,6 +13,7 @@ import spack import spack.cmd import spack.cmd.common.arguments as arguments +import spack.spec description = "show what would be installed, given a spec" section = "build" @@ -42,11 +43,14 @@ def setup_parser(subparser): def spec(parser, args): name_fmt = '$.' if args.namespaces else '$_' - kwargs = {'cover': args.cover, - 'format': name_fmt + '$@$%@+$+$=', - 'hashlen': None if args.very_long else 7, - 'show_types': args.types, - 'install_status': args.install_status} + install_status_fn = spack.spec.Spec.install_status + kwargs = { + 'cover': args.cover, + 'format': name_fmt + '$@$%@+$+$=', + 'hashlen': None if args.very_long else 7, + 'show_types': args.types, + 'status_fn': install_status_fn if args.install_status else None + } if not args.specs: tty.die("spack spec requires at least one spec") diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 57a7f8c454..e9e3328bf6 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -595,8 +595,10 @@ def concretize(self, force=False): # Display concretized spec to the user sys.stdout.write(concrete.tree( - recurse_dependencies=True, install_status=True, - hashlen=7, hashes=True)) + recurse_dependencies=True, + status_fn=spack.spec.Spec.install_status, + hashlen=7, hashes=True) + ) def install(self, user_spec, concrete_spec=None, **install_args): """Install a single spec into an environment. diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index 2d3bc6f837..6e508c1881 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -9,8 +9,9 @@ import re import spack.repo import spack.cmd +import llnl.util.lang +import llnl.util.filesystem as fs from spack.util.executable import Executable, ProcessError -from llnl.util.filesystem import filter_file import llnl.util.tty as tty @@ -327,7 +328,7 @@ def relocate_binary(path_names, old_dir, new_dir, allow_root): if (not allow_root and old_dir != new_dir and strings_contains_installroot(path_name, old_dir)): - raise InstallRootStringException(path_name, old_dir) + raise InstallRootStringException(path_name, old_dir) elif platform.system() == 'Linux': for path_name in path_names: @@ -346,7 +347,7 @@ def relocate_binary(path_names, old_dir, new_dir, allow_root): if (not allow_root and old_dir != new_dir and strings_contains_installroot(path_name, old_dir)): - raise InstallRootStringException(path_name, old_dir) + raise InstallRootStringException(path_name, old_dir) else: tty.die("Relocation not implemented for %s" % platform.system()) @@ -379,7 +380,7 @@ def make_binary_relative(cur_path_names, orig_path_names, old_dir, allow_root): new_rpaths, new_deps, new_idpath) if (not allow_root and strings_contains_installroot(cur_path)): - raise InstallRootStringException(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) @@ -389,7 +390,7 @@ def make_binary_relative(cur_path_names, orig_path_names, old_dir, allow_root): 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) + raise InstallRootStringException(cur_path, old_dir) else: tty.die("Prelocation not implemented for %s" % platform.system()) @@ -466,8 +467,7 @@ def relocate_text(path_names, old_dir, new_dir): """ Replace old path with new path in text file path_name """ - filter_file('%s' % old_dir, '%s' % new_dir, - *path_names, backup=False) + fs.filter_file('%s' % old_dir, '%s' % new_dir, *path_names, backup=False) def substitute_rpath(orig_rpath, topdir, new_root_path): @@ -479,3 +479,127 @@ def substitute_rpath(orig_rpath, topdir, new_root_path): new_rpath = path.replace(topdir, new_root_path) new_rpaths.append(new_rpath) return new_rpaths + + +def is_relocatable(spec): + """Returns True if an installed spec is relocatable. + + Args: + spec (Spec): spec to be analyzed + + Returns: + True if the binaries of an installed spec + are relocatable and False otherwise. + + Raises: + ValueError: if the spec is not installed + """ + if not spec.install_status(): + raise ValueError('spec is not installed [{0}]'.format(str(spec))) + + if spec.external or spec.virtual: + return False + + # Explore the installation prefix of the spec + for root, dirs, files in os.walk(spec.prefix, topdown=True): + dirs[:] = [d for d in dirs if d not in ('.spack', 'man')] + abs_files = [os.path.join(root, f) for f in files] + if not all(file_is_relocatable(f) for f in abs_files if is_binary(f)): + # If any of the file is not relocatable, the entire + # package is not relocatable + return False + + return True + + +def file_is_relocatable(file): + """Returns True if the file passed as argument is relocatable. + + Args: + file: absolute path of the file to be analyzed + + Returns: + True or false + + Raises: + + ValueError: if the file does not exist or the path is not absolute + """ + + if not (platform.system().lower() == 'darwin' + or platform.system().lower() == 'linux'): + msg = 'function currently implemented only for linux and macOS' + raise NotImplementedError(msg) + + if not os.path.exists(file): + raise ValueError('{0} does not exist'.format(file)) + + if not os.path.isabs(file): + raise ValueError('{0} is not an absolute path'.format(file)) + + strings = Executable('strings') + patchelf = Executable('patchelf') + + # Remove the RPATHS from the strings in the executable + set_of_strings = set(strings(file, output=str).split()) + + m_type, m_subtype = mime_type(file) + if m_type == 'application': + tty.debug('{0},{1}'.format(m_type, m_subtype)) + + if platform.system().lower() == 'linux': + if m_subtype == 'x-executable' or m_subtype == 'x-sharedlib': + rpaths = patchelf('--print-rpath', file, output=str).strip() + set_of_strings.discard(rpaths.strip()) + if platform.system().lower() == 'darwin': + if m_subtype == 'x-mach-binary': + rpaths, deps, idpath = macho_get_paths(file) + set_of_strings.discard(set(rpaths)) + set_of_strings.discard(set(deps)) + if idpath is not None: + set_of_strings.discard(idpath) + + if any(spack.store.layout.root in x for x in set_of_strings): + # One binary has the root folder not in the RPATH, + # meaning that this spec is not relocatable + msg = 'Found "{0}" in {1} strings' + tty.debug(msg.format(spack.store.layout.root, file)) + return False + + return True + + +def is_binary(file): + """Returns true if a file is binary, False otherwise + + Args: + file: file to be tested + + Returns: + True or False + """ + m_type, _ = mime_type(file) + + msg = '[{0}] -> '.format(file) + if m_type == 'application': + tty.debug(msg + 'BINARY FILE') + return True + + tty.debug(msg + 'TEXT FILE') + return False + + +@llnl.util.lang.memoized +def mime_type(file): + """Returns the mime type and subtype of a file. + + Args: + file: file to be analyzed + + Returns: + Tuple containing the MIME type and subtype + """ + file_cmd = Executable('file') + output = file_cmd('-b', '--mime-type', file, output=str, error=str) + tty.debug('[MIME_TYPE] {0} -> {1}'.format(file, output.strip())) + return tuple(output.strip().split('/')) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index d37a6e402f..76c06d3fd7 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -3251,7 +3251,7 @@ def __str__(self): ret = self.format() + self.dep_string() return ret.strip() - def _install_status(self): + def install_status(self): """Helper for tree to print DB install status.""" if not self.concrete: return None @@ -3278,7 +3278,7 @@ def tree(self, **kwargs): depth = kwargs.pop('depth', False) hashes = kwargs.pop('hashes', False) hlen = kwargs.pop('hashlen', None) - install_status = kwargs.pop('install_status', False) + status_fn = kwargs.pop('status_fn', False) cover = kwargs.pop('cover', 'nodes') indent = kwargs.pop('indent', 0) fmt = kwargs.pop('format', '$_$@$%@+$+$=') @@ -3300,8 +3300,8 @@ def tree(self, **kwargs): if depth: out += "%-4d" % d - if install_status: - status = node._install_status() + if status_fn: + status = status_fn(node) if status is None: out += colorize("@K{ - } ", color=color) # not installed elif status: diff --git a/lib/spack/spack/test/cmd/buildcache.py b/lib/spack/spack/test/cmd/buildcache.py new file mode 100644 index 0000000000..329f3501d5 --- /dev/null +++ b/lib/spack/spack/test/cmd/buildcache.py @@ -0,0 +1,21 @@ +# Copyright 2013-2018 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) + +import platform + +import pytest + +import spack.main + + +buildcache = spack.main.SpackCommand('buildcache') + + +@pytest.mark.skipif( + platform.system().lower() != 'linux', + reason='implementation for MacOS still missing' +) +def test_buildcache_preview_just_runs(database): + buildcache('preview', 'mpileaks') diff --git a/lib/spack/spack/test/data/templates/non_relocatable.c b/lib/spack/spack/test/data/templates/non_relocatable.c new file mode 100644 index 0000000000..c2e3724c2c --- /dev/null +++ b/lib/spack/spack/test/data/templates/non_relocatable.c @@ -0,0 +1,5 @@ +#include + +int main(){ + printf("Hello World from {{ prefix }} !"); +} diff --git a/lib/spack/spack/test/data/templates/relocatable.c b/lib/spack/spack/test/data/templates/relocatable.c new file mode 100644 index 0000000000..f3e4959692 --- /dev/null +++ b/lib/spack/spack/test/data/templates/relocatable.c @@ -0,0 +1,5 @@ +#include + +int main(){ + printf("Hello World!"); +} diff --git a/lib/spack/spack/test/relocate.py b/lib/spack/spack/test/relocate.py new file mode 100644 index 0000000000..083686d5e6 --- /dev/null +++ b/lib/spack/spack/test/relocate.py @@ -0,0 +1,95 @@ +# Copyright 2013-2018 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) + +import os.path +import platform +import shutil + +import pytest + +import llnl.util.filesystem +import spack.paths +import spack.relocate +import spack.store +import spack.tengine +import spack.util.executable + + +@pytest.fixture(autouse=True) +def _skip_if_missing_executables(request): + """Permits to mark tests with 'require_executables' and skip the + tests if the executables passed as arguments are not found. + """ + if request.node.get_marker('requires_executables'): + required_execs = request.node.get_marker('requires_executables').args + missings_execs = [ + x for x in required_execs if spack.util.executable.which(x) is None + ] + if missings_execs: + msg = 'could not find executables: {0}' + pytest.skip(msg.format(', '.join(missings_execs))) + + +@pytest.fixture(params=[True, False]) +def is_relocatable(request): + return request.param + + +@pytest.fixture() +def source_file(tmpdir, is_relocatable): + """Returns the path to a source file of a relocatable executable.""" + if is_relocatable: + template_src = os.path.join( + spack.paths.test_path, 'data', 'templates', 'relocatable.c' + ) + src = tmpdir.join('relocatable.c') + shutil.copy(template_src, str(src)) + else: + template_dirs = [ + os.path.join(spack.paths.test_path, 'data', 'templates') + ] + env = spack.tengine.make_environment(template_dirs) + template = env.get_template('non_relocatable.c') + text = template.render({'prefix': spack.store.layout.root}) + + src = tmpdir.join('non_relocatable.c') + src.write(text) + + return src + + +@pytest.mark.requires_executables( + '/usr/bin/gcc', 'patchelf', 'strings', 'file' +) +def test_file_is_relocatable(source_file, is_relocatable): + compiler = spack.util.executable.Executable('/usr/bin/gcc') + executable = str(source_file).replace('.c', '.x') + compiler_env = { + 'PATH': '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin' + } + compiler(str(source_file), '-o', executable, env=compiler_env) + + assert spack.relocate.is_binary(executable) + assert spack.relocate.file_is_relocatable(executable) is is_relocatable + + +@pytest.mark.skipif( + platform.system().lower() != 'linux', + reason='implementation for MacOS still missing' +) +def test_file_is_relocatable_errors(tmpdir): + # The file passed in as argument must exist... + with pytest.raises(ValueError) as exc_info: + spack.relocate.file_is_relocatable('/usr/bin/does_not_exist') + assert 'does not exist' in str(exc_info.value) + + # ...and the argument must be an absolute path to it + file = tmpdir.join('delete.me') + file.write('foo') + + with llnl.util.filesystem.working_dir(str(tmpdir)): + with pytest.raises(ValueError) as exc_info: + spack.relocate.file_is_relocatable('delete.me') + assert 'is not an absolute path' in str(exc_info.value)