From c48b733773efdb54d582a46e81d934adc8cd4c95 Mon Sep 17 00:00:00 2001 From: "Seth R. Johnson" Date: Mon, 18 Oct 2021 13:34:16 -0400 Subject: Make macOS installed libraries more relocatable (#26608) * relocate: call install_name_tool less * zstd: fix race condition Multiple times on my mac, trying to install in parallel led to failures from multiple tasks trying to simultaneously create `$PREFIX/lib`. * PackageMeta: simplify callback flush * Relocate: use spack.platforms instead of platform * Relocate: code improvements * fix zstd * Automatically fix rpaths for packages on macOS * Only change library IDs when the path is already in the rpath This restores the hardcoded library path for GCC. * Delete nonexistent rpaths and add more testing * Relocate: Allow @executable_path and @loader_path --- lib/spack/spack/build_systems/autotools.py | 3 + lib/spack/spack/build_systems/makefile.py | 3 + lib/spack/spack/filesystem_view.py | 4 +- lib/spack/spack/package.py | 42 ++-- lib/spack/spack/relocate.py | 292 +++++++++++++++++------ lib/spack/spack/test/relocate.py | 160 ++++++++++--- var/spack/repos/builtin/packages/zstd/package.py | 6 +- 7 files changed, 395 insertions(+), 115 deletions(-) diff --git a/lib/spack/spack/build_systems/autotools.py b/lib/spack/spack/build_systems/autotools.py index f6ac0b3821..f19d5fa54b 100644 --- a/lib/spack/spack/build_systems/autotools.py +++ b/lib/spack/spack/build_systems/autotools.py @@ -645,3 +645,6 @@ To resolve this problem, please try the following: fs.mkdirp(os.path.dirname(self._removed_la_files_log)) with open(self._removed_la_files_log, mode='w') as f: f.write('\n'.join(libtool_files)) + + # On macOS, force rpaths for shared library IDs and remove duplicate rpaths + run_after('install')(PackageBase.apply_macos_rpath_fixups) diff --git a/lib/spack/spack/build_systems/makefile.py b/lib/spack/spack/build_systems/makefile.py index 45c1432dd4..ef046d5782 100644 --- a/lib/spack/spack/build_systems/makefile.py +++ b/lib/spack/spack/build_systems/makefile.py @@ -110,3 +110,6 @@ class MakefilePackage(PackageBase): # Check that self.prefix is there after installation run_after('install')(PackageBase.sanity_check_prefix) + + # On macOS, force rpaths for shared library IDs and remove duplicate rpaths + run_after('install')(PackageBase.apply_macos_rpath_fixups) diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index 26457c0f60..27a8708596 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/lib/spack/spack/filesystem_view.py @@ -19,7 +19,6 @@ from llnl.util.tty.color import colorize import spack.config import spack.projections -import spack.relocate import spack.schema.projections import spack.spec import spack.store @@ -74,6 +73,9 @@ def view_copy(src, dst, view, spec=None): # TODO: Not sure which one to use... import spack.hooks.sbang as sbang + # Break a package include cycle + import spack.relocate + orig_sbang = '#!/bin/bash {0}/bin/sbang'.format(spack.paths.spack_root) new_sbang = sbang.sbang_shebang_line() diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 0e9e07f52e..4fbf24c56e 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -286,34 +286,27 @@ class PackageMeta( def _flush_callbacks(check_name): # Name of the attribute I am going to check it exists - attr_name = PackageMeta.phase_fmt.format(check_name) - checks = getattr(cls, attr_name) + check_attr = PackageMeta.phase_fmt.format(check_name) + checks = getattr(cls, check_attr) if checks: for phase_name, funcs in checks.items(): + phase_attr = PackageMeta.phase_fmt.format(phase_name) try: # Search for the phase in the attribute dictionary - phase = attr_dict[ - PackageMeta.phase_fmt.format(phase_name)] + phase = attr_dict[phase_attr] except KeyError: # If it is not there it's in the bases # and we added a check. We need to copy # and extend for base in bases: - phase = getattr( - base, - PackageMeta.phase_fmt.format(phase_name), - None - ) + phase = getattr(base, phase_attr, None) if phase is not None: break - attr_dict[PackageMeta.phase_fmt.format( - phase_name)] = phase.copy() - phase = attr_dict[ - PackageMeta.phase_fmt.format(phase_name)] + phase = attr_dict[phase_attr] = phase.copy() getattr(phase, check_name).extend(funcs) # Clear the attribute for the next class - setattr(cls, attr_name, {}) + setattr(cls, check_attr, {}) _flush_callbacks('run_before') _flush_callbacks('run_after') @@ -1962,6 +1955,25 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)): raise InstallError( "Install failed for %s. Nothing was installed!" % self.name) + def apply_macos_rpath_fixups(self): + """On Darwin, make installed libraries more easily relocatable. + + Some build systems (handrolled, autotools, makefiles) can set their own + rpaths that are duplicated by spack's compiler wrapper. Additionally, + many simpler build systems do not link using ``-install_name + @rpath/foo.dylib``, which propagates the library's hardcoded + absolute path into downstream dependencies. This fixup interrogates, + and postprocesses if necessary, all libraries installed by the code. + + It should be added as a @run_after to packaging systems (or individual + packages) that do not install relocatable libraries by default. + """ + if 'platform=darwin' not in self.spec: + return + + from spack.relocate import fixup_macos_rpaths + fixup_macos_rpaths(self.spec) + @property def build_log_path(self): """ @@ -2702,6 +2714,8 @@ class Package(PackageBase): # This will be used as a registration decorator in user # packages, if need be run_after('install')(PackageBase.sanity_check_prefix) + # On macOS, force rpaths for shared library IDs and remove duplicate rpaths + run_after('install')(PackageBase.apply_macos_rpath_fixups) def install_dependency_symlinks(pkg, spec, prefix): diff --git a/lib/spack/spack/relocate.py b/lib/spack/spack/relocate.py index bbfc4bd52f..212223e7e0 100644 --- a/lib/spack/spack/relocate.py +++ b/lib/spack/spack/relocate.py @@ -4,9 +4,9 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import multiprocessing.pool import os -import platform import re import shutil +from collections import defaultdict import macholib.mach_o import macholib.MachO @@ -20,6 +20,8 @@ import spack.repo import spack.spec import spack.util.executable as executable +is_macos = (str(spack.platforms.real_host()) == 'darwin') + class InstallRootStringError(spack.error.SpackError): def __init__(self, file_path, root_path): @@ -82,7 +84,7 @@ def _patchelf(): Return None on Darwin or if patchelf cannot be found. """ # Check if patchelf is already in the PATH - patchelf = spack.util.executable.which('patchelf') + patchelf = executable.which('patchelf') if patchelf is not None: return patchelf.path @@ -94,7 +96,7 @@ def _patchelf(): return exe_path # Skip darwin - if str(spack.platforms.host()) == 'darwin': + if is_macos: return None # Install the spec and return its path @@ -197,6 +199,10 @@ def _placeholder(dirname): return '@' * len(dirname) +def _decode_macho_data(bytestring): + return bytestring.rstrip(b'\x00').decode('ascii') + + def macho_make_paths_relative(path_name, old_layout_root, rpaths, deps, idpath): """ @@ -309,21 +315,27 @@ def modify_macho_object(cur_path, rpaths, deps, idpath, # avoid error message for libgcc_s if 'libgcc_' in cur_path: return - install_name_tool = executable.Executable('install_name_tool') + args = [] if idpath: new_idpath = paths_to_paths.get(idpath, None) if new_idpath and not idpath == new_idpath: - install_name_tool('-id', new_idpath, str(cur_path)) + args += ['-id', new_idpath] for dep in deps: new_dep = paths_to_paths.get(dep) if new_dep and dep != new_dep: - install_name_tool('-change', dep, new_dep, str(cur_path)) + args += ['-change', dep, new_dep] for orig_rpath in rpaths: new_rpath = paths_to_paths.get(orig_rpath) if new_rpath and not orig_rpath == new_rpath: - install_name_tool('-rpath', orig_rpath, new_rpath, str(cur_path)) + args += ['-rpath', orig_rpath, new_rpath] + + if args: + args.append(str(cur_path)) + install_name_tool = executable.Executable('install_name_tool') + install_name_tool(*args) + return @@ -339,14 +351,7 @@ def modify_object_macholib(cur_path, paths_to_paths): """ dll = macholib.MachO.MachO(cur_path) - - changedict = paths_to_paths - - def changefunc(path): - npath = changedict.get(path, None) - return npath - - dll.rewriteLoadCommands(changefunc) + dll.rewriteLoadCommands(paths_to_paths.get) try: f = open(dll.filename, 'rb+') @@ -363,30 +368,35 @@ def modify_object_macholib(cur_path, paths_to_paths): def macholib_get_paths(cur_path): + """Get rpaths, dependent libraries, and library id of mach-o objects. """ - Get rpaths, dependencies and id of mach-o objects - using python macholib package - """ - dll = macholib.MachO.MachO(cur_path) + headers = macholib.MachO.MachO(cur_path).headers + if not headers: + tty.warn("Failed to read Mach-O headers: {0}".format(cur_path)) + commands = [] + else: + if len(headers) > 1: + # Reproduce original behavior of only returning the last mach-O + # header section + tty.warn("Encountered fat binary: {0}".format(cur_path)) + commands = headers[-1].commands + + LC_ID_DYLIB = macholib.mach_o.LC_ID_DYLIB + LC_LOAD_DYLIB = macholib.mach_o.LC_LOAD_DYLIB + LC_RPATH = macholib.mach_o.LC_RPATH ident = None - rpaths = list() - deps = list() - for header in dll.headers: - rpaths = [data.rstrip(b'\0').decode('utf-8') - for load_command, dylib_command, data in header.commands if - load_command.cmd == macholib.mach_o.LC_RPATH] - deps = [data.rstrip(b'\0').decode('utf-8') - for load_command, dylib_command, data in header.commands if - load_command.cmd == macholib.mach_o.LC_LOAD_DYLIB] - idents = [data.rstrip(b'\0').decode('utf-8') - for load_command, dylib_command, data in header.commands if - load_command.cmd == macholib.mach_o.LC_ID_DYLIB] - if len(idents) == 1: - ident = idents[0] - tty.debug('ident: %s' % ident) - tty.debug('deps: %s' % deps) - tty.debug('rpaths: %s' % rpaths) + rpaths = [] + deps = [] + for load_command, dylib_command, data in commands: + cmd = load_command.cmd + if cmd == LC_RPATH: + rpaths.append(_decode_macho_data(data)) + elif cmd == LC_LOAD_DYLIB: + deps.append(_decode_macho_data(data)) + elif cmd == LC_ID_DYLIB: + ident = _decode_macho_data(data) + return (rpaths, deps, ident) @@ -539,7 +549,7 @@ def relocate_macho_binaries(path_names, old_layout_root, new_layout_root, rpaths, deps, idpath) # replace the relativized paths with normalized paths - if platform.system().lower() == 'darwin': + if is_macos: modify_macho_object(path_name, rpaths, deps, idpath, rel_to_orig) else: @@ -552,7 +562,7 @@ def relocate_macho_binaries(path_names, old_layout_root, new_layout_root, old_layout_root, prefix_to_prefix) # replace the old paths with new paths - if platform.system().lower() == 'darwin': + if is_macos: modify_macho_object(path_name, rpaths, deps, idpath, paths_to_paths) else: @@ -565,7 +575,7 @@ def relocate_macho_binaries(path_names, old_layout_root, new_layout_root, new_layout_root, rpaths, deps, idpath) # replace the new paths with relativized paths in the new prefix - if platform.system().lower() == 'darwin': + if is_macos: modify_macho_object(path_name, rpaths, deps, idpath, paths_to_paths) else: @@ -579,7 +589,7 @@ def relocate_macho_binaries(path_names, old_layout_root, new_layout_root, old_layout_root, prefix_to_prefix) # replace the old paths with new paths - if platform.system().lower() == 'darwin': + if is_macos: modify_macho_object(path_name, rpaths, deps, idpath, paths_to_paths) else: @@ -695,18 +705,15 @@ def make_macho_binaries_relative(cur_path_names, orig_path_names, """ Replace old RPATHs with paths relative to old_dir in binary files """ + if not is_macos: + return + for cur_path, orig_path in zip(cur_path_names, orig_path_names): - rpaths = set() - deps = set() - idpath = None - if platform.system().lower() == 'darwin': - (rpaths, deps, idpath) = macholib_get_paths(cur_path) - paths_to_paths = macho_make_paths_relative(orig_path, - old_layout_root, - rpaths, deps, idpath) - modify_macho_object(cur_path, - rpaths, deps, idpath, - paths_to_paths) + (rpaths, deps, idpath) = macholib_get_paths(cur_path) + paths_to_paths = macho_make_paths_relative( + orig_path, old_layout_root, rpaths, deps, idpath + ) + modify_macho_object(cur_path, rpaths, deps, idpath, paths_to_paths) def make_elf_binaries_relative(new_binaries, orig_binaries, orig_layout_root): @@ -915,11 +922,6 @@ def file_is_relocatable(filename, paths_to_relocate=None): default_paths_to_relocate = [spack.store.layout.root, spack.paths.prefix] paths_to_relocate = paths_to_relocate or default_paths_to_relocate - 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(filename): raise ValueError('{0} does not exist'.format(filename)) @@ -935,11 +937,11 @@ def file_is_relocatable(filename, paths_to_relocate=None): if m_type == 'application': tty.debug('{0},{1}'.format(m_type, m_subtype)) - if platform.system().lower() == 'linux': + if not is_macos: if m_subtype == 'x-executable' or m_subtype == 'x-sharedlib': rpaths = ':'.join(_elf_rpaths_for(filename)) set_of_strings.discard(rpaths) - if platform.system().lower() == 'darwin': + else: if m_subtype == 'x-mach-binary': rpaths, deps, idpath = macholib_get_paths(filename) set_of_strings.discard(set(rpaths)) @@ -978,6 +980,14 @@ def is_binary(filename): return False +@llnl.util.lang.memoized +def _get_mime_type(): + file_cmd = executable.which('file') + for arg in ['-b', '-h', '--mime-type']: + file_cmd.add_default_arg(arg) + return file_cmd + + @llnl.util.lang.memoized def mime_type(filename): """Returns the mime type and subtype of a file. @@ -988,13 +998,159 @@ def mime_type(filename): Returns: Tuple containing the MIME type and subtype """ - file_cmd = executable.Executable('file') - output = file_cmd( - '-b', '-h', '--mime-type', filename, output=str, error=str) - tty.debug('[MIME_TYPE] {0} -> {1}'.format(filename, output.strip())) - # In corner cases the output does not contain a subtype prefixed with a / - # In those cases add the / so the tuple can be formed. - if '/' not in output: - output += '/' - split_by_slash = output.strip().split('/') - return split_by_slash[0], "/".join(split_by_slash[1:]) + output = _get_mime_type()(filename, output=str, error=str).strip() + tty.debug('==> ' + output) + type, _, subtype = output.partition('/') + return type, subtype + + +# Memoize this due to repeated calls to libraries in the same directory. +@llnl.util.lang.memoized +def _exists_dir(dirname): + return os.path.isdir(dirname) + + +def fixup_macos_rpath(root, filename): + """Apply rpath fixups to the given file. + + Args: + root: absolute path to the parent directory + filename: relative path to the library or binary + + Returns: + True if fixups were applied, else False + """ + abspath = os.path.join(root, filename) + if mime_type(abspath) != ('application', 'x-mach-binary'): + return False + + # Get Mach-O header commands + (rpath_list, deps, id_dylib) = macholib_get_paths(abspath) + + # Convert rpaths list to (name -> number of occurrences) + add_rpaths = set() + del_rpaths = set() + rpaths = defaultdict(int) + for rpath in rpath_list: + rpaths[rpath] += 1 + + args = [] + + # Check dependencies for non-rpath entries + spack_root = spack.store.layout.root + for name in deps: + if name.startswith(spack_root): + tty.debug("Spack-installed dependency for {0}: {1}" + .format(abspath, name)) + (dirname, basename) = os.path.split(name) + if dirname != root or dirname in rpaths: + # Only change the rpath if it's a dependency *or* if the root + # rpath was already added to the library (this is to prevent + # GCC or similar getting rpaths when they weren't at all + # configured) + args += ['-change', name, '@rpath/' + basename] + add_rpaths.add(dirname.rstrip('/')) + + # Check for nonexistent rpaths (often added by spack linker overzealousness + # with both lib/ and lib64/) and duplicate rpaths + for (rpath, count) in rpaths.items(): + if (rpath.startswith('@loader_path') + or rpath.startswith('@executable_path')): + # Allowable relative paths + pass + elif not _exists_dir(rpath): + tty.debug("Nonexistent rpath in {0}: {1}".format(abspath, rpath)) + del_rpaths.add(rpath) + elif count > 1: + # Rpath should only be there once, but it can sometimes be + # duplicated between Spack's compiler and libtool. If there are + # more copies of the same one, something is very odd.... + tty_debug = tty.debug if count == 2 else tty.warn + tty_debug("Rpath appears {0} times in {1}: {2}".format( + count, abspath, rpath + )) + del_rpaths.add(rpath) + + # Check for relocatable ID + if id_dylib is None: + tty.debug("No dylib ID is set for {0}".format(abspath)) + elif not id_dylib.startswith('@'): + tty.debug("Non-relocatable dylib ID for {0}: {1}" + .format(abspath, id_dylib)) + if root in rpaths or root in add_rpaths: + args += ['-id', '@rpath/' + filename] + else: + tty.debug("Allowing hardcoded dylib ID because its rpath " + "is *not* in the library already") + + # Delete bad rpaths + for rpath in del_rpaths: + args += ['-delete_rpath', rpath] + + # Add missing rpaths that are not set for deletion + for rpath in add_rpaths - del_rpaths - set(rpaths): + args += ['-add_rpath', rpath] + + if not args: + # No fixes needed + return False + + args.append(abspath) + executable.Executable('install_name_tool')(*args) + return True + + +def fixup_macos_rpaths(spec): + """Remove duplicate rpaths and make shared library IDs relocatable. + + Some autotools packages write their own ``-rpath`` entries in addition to + those implicitly added by the Spack compiler wrappers. On Linux these + duplicate rpaths are eliminated, but on macOS they result in multiple + entries which makes it harder to adjust with ``install_name_tool + -delete_rpath``. + + Furthermore, many autotools programs (on macOS) set a library's install + paths to use absolute paths rather than relative paths. + """ + if spec.external or spec.virtual: + tty.warn('external or virtual package cannot be fixed up: {0!s}' + .format(spec)) + return False + + if 'platform=darwin' not in spec: + raise NotImplementedError('fixup_macos_rpaths requires macOS') + + applied = 0 + + libs = frozenset(['lib', 'lib64', 'libexec', 'plugins', + 'Library', 'Frameworks']) + prefix = spec.prefix + + if not os.path.exists(prefix): + raise RuntimeError( + 'Could not fix up install prefix spec {0} because it does ' + 'not exist: {1!s}'.format(prefix, spec.name) + ) + + # Explore the installation prefix of the spec + for root, dirs, files in os.walk(prefix, topdown=True): + dirs[:] = set(dirs) & libs + for name in files: + try: + needed_fix = fixup_macos_rpath(root, name) + except Exception as e: + tty.warn("Failed to apply library fixups to: {0}/{1}: {2!s}" + .format(root, name, e)) + needed_fix = False + if needed_fix: + applied += 1 + + specname = spec.format('{name}{/hash:7}') + if applied: + tty.info('Fixed rpaths for {0:d} {1} installed to {2}'.format( + applied, + "binary" if applied == 1 else "binaries", + specname + )) + else: + tty.debug('No rpath fixup needed for ' + specname) diff --git a/lib/spack/spack/test/relocate.py b/lib/spack/spack/test/relocate.py index 2743235174..386ca88d2d 100644 --- a/lib/spack/spack/test/relocate.py +++ b/lib/spack/spack/test/relocate.py @@ -4,7 +4,6 @@ # SPDX-License-Identifier: (Apache-2.0 OR MIT) import collections import os.path -import platform import re import shutil @@ -14,6 +13,7 @@ import llnl.util.filesystem import spack.concretize import spack.paths +import spack.platforms import spack.relocate import spack.spec import spack.store @@ -21,6 +21,13 @@ import spack.tengine import spack.util.executable +def skip_unless_linux(f): + return pytest.mark.skipif( + str(spack.platforms.real_host()) != 'linux', + reason='implementation currently requires linux' + )(f) + + def rpaths_for(new_binary): """Return the RPATHs or RUNPATHs of a binary.""" patchelf = spack.util.executable.which('patchelf') @@ -144,6 +151,66 @@ def hello_world(tmpdir): return _factory +@pytest.fixture() +def make_dylib(tmpdir_factory): + """Create a shared library with unfriendly qualities. + + - Writes the same rpath twice + - Writes its install path as an absolute path + """ + cc = spack.util.executable.which('cc') + + def _factory(abs_install_name="abs", extra_rpaths=[]): + assert all(extra_rpaths) + + tmpdir = tmpdir_factory.mktemp( + abs_install_name + '-'.join(extra_rpaths).replace('/', '') + ) + src = tmpdir.join('foo.c') + src.write("int foo() { return 1; }\n") + + filename = 'foo.dylib' + lib = tmpdir.join(filename) + + args = ['-shared', str(src), '-o', str(lib)] + rpaths = list(extra_rpaths) + if abs_install_name.startswith('abs'): + args += ['-install_name', str(lib)] + else: + args += ['-install_name', '@rpath/' + filename] + + if abs_install_name.endswith('rpath'): + rpaths.append(str(tmpdir)) + + args.extend('-Wl,-rpath,' + s for s in rpaths) + + cc(*args) + + return (str(tmpdir), filename) + + return _factory + + +@pytest.fixture() +def make_object_file(tmpdir): + cc = spack.util.executable.which('cc') + + def _factory(): + src = tmpdir.join('bar.c') + src.write("int bar() { return 2; }\n") + + filename = 'bar.o' + lib = tmpdir.join(filename) + + args = ['-c', str(src), '-o', str(lib)] + + cc(*args) + + return (str(tmpdir), filename) + + return _factory + + @pytest.fixture() def copy_binary(): """Returns a function that copies a binary somewhere and @@ -179,10 +246,7 @@ def test_patchelf_is_relocatable(): assert spack.relocate.file_is_relocatable(patchelf) -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_file_is_relocatable_errors(tmpdir): # The file passed in as argument must exist... with pytest.raises(ValueError) as exc_info: @@ -199,10 +263,7 @@ def test_file_is_relocatable_errors(tmpdir): assert 'is not an absolute path' in str(exc_info.value) -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_search_patchelf(expected_patchelf_path): current = spack.relocate._patchelf() assert current == expected_patchelf_path @@ -272,10 +333,7 @@ def test_set_elf_rpaths_warning(mock_patchelf): @pytest.mark.requires_executables('patchelf', 'strings', 'file', 'gcc') -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_replace_prefix_bin(hello_world): # Compile an "Hello world!" executable and set RPATHs executable = hello_world(rpaths=['/usr/lib', '/usr/lib64']) @@ -288,10 +346,7 @@ def test_replace_prefix_bin(hello_world): @pytest.mark.requires_executables('patchelf', 'strings', 'file', 'gcc') -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_relocate_elf_binaries_absolute_paths( hello_world, copy_binary, tmpdir ): @@ -316,10 +371,7 @@ def test_relocate_elf_binaries_absolute_paths( @pytest.mark.requires_executables('patchelf', 'strings', 'file', 'gcc') -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_relocate_elf_binaries_relative_paths(hello_world, copy_binary): # Create an executable, set some RPATHs, copy it to another location orig_binary = hello_world(rpaths=['lib', 'lib64', '/opt/local/lib']) @@ -340,10 +392,7 @@ def test_relocate_elf_binaries_relative_paths(hello_world, copy_binary): @pytest.mark.requires_executables('patchelf', 'strings', 'file', 'gcc') -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_make_elf_binaries_relative(hello_world, copy_binary, tmpdir): orig_binary = hello_world(rpaths=[ str(tmpdir.mkdir('lib')), str(tmpdir.mkdir('lib64')), '/opt/local/lib' @@ -367,10 +416,7 @@ def test_raise_if_not_relocatable(monkeypatch): @pytest.mark.requires_executables('patchelf', 'strings', 'file', 'gcc') -@pytest.mark.skipif( - platform.system().lower() != 'linux', - reason='implementation for MacOS still missing' -) +@skip_unless_linux def test_relocate_text_bin(hello_world, copy_binary, tmpdir): orig_binary = hello_world(rpaths=[ str(tmpdir.mkdir('lib')), str(tmpdir.mkdir('lib64')), '/opt/local/lib' @@ -406,3 +452,59 @@ def test_relocate_text_bin_raise_if_new_prefix_is_longer(tmpdir): spack.relocate.relocate_text_bin( [fpath], {short_prefix: long_prefix} ) + + +@pytest.mark.requires_executables('install_name_tool', 'file', 'cc') +def test_fixup_macos_rpaths(make_dylib, make_object_file): + # For each of these tests except for the "correct" case, the first fixup + # should make changes, and the second fixup should be a null-op. + fixup_rpath = spack.relocate.fixup_macos_rpath + + no_rpath = [] + duplicate_rpaths = ['/usr', '/usr'] + bad_rpath = ['/nonexistent/path'] + + # Non-relocatable library id and duplicate rpaths + (root, filename) = make_dylib("abs", duplicate_rpaths) + assert fixup_rpath(root, filename) + assert not fixup_rpath(root, filename) + + # Bad but relocatable library id + (root, filename) = make_dylib("abs_with_rpath", no_rpath) + assert fixup_rpath(root, filename) + assert not fixup_rpath(root, filename) + + # Library id uses rpath but there are extra duplicate rpaths + (root, filename) = make_dylib("rpath", duplicate_rpaths) + assert fixup_rpath(root, filename) + assert not fixup_rpath(root, filename) + + # Shared library was constructed with relocatable id from the get-go + (root, filename) = make_dylib("rpath", no_rpath) + assert not fixup_rpath(root, filename) + + # Non-relocatable library id + (root, filename) = make_dylib("abs", no_rpath) + assert not fixup_rpath(root, filename) + + # Relocatable with executable paths and loader paths + (root, filename) = make_dylib("rpath", ['@executable_path/../lib', + '@loader_path']) + assert not fixup_rpath(root, filename) + + # Non-relocatable library id but nonexistent rpath + (root, filename) = make_dylib("abs", bad_rpath) + assert fixup_rpath(root, filename) + assert not fixup_rpath(root, filename) + + # Duplicate nonexistent rpath will need *two* passes + (root, filename) = make_dylib("rpath", bad_rpath * 2) + assert fixup_rpath(root, filename) + assert fixup_rpath(root, filename) + assert not fixup_rpath(root, filename) + + # Test on an object file, which *also* has type 'application/x-mach-binary' + # but should be ignored (no ID headers, no RPATH) + # (this is a corner case for GCC installation) + (root, filename) = make_object_file() + assert not fixup_rpath(root, filename) diff --git a/var/spack/repos/builtin/packages/zstd/package.py b/var/spack/repos/builtin/packages/zstd/package.py index 299ff0790f..9f35a34af0 100644 --- a/var/spack/repos/builtin/packages/zstd/package.py +++ b/var/spack/repos/builtin/packages/zstd/package.py @@ -37,10 +37,10 @@ class Zstd(MakefilePackage): depends_on('lzma', when='+programs') depends_on('lz4', when='+programs') - def _make(self, *args): + def _make(self, *args, **kwargs): # PREFIX must be defined on macOS even when building the library, since # it gets hardcoded into the library's install_path - make('VERBOSE=1', 'PREFIX=' + self.prefix, '-C', *args) + make('VERBOSE=1', 'PREFIX=' + self.prefix, '-C', *args, **kwargs) def build(self, spec, prefix): self._make('lib') @@ -48,6 +48,6 @@ class Zstd(MakefilePackage): self._make('programs') def install(self, spec, prefix): - self._make('lib', 'install') + self._make('lib', 'install', parallel=False) if spec.variants['programs'].value: self._make('programs', 'install') -- cgit v1.2.3-70-g09d2