diff --git a/Changelog b/Changelog index 0bfc5e46..71658631 100644 --- a/Changelog +++ b/Changelog @@ -30,6 +30,8 @@ Releases [#123](https://github.com/matthew-brett/delocate/pull/123) * Wheels with multiple packages will no longer copy duplicate libraries. [#35](https://github.com/matthew-brett/delocate/issues/35) + * ``delocate.tools.back_tick`` has been deprecated. + [#126](https://github.com/matthew-brett/delocate/pull/126) * 0.9.1 (Friday September 17th 2021) diff --git a/delocate/tests/test_delocating.py b/delocate/tests/test_delocating.py index 4979995d..15a9439c 100644 --- a/delocate/tests/test_delocating.py +++ b/delocate/tests/test_delocating.py @@ -6,6 +6,7 @@ from os.path import (join as pjoin, dirname, basename, relpath, realpath, splitext) import shutil +import subprocess from typing import Any, Callable, Dict, Iterable, List, Set, Text, Tuple from collections import namedtuple @@ -16,11 +17,11 @@ filter_system_libs) from ..libsana import (tree_libs, search_environment_for_lib, tree_libs_from_directory) -from ..tools import (get_install_names, set_install_name, back_tick) +from ..tools import (get_install_names, set_install_name) from ..tmpdirs import InTemporaryDirectory -from .pytest_tools import (assert_true, assert_raises, assert_equal) +from .pytest_tools import (assert_raises, assert_equal) from .test_install_names import (LIBA, LIBB, LIBC, TEST_LIB, _copy_libs, EXT_LIBS) @@ -34,8 +35,7 @@ ('liba', 'libb', 'libc', 'test_lib', 'slibc', 'stest_lib')) -def _make_libtree(out_path): - # type: (Text) -> LibtreeLibs +def _make_libtree(out_path: str) -> LibtreeLibs: liba, libb, libc, test_lib = _copy_libs( [LIBA, LIBB, LIBC, TEST_LIB], out_path) sub_path = pjoin(out_path, 'subsub') @@ -44,8 +44,10 @@ def _make_libtree(out_path): for exe in (test_lib, stest_lib): os.chmod(exe, 0o744) # Check test-lib doesn't work because of relative library paths - assert_raises(RuntimeError, back_tick, [test_lib]) - assert_raises(RuntimeError, back_tick, [stest_lib]) + with pytest.raises(subprocess.CalledProcessError): + subprocess.run([test_lib], check=True) + with pytest.raises(subprocess.CalledProcessError): + subprocess.run([stest_lib], check=True) # Fixup the relative path library names by setting absolute paths for fname, using, path in ( (libb, 'liba.dylib', out_path), @@ -58,8 +60,8 @@ def _make_libtree(out_path): ): set_install_name(fname, using, pjoin(path, using)) # Check scripts now execute correctly - back_tick([test_lib]) - back_tick([stest_lib]) + subprocess.run([test_lib], check=True) + subprocess.run([stest_lib], check=True) return LibtreeLibs(liba, libb, libc, test_lib, slibc, stest_lib) @@ -108,16 +110,15 @@ def test_delocate_tree_libs( lib_dict = without_system_libs(tree_libs_func(subtree)) copied = delocate_tree_libs(lib_dict, copy_dir, subtree) # Out-of-tree library copied. - assert_equal(copied, {fake_lib: {realpath(liba): fake_lib}}) - assert_equal(os.listdir(copy_dir), [basename(fake_lib)]) + assert copied == {fake_lib: {realpath(liba): fake_lib}} + assert os.listdir(copy_dir) == [basename(fake_lib)] # Library using the copied library now has an # install name starting with @loader_path, then # pointing to the copied library directory pathto_copies = relpath(realpath(copy_dir), dirname(realpath(liba))) lib_inames = without_system_libs(get_install_names(liba)) - new_link = '@loader_path/{0}/{1}'.format(pathto_copies, - basename(fake_lib)) - assert_true([new_link] <= lib_inames) + new_link = f'@loader_path/{pathto_copies}/{basename(fake_lib)}' + assert [new_link] <= lib_inames # Libraries now have a relative loader_path to their corresponding # in-tree libraries for requiring, using, rel_path in ( @@ -130,17 +131,17 @@ def test_delocate_tree_libs( (stest_lib, 'libc.dylib', '')): loader_path = '@loader_path/' + rel_path + using not_sys_req = without_system_libs(get_install_names(requiring)) - assert_true(loader_path in not_sys_req) + assert loader_path in not_sys_req # Another copy to delocate, now without faked out-of-tree dependency. subtree = pjoin(tmpdir, 'subtree1') out_libs = _make_libtree(subtree) lib_dict = without_system_libs(tree_libs_func(subtree)) copied = delocate_tree_libs(lib_dict, copy_dir, subtree) # Now no out-of-tree libraries, nothing copied. - assert_equal(copied, {}) + assert copied == {} # Check test libs still work - back_tick([out_libs.test_lib]) - back_tick([out_libs.stest_lib]) + subprocess.run([out_libs.test_lib], check=True) + subprocess.run([out_libs.stest_lib], check=True) # Check case where all local libraries are out of tree subtree2 = pjoin(tmpdir, 'subtree2') liba, libb, libc, test_lib, slibc, stest_lib = _make_libtree(subtree2) @@ -150,8 +151,8 @@ def test_delocate_tree_libs( # out-of-tree will raise an error because of duplicate library names # (libc and slibc both named /libc.dylib) lib_dict2 = without_system_libs(tree_libs_func(subtree2)) - assert_raises(DelocationError, - delocate_tree_libs, lib_dict2, copy_dir2, '/fictional') + with pytest.raises(DelocationError): + delocate_tree_libs(lib_dict2, copy_dir2, '/fictional') # Rename a library to make this work new_slibc = pjoin(dirname(slibc), 'libc2.dylib') os.rename(slibc, new_slibc) @@ -159,8 +160,8 @@ def test_delocate_tree_libs( set_install_name(stest_lib, slibc, new_slibc) slibc = new_slibc # Confirm new test-lib still works - back_tick([test_lib]) - back_tick([stest_lib]) + subprocess.run([test_lib], check=True) + subprocess.run([stest_lib], check=True) # Delocation now works lib_dict2 = without_system_libs(tree_libs_func(subtree2)) copied2 = delocate_tree_libs(lib_dict2, copy_dir2, '/fictional') @@ -175,24 +176,23 @@ def test_delocate_tree_libs( rp_liba: {rp_slibc: liba, rp_libc: liba, rp_libb: liba}} - assert_equal(copied2, exp_dict) + assert copied2 == exp_dict ext_local_libs = {liba, libb, libc, slibc} - assert_equal(set(os.listdir(copy_dir2)), - set([basename(lib) for lib in ext_local_libs])) + assert set(os.listdir(copy_dir2)) == { + basename(lib) for lib in ext_local_libs + } # Libraries using the copied libraries now have an install name starting # with @loader_path, then pointing to the copied library directory for lib in (liba, libb, libc, test_lib, slibc, stest_lib): pathto_copies = relpath(realpath(copy_dir2), dirname(realpath(lib))) lib_inames = get_install_names(lib) - new_links = ['@loader_path/{0}/{1}'.format(pathto_copies, - basename(elib)) + new_links = [f'@loader_path/{pathto_copies}/{basename(elib)}' for elib in copied] - assert_true(set(new_links) <= set(lib_inames)) + assert set(new_links) <= set(lib_inames) -def _copy_fixpath(files, directory): - # type: (Iterable[Text], Text) -> List[Text] +def _copy_fixpath(files: Iterable[str], directory: str) -> List[str]: new_fnames = [] for fname in files: shutil.copy2(fname, directory) @@ -204,8 +204,7 @@ def _copy_fixpath(files, directory): return new_fnames -def _copy_to(fname, directory, new_base): - # type: (Text, Text, Text) -> Text +def _copy_to(fname: str, directory: str, new_base: str) -> str: new_name = pjoin(directory, new_base) shutil.copy2(fname, new_name) return new_name @@ -213,8 +212,7 @@ def _copy_to(fname, directory, new_base): @pytest.mark.filterwarnings("ignore:tree_libs:DeprecationWarning") @pytest.mark.filterwarnings("ignore:copy_recurse:DeprecationWarning") -def test_copy_recurse(): - # type: () -> None +def test_copy_recurse() -> None: # Function to find / copy needed libraries recursively with InTemporaryDirectory(): # Get some fixed up libraries to play with @@ -224,35 +222,34 @@ def test_copy_recurse(): # Set execute permissions os.chmod(test_lib, 0o744) # Check system finds libraries - back_tick(['./libcopy/test-lib']) + subprocess.run(['./libcopy/test-lib'], check=True) # One library, depends only on system libs, system libs filtered - def filt_func(libname): - # type: (Text) -> bool + def filt_func(libname: str) -> bool: return not libname.startswith('/usr/lib') os.makedirs('subtree') _copy_fixpath([LIBA], 'subtree') # Nothing copied therefore - assert_equal(copy_recurse('subtree', copy_filt_func=filt_func), {}) - assert_equal(set(os.listdir('subtree')), {'liba.dylib'}) + assert copy_recurse('subtree', copy_filt_func=filt_func) == {} + assert set(os.listdir('subtree')) == {'liba.dylib'} # shortcut _rp = realpath # An object that depends on a library that depends on two libraries # test_lib depends on libc, libc depends on liba and libb. libc gets # copied first, then liba, libb - def _st(fname): - # type: (Text) -> Text + def _st(fname: str) -> str: return _rp(pjoin('subtree2', basename(fname))) os.makedirs('subtree2') shutil.copy2(test_lib, 'subtree2') - assert_equal(copy_recurse('subtree2', filt_func), - {_rp(libc): {_st(test_lib): libc}, - _rp(libb): {_rp(libc): libb}, - _rp(liba): {_rp(libb): liba, - _rp(libc): liba}}) - assert_equal(set(os.listdir('subtree2')), - {'liba.dylib', 'libb.dylib', 'libc.dylib', 'test-lib'}) + assert copy_recurse('subtree2', filt_func) == { + _rp(libc): {_st(test_lib): libc}, + _rp(libb): {_rp(libc): libb}, + _rp(liba): {_rp(libb): liba, _rp(libc): liba} + } + assert set(os.listdir('subtree2')) == { + 'liba.dylib', 'libb.dylib', 'libc.dylib', 'test-lib' + } # A circular set of libraries os.makedirs('libcopy2') libw = _copy_to(LIBA, 'libcopy2', 'libw.dylib') @@ -274,26 +271,22 @@ def _st(fname): os.makedirs('subtree3') seed_path = pjoin('subtree3', 'seed') shutil.copy2(libw, seed_path) - assert_equal(copy_recurse('subtree3'), # not filtered - # First pass, libx, liby get copied - {_rp(libx): {_rp(seed_path): libx, - _rp(libw): libx, - _rp(libz): libx}, - _rp(liby): {_rp(seed_path): liby, - _rp(libw): liby, - _rp(libx): liby}, - _rp(libw): {_rp(libx): libw, - _rp(liby): libw, - _rp(libz): libw}, - _rp(libz): {_rp(liby): libz}}) - assert_equal(set(os.listdir('subtree3')), - {'seed', 'libw.dylib', 'libx.dylib', - 'liby.dylib', 'libz.dylib'}) + assert copy_recurse('subtree3') == { # not filtered + # First pass, libx, liby get copied + _rp(libx): {_rp(seed_path): libx, _rp(libw): libx, _rp(libz): libx}, + _rp(liby): {_rp(seed_path): liby, _rp(libw): liby, _rp(libx): liby}, + _rp(libw): {_rp(libx): libw, _rp(liby): libw, _rp(libz): libw}, + _rp(libz): {_rp(liby): libz} + } + assert set(os.listdir('subtree3')) == { + 'seed', 'libw.dylib', 'libx.dylib', 'liby.dylib', 'libz.dylib' + } for tlib, dep1, dep2 in t_dep1_dep2: out_lib = pjoin('subtree3', basename(tlib)) - assert_equal(set(get_install_names(out_lib)), - {'@loader_path/' + basename(dep1), - '@loader_path/' + basename(dep2)}) + assert set(get_install_names(out_lib)) == { + '@loader_path/' + basename(dep1), + '@loader_path/' + basename(dep2), + } # Check case of not-empty copied_libs os.makedirs('subtree4') shutil.copy2(libw, 'subtree4') @@ -301,17 +294,14 @@ def _st(fname): _rp(liby): libw, _rp(libz): libw}} copied_copied = copied_libs.copy() - assert_equal(copy_recurse('subtree4', None, copied_libs), - {_rp(libw): {_rp(libx): libw, - _rp(liby): libw, - _rp(libz): libw}, - _rp(libx): {_rp(libw): libx, - _rp(libz): libx}, - _rp(liby): {_rp(libw): liby, - _rp(libx): liby}, - _rp(libz): {_rp(liby): libz}}) + assert copy_recurse('subtree4', None, copied_libs) == { + _rp(libw): {_rp(libx): libw, _rp(liby): libw, _rp(libz): libw}, + _rp(libx): {_rp(libw): libx, _rp(libz): libx}, + _rp(liby): {_rp(libw): liby, _rp(libx): liby}, + _rp(libz): {_rp(liby): libz} + } # Not modified in-place - assert_equal(copied_libs, copied_copied) + assert copied_libs == copied_copied @pytest.mark.filterwarnings("ignore:tree_libs:DeprecationWarning") @@ -340,18 +330,17 @@ def filt_func(libname): copy_recurse('subtree', filt_func) -def test_delocate_path(): - # type: () -> None +def test_delocate_path() -> None: # Test high-level path delocator script with InTemporaryDirectory(): # Make a tree; use realpath for OSX /private/var - /var _, _, _, test_lib, slibc, stest_lib = _make_libtree( realpath('subtree')) # Check it fixes up correctly - assert_equal(delocate_path('subtree', 'deplibs'), {}) - assert_equal(len(os.listdir('deplibs')), 0) - back_tick([test_lib]) - back_tick([stest_lib]) + assert delocate_path('subtree', 'deplibs') == {} + assert len(os.listdir('deplibs')) == 0 + subprocess.run([test_lib], check=True) + subprocess.run([stest_lib], check=True) # Make a fake external library to link to os.makedirs('fakelibs') fake_lib = realpath(_copy_to(LIBA, 'fakelibs', 'libfake.dylib')) @@ -362,38 +351,38 @@ def test_delocate_path(): _rp = realpath # Check fake libary gets copied and delocated slc_rel = pjoin('subtree2', 'subsub', 'libc.dylib') - assert_equal(delocate_path('subtree2', 'deplibs2'), - {_rp(fake_lib): {_rp(slc_rel): fake_lib}}) - assert_equal(os.listdir('deplibs2'), ['libfake.dylib']) - assert_true('@loader_path/../../deplibs2/libfake.dylib' in - get_install_names(slibc)) + assert delocate_path('subtree2', 'deplibs2') == { + _rp(fake_lib): {_rp(slc_rel): fake_lib} + } + assert os.listdir('deplibs2') == ['libfake.dylib'] + assert( + '@loader_path/../../deplibs2/libfake.dylib' + in get_install_names(slibc) + ) # Unless we set the filter otherwise _, _, _, test_lib, slibc, stest_lib = _make_libtree( realpath('subtree3')) set_install_name(slibc, EXT_LIBS[0], fake_lib) - def filt(libname): - # type: (Text) -> bool - return not (libname.startswith('/usr') or - 'libfake' in libname) - assert_equal(delocate_path('subtree3', 'deplibs3', None, filt), {}) - assert_equal(len(os.listdir('deplibs3')), 0) + def filt(libname: str) -> bool: + return not (libname.startswith('/usr') or 'libfake' in libname) + assert delocate_path('subtree3', 'deplibs3', None, filt) == {} + assert len(os.listdir('deplibs3')) == 0 # Test tree names filtering works _, _, _, test_lib, slibc, stest_lib = _make_libtree( realpath('subtree4')) set_install_name(slibc, EXT_LIBS[0], fake_lib) - def lib_filt(filename): - # type: (Text) -> bool + def lib_filt(filename: str) -> bool: return not filename.endswith('subsub/libc.dylib') - assert_equal(delocate_path('subtree4', 'deplibs4', lib_filt), {}) - assert_equal(len(os.listdir('deplibs4')), 0) + assert delocate_path('subtree4', 'deplibs4', lib_filt) == {} + assert len(os.listdir('deplibs4')) == 0 # Check can use already existing directory os.makedirs('deplibs5') _, _, _, test_lib, slibc, stest_lib = _make_libtree( realpath('subtree5')) - assert_equal(delocate_path('subtree5', 'deplibs5'), {}) - assert_equal(len(os.listdir('deplibs5')), 0) + assert delocate_path('subtree5', 'deplibs5') == {} + assert len(os.listdir('deplibs5')) == 0 def _make_bare_depends(): @@ -563,8 +552,7 @@ def test_bads_report(): (LIBM1, ARCH_M1)}) -def test_dyld_library_path_lookups(): - # type: () -> None +def test_dyld_library_path_lookups() -> None: # Test that DYLD_LIBRARY_PATH can be used to find libs during # delocation with TempDirWithoutEnvVars('DYLD_LIBRARY_PATH') as tmpdir: @@ -576,14 +564,14 @@ def test_dyld_library_path_lookups(): hidden_dir = 'hidden' os.mkdir(hidden_dir) new_libb = os.path.join(hidden_dir, os.path.basename(LIBB)) - shutil.move(libb, - new_libb) - assert_raises(RuntimeError, back_tick, [test_lib]) + shutil.move(libb, new_libb) + with pytest.raises(subprocess.CalledProcessError): + subprocess.run([test_lib], check=True) # Update DYLD_LIBRARY_PATH and confirm that we can now # successfully delocate test_lib os.environ['DYLD_LIBRARY_PATH'] = hidden_dir delocate_path('subtree', 'deplibs') - back_tick(test_lib) + subprocess.run([test_lib], check=True) def test_dyld_library_path_beats_basename(): diff --git a/delocate/tests/test_fuse.py b/delocate/tests/test_fuse.py index 8eff8196..1592c046 100644 --- a/delocate/tests/test_fuse.py +++ b/delocate/tests/test_fuse.py @@ -4,26 +4,27 @@ import os import sys from os.path import (join as pjoin, relpath, isdir, dirname, basename) +import subprocess import shutil -from ..tools import (cmp_contents, get_archs, zip2dir, dir2zip, back_tick, +from ..tools import (cmp_contents, get_archs, zip2dir, dir2zip, open_readable) from ..fuse import fuse_trees, fuse_wheels from ..tmpdirs import InTemporaryDirectory from ..wheeltools import rewrite_record -from .pytest_tools import (assert_true, assert_false, assert_equal) +from .pytest_tools import (assert_false, assert_equal) from .test_tools import LIBM1, LIB64, LIB64A from .test_wheelies import PURE_WHEEL from .test_wheeltools import assert_record_equal -def assert_same_tree(tree1, tree2): +def assert_same_tree(tree1: str, tree2: str) -> None: for dirpath, dirnames, filenames in os.walk(tree1): tree2_dirpath = pjoin(tree2, relpath(dirpath, tree1)) for dname in dirnames: - assert_true(isdir(pjoin(tree2_dirpath, dname))) + assert isdir(pjoin(tree2_dirpath, dname)) for fname in filenames: tree1_path = pjoin(dirpath, fname) with open_readable(tree1_path, 'rb') as fobj: @@ -33,7 +34,7 @@ def assert_same_tree(tree1, tree2): if fname == 'RECORD': # Record can have different line orders assert_record_equal(contents1, contents2) else: - assert_equal(contents1, contents2) + assert contents1 == contents2 def assert_listdir_equal(path, listing): @@ -85,7 +86,7 @@ def test_fuse_trees(): ['afile.txt', 'anotherfile.txt', 'liba.a', 'tests']) -def test_fuse_wheels(): +def test_fuse_wheels() -> None: # Test function to fuse two wheels wheel_base = basename(PURE_WHEEL) with InTemporaryDirectory(): @@ -97,7 +98,9 @@ def test_fuse_wheels(): zip2dir(wheel_base, 'fused_wheel') assert_same_tree('to_wheel', 'fused_wheel') # Check unpacking works on fused wheel - back_tick([sys.executable, '-m', 'wheel', 'unpack', wheel_base]) + subprocess.run( + [sys.executable, '-m', 'wheel', 'unpack', wheel_base], check=True + ) # Put lib into wheel shutil.copyfile(LIB64A, pjoin('from_wheel', 'fakepkg2', 'liba.a')) rewrite_record('from_wheel') @@ -118,4 +121,4 @@ def test_fuse_wheels(): fuse_wheels('to_wheel.whl', 'from_wheel.whl', wheel_base) zip2dir(wheel_base, 'fused_wheel') fused_fname = pjoin('fused_wheel', 'fakepkg2', 'liba.dylib') - assert_equal(get_archs(fused_fname), set(('arm64', 'x86_64'))) + assert get_archs(fused_fname) == set(('arm64', 'x86_64')) diff --git a/delocate/tests/test_scripts.py b/delocate/tests/test_scripts.py index f1a9f5ec..32f821ff 100644 --- a/delocate/tests/test_scripts.py +++ b/delocate/tests/test_scripts.py @@ -12,10 +12,11 @@ from os.path import (dirname, join as pjoin, isfile, abspath, realpath, basename, exists, splitext) import shutil +import subprocess from typing import Text from ..tmpdirs import InTemporaryDirectory, InGivenDirectory -from ..tools import back_tick, set_install_name, zip2dir, dir2zip +from ..tools import set_install_name, zip2dir, dir2zip from ..wheeltools import InWheel from .scriptrunner import ScriptRunner @@ -142,7 +143,7 @@ def test_listdeps(plat_wheel: PlatWheel) -> None: ] -def test_path(): +def test_path() -> None: # Test path cleaning with InTemporaryDirectory(): # Make a tree; use realpath for OSX /private/var - /var @@ -153,16 +154,16 @@ def test_path(): fake_lib = realpath(_copy_to(liba, 'fakelibs', 'libfake.dylib')) _, _, _, test_lib, slibc, stest_lib = _make_libtree( realpath('subtree2')) - back_tick([test_lib]) - back_tick([stest_lib]) + subprocess.run([test_lib], check=True) + subprocess.run([stest_lib], check=True) set_install_name(slibc, EXT_LIBS[0], fake_lib) # Check it fixes up correctly code, stdout, stderr = run_command( ['delocate-path', 'subtree', 'subtree2', '-L', 'deplibs']) - assert_equal(len(os.listdir(pjoin('subtree', 'deplibs'))), 0) + assert len(os.listdir(pjoin('subtree', 'deplibs'))) == 0 # Check fake libary gets copied and delocated out_path = pjoin('subtree2', 'deplibs') - assert_equal(os.listdir(out_path), ['libfake.dylib']) + assert os.listdir(out_path) == ['libfake.dylib'] def test_path_dylibs(): diff --git a/delocate/tests/test_tools.py b/delocate/tests/test_tools.py index 1d1159e1..0ec57c25 100644 --- a/delocate/tests/test_tools.py +++ b/delocate/tests/test_tools.py @@ -3,9 +3,12 @@ import os from os.path import join as pjoin, dirname +import subprocess import stat import shutil +import pytest + from ..tools import (back_tick, unique_by_index, ensure_writable, chmod_perms, ensure_permissions, parse_install_name, zip2dir, dir2zip, find_package_dirs, cmp_contents, get_archs, lipo_fuse, @@ -30,13 +33,14 @@ ARCH_32 = frozenset(['i386']) -def test_back_tick(): - cmd = 'python -c "print(\'Hello\')"' - assert_equal(back_tick(cmd), "Hello") - assert_equal(back_tick(cmd, ret_err=True), ("Hello", "")) - assert_equal(back_tick(cmd, True, False), (b"Hello", b"")) - cmd = 'python -c "raise ValueError()"' - assert_raises(RuntimeError, back_tick, cmd) +@pytest.mark.filterwarnings("ignore:back_tick is deprecated") +def test_back_tick() -> None: + cmd = '''python -c "print('Hello')"''' + assert back_tick(cmd) == "Hello" + assert back_tick(cmd, ret_err=True) == ("Hello", "") + assert back_tick(cmd, True, False) == (b"Hello", b"") + with pytest.raises(RuntimeError): + back_tick('python -c "raise ValueError()"') def test_uniqe_by_index(): @@ -239,11 +243,11 @@ def test_get_archs_fuse(): 'libcopy64', LIB64, 'yetanother') -def test_validate_signature(): +def test_validate_signature() -> None: # Fully test the validate_signature tool - def check_signature(filename): - """Raises RuntimeError if codesign can not verify the signature.""" - back_tick(['codesign', '--verify', filename], raise_err=True) + def check_signature(filename: str) -> None: + """Raises CalledProcessError if the signature can not be verified.""" + subprocess.run(['codesign', '--verify', filename], check=True) with InTemporaryDirectory(): # Copy a binary file to test with, any binary file would work @@ -253,7 +257,8 @@ def check_signature(filename): validate_signature('libcopy') # codesign should raise an error (missing signature) - assert_raises(RuntimeError, check_signature, 'libcopy') + with pytest.raises(subprocess.CalledProcessError): + check_signature('libcopy') replace_signature('libcopy', '-') # Force this file to be signed validate_signature('libcopy') # Cover the `is already valid` code path @@ -264,7 +269,8 @@ def check_signature(filename): add_rpath('libcopy', '/dummy/path', ad_hoc_sign=False) # codesign should raise a new error (invalid signature) - assert_raises(RuntimeError, check_signature, 'libcopy') + with pytest.raises(subprocess.CalledProcessError): + check_signature('libcopy') validate_signature('libcopy') # Replace the broken signature check_signature('libcopy') @@ -277,15 +283,20 @@ def check_signature(filename): set_install_id('libcopy', 'libcopy-name') check_signature('libcopy') - set_install_name('libcopy', LIBSTDCXX, - '/usr/lib/libstdc++.7.dylib') + set_install_name('libcopy', LIBSTDCXX, '/usr/lib/libstdc++.7.dylib') check_signature('libcopy') # check that altering the contents without ad-hoc sign invalidates # signatures set_install_id('libcopy', 'libcopy-name2', ad_hoc_sign=False) - assert_raises(RuntimeError, check_signature, 'libcopy') - - set_install_name('libcopy', '/usr/lib/libstdc++.7.dylib', - '/usr/lib/libstdc++.8.dylib', ad_hoc_sign=False) - assert_raises(RuntimeError, check_signature, 'libcopy') + with pytest.raises(subprocess.CalledProcessError): + check_signature('libcopy') + + set_install_name( + 'libcopy', + '/usr/lib/libstdc++.7.dylib', + '/usr/lib/libstdc++.8.dylib', + ad_hoc_sign=False, + ) + with pytest.raises(subprocess.CalledProcessError): + check_signature('libcopy') diff --git a/delocate/tests/test_wheelies.py b/delocate/tests/test_wheelies.py index 9fb1ddd0..8307fff9 100644 --- a/delocate/tests/test_wheelies.py +++ b/delocate/tests/test_wheelies.py @@ -6,6 +6,7 @@ import stat from glob import glob import shutil +import subprocess from subprocess import check_call from typing import NamedTuple @@ -14,7 +15,7 @@ from ..delocating import (DelocationError, delocate_wheel, patch_wheel, DLC_PREFIX) from ..tools import (get_install_names, set_install_name, zip2dir, - dir2zip, back_tick, get_install_id, get_archs) + dir2zip, get_install_id, get_archs) from ..wheeltools import InWheel from ..tmpdirs import InTemporaryDirectory, InGivenDirectory @@ -93,53 +94,54 @@ def _rename_module(in_wheel, mod_fname, out_wheel): return out_wheel -def test_fix_plat(): +def test_fix_plat() -> None: # Can we fix a wheel with a stray library? # We have to make one that works first with InTemporaryDirectory() as tmpdir: fixed_wheel, stray_lib = _fixed_wheel(tmpdir) - assert_true(exists(stray_lib)) + assert exists(stray_lib) # Shortcut _rp = realpath # In-place fix dep_mod = pjoin('fakepkg1', 'subpkg', 'module2.abi3.so') - assert_equal(delocate_wheel(fixed_wheel), - {_rp(stray_lib): {dep_mod: stray_lib}}) + assert delocate_wheel(fixed_wheel) == { + _rp(stray_lib): {dep_mod: stray_lib} + } zip2dir(fixed_wheel, 'plat_pkg') - assert_true(exists(pjoin('plat_pkg', 'fakepkg1'))) + assert exists(pjoin('plat_pkg', 'fakepkg1')) dylibs = pjoin('plat_pkg', 'fakepkg1', '.dylibs') - assert_true(exists(dylibs)) - assert_equal(os.listdir(dylibs), ['libextfunc.dylib']) + assert exists(dylibs) + assert os.listdir(dylibs) == ['libextfunc.dylib'] # New output name fixed_wheel, stray_lib = _fixed_wheel(tmpdir) - assert_equal(delocate_wheel(fixed_wheel, 'fixed_wheel.ext'), - {_rp(stray_lib): {dep_mod: stray_lib}}) + assert delocate_wheel( + fixed_wheel, 'fixed_wheel.ext' + ) == {_rp(stray_lib): {dep_mod: stray_lib}} zip2dir('fixed_wheel.ext', 'plat_pkg1') - assert_true(exists(pjoin('plat_pkg1', 'fakepkg1'))) + assert exists(pjoin('plat_pkg1', 'fakepkg1')) dylibs = pjoin('plat_pkg1', 'fakepkg1', '.dylibs') - assert_true(exists(dylibs)) - assert_equal(os.listdir(dylibs), ['libextfunc.dylib']) + assert exists(dylibs) + assert os.listdir(dylibs) == ['libextfunc.dylib'] # Test another lib output directory - assert_equal(delocate_wheel(fixed_wheel, - 'fixed_wheel2.ext', - 'dylibs_dir'), - {_rp(stray_lib): {dep_mod: stray_lib}}) + assert delocate_wheel( + fixed_wheel, 'fixed_wheel2.ext', 'dylibs_dir' + ) == {_rp(stray_lib): {dep_mod: stray_lib}} zip2dir('fixed_wheel2.ext', 'plat_pkg2') - assert_true(exists(pjoin('plat_pkg2', 'fakepkg1'))) + assert exists(pjoin('plat_pkg2', 'fakepkg1')) dylibs = pjoin('plat_pkg2', 'fakepkg1', 'dylibs_dir') - assert_true(exists(dylibs)) - assert_equal(os.listdir(dylibs), ['libextfunc.dylib']) + assert exists(dylibs) + assert os.listdir(dylibs) == ['libextfunc.dylib'] # Test check for existing output directory - assert_raises(DelocationError, - delocate_wheel, - fixed_wheel, - 'broken_wheel.ext', - 'subpkg') + with pytest.raises(DelocationError): + delocate_wheel(fixed_wheel, 'broken_wheel.ext', 'subpkg') # Test that `wheel unpack` works fixed_wheel, stray_lib = _fixed_wheel(tmpdir) - assert_equal(delocate_wheel(fixed_wheel), - {_rp(stray_lib): {dep_mod: stray_lib}}) - back_tick([sys.executable, '-m', 'wheel', 'unpack', fixed_wheel]) + assert delocate_wheel(fixed_wheel) == { + _rp(stray_lib): {dep_mod: stray_lib} + } + subprocess.run( + [sys.executable, '-m', 'wheel', 'unpack', fixed_wheel], check=True + ) # Check that copied libraries have modified install_name_ids zip2dir(fixed_wheel, 'plat_pkg3') base_stray = basename(stray_lib) @@ -270,7 +272,7 @@ def _fix_break_fix(arch_): require_archs=(), check_verbose=True) -def test_patch_wheel(): +def test_patch_wheel() -> None: # Check patching of wheel with InTemporaryDirectory(): # First wheel needs proper wheel filename for later unpack test @@ -278,23 +280,25 @@ def test_patch_wheel(): patch_wheel(PURE_WHEEL, WHEEL_PATCH, out_fname) zip2dir(out_fname, 'wheel1') with open(pjoin('wheel1', 'fakepkg2', '__init__.py'), 'rt') as fobj: - assert_equal(fobj.read(), 'print("Am in init")\n') + assert fobj.read() == 'print("Am in init")\n' # Check that wheel unpack works - back_tick([sys.executable, '-m', 'wheel', 'unpack', out_fname]) + subprocess.run( + [sys.executable, '-m', 'wheel', 'unpack', out_fname], check=True + ) # Copy the original, check it doesn't have patch shutil.copyfile(PURE_WHEEL, 'copied.whl') zip2dir('copied.whl', 'wheel2') with open(pjoin('wheel2', 'fakepkg2', '__init__.py'), 'rt') as fobj: - assert_equal(fobj.read(), '') + assert fobj.read() == '' # Overwrite input wheel (the default) patch_wheel('copied.whl', WHEEL_PATCH) # Patched zip2dir('copied.whl', 'wheel3') with open(pjoin('wheel3', 'fakepkg2', '__init__.py'), 'rt') as fobj: - assert_equal(fobj.read(), 'print("Am in init")\n') + assert fobj.read() == 'print("Am in init")\n' # Check bad patch raises error - assert_raises(RuntimeError, - patch_wheel, PURE_WHEEL, WHEEL_PATCH_BAD, 'out.whl') + with pytest.raises(RuntimeError): + patch_wheel(PURE_WHEEL, WHEEL_PATCH_BAD, 'out.whl') def test_fix_rpath(): diff --git a/delocate/tests/test_wheeltools.py b/delocate/tests/test_wheeltools.py index 91860aeb..1ff25f99 100644 --- a/delocate/tests/test_wheeltools.py +++ b/delocate/tests/test_wheeltools.py @@ -4,6 +4,7 @@ import os from os.path import join as pjoin, exists, isfile, basename, realpath, splitext import shutil +from typing import AnyStr try: from wheel.install import WheelFile @@ -37,9 +38,8 @@ for plat in (EXP_PLAT,) + EXTRA_PLATS] -def assert_record_equal(record_orig, record_new): - assert_equal(sorted(record_orig.splitlines()), - sorted(record_new.splitlines())) +def assert_record_equal(record_orig: AnyStr, record_new: AnyStr) -> None: + assert sorted(record_orig.splitlines()) == sorted(record_new.splitlines()) def test_rewrite_record(): diff --git a/delocate/tools.py b/delocate/tools.py index 3577622e..416c9af0 100644 --- a/delocate/tools.py +++ b/delocate/tools.py @@ -1,10 +1,9 @@ """ Tools for getting and setting install names """ - -from subprocess import Popen, PIPE - import os from os.path import join as pjoin, relpath, isdir, exists -from typing import Set +from typing import Any, FrozenSet, List, Optional, Sequence, Set, Union +import subprocess +import warnings import zipfile import re import stat @@ -15,7 +14,12 @@ class InstallNameError(Exception): pass -def back_tick(cmd, ret_err=False, as_str=True, raise_err=None): +def back_tick( + cmd: Union[str, Sequence[str]], + ret_err: bool = False, + as_str: bool = True, + raise_err: Optional[bool] = None, +) -> Any: """ Run command `cmd`, return stdout, or stdout, stderr if `ret_err` Roughly equivalent to ``check_output`` in Python 2.7 @@ -45,29 +49,35 @@ def back_tick(cmd, ret_err=False, as_str=True, raise_err=None): ------ Raises RuntimeError if command returns non-zero exit code and `raise_err` is True + + .. deprecated:: 0.10 + This function was deprecated because the return type is too dynamic. + You should use :func:`subprocess.run` instead. """ + warnings.warn( + "back_tick is deprecated, replace this call with subprocess.run.", + DeprecationWarning, + stacklevel=2, + ) if raise_err is None: raise_err = False if ret_err else True cmd_is_seq = isinstance(cmd, (list, tuple)) - proc = Popen(cmd, stdout=PIPE, stderr=PIPE, shell=not cmd_is_seq) - out, err = proc.communicate() - retcode = proc.returncode - cmd_str = ' '.join(cmd) if cmd_is_seq else cmd - if retcode is None: - proc.terminate() - raise RuntimeError(cmd_str + ' process did not terminate') - if raise_err and retcode != 0: - raise RuntimeError('{0} returned code {1} with error {2}'.format( - cmd_str, retcode, err.decode('latin-1'))) - out = out.strip() - if as_str: - out = out.decode('latin-1') + try: + proc = subprocess.run( + cmd, + shell=not cmd_is_seq, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=as_str, + check=raise_err, + ) + except subprocess.CalledProcessError as exc: + raise RuntimeError( + f"{exc.cmd} returned code {exc.returncode} with error {exc.stderr}" + ) if not ret_err: - return out - err = err.strip() - if as_str: - err = err.decode('latin-1') - return out, err + return proc.stdout.strip() + return proc.stdout.strip(), proc.stderr.strip() def unique_by_index(sequence): @@ -174,10 +184,17 @@ def parse_install_name(line): ] -def _cmd_out_err(cmd): - # Run command, return stdout or stderr if stdout is empty - out, err = back_tick(cmd, ret_err=True) - out = err if not len(out) else out +def _cmd_out_err(cmd: Sequence[str]) -> List[str]: + """Run command, return stdout or stderr if stdout is empty.""" + proc = subprocess.run( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + ) + out = proc.stdout.strip() + if not out: + out = proc.stderr.strip() return out.split('\n') @@ -260,7 +277,9 @@ def get_install_id(filename): @ensure_writable -def set_install_name(filename, oldname, newname, ad_hoc_sign=True): +def set_install_name( + filename: str, oldname: str, newname: str, ad_hoc_sign: bool = True +) -> None: """ Set install name `oldname` to `newname` in library filename Parameters @@ -278,7 +297,12 @@ def set_install_name(filename, oldname, newname, ad_hoc_sign=True): if oldname not in names: raise InstallNameError('{0} not in install names for {1}'.format( oldname, filename)) - back_tick(['install_name_tool', '-change', oldname, newname, filename]) + subprocess.run( + ['install_name_tool', '-change', oldname, newname, filename], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True + ) if ad_hoc_sign: # ad hoc signature is represented by a dash # https://developer.apple.com/documentation/security/seccodesignatureflags/kseccodesignatureadhoc @@ -286,7 +310,7 @@ def set_install_name(filename, oldname, newname, ad_hoc_sign=True): @ensure_writable -def set_install_id(filename, install_id, ad_hoc_sign=True): +def set_install_id(filename: str, install_id: str, ad_hoc_sign: bool = True): """ Set install id for library named in `filename` Parameters @@ -304,7 +328,12 @@ def set_install_id(filename, install_id, ad_hoc_sign=True): """ if get_install_id(filename) is None: raise InstallNameError('{0} has no install id'.format(filename)) - back_tick(['install_name_tool', '-id', install_id, filename]) + subprocess.run( + ['install_name_tool', '-id', install_id, filename], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True + ) if ad_hoc_sign: replace_signature(filename, '-') @@ -373,7 +402,7 @@ def get_environment_variable_paths(): @ensure_writable -def add_rpath(filename, newpath, ad_hoc_sign=True): +def add_rpath(filename: str, newpath: str, ad_hoc_sign: bool = True) -> None: """ Add rpath `newpath` to library `filename` Parameters @@ -385,12 +414,17 @@ def add_rpath(filename, newpath, ad_hoc_sign=True): ad_hoc_sign : {True, False}, optional If True, sign file with ad-hoc signature """ - back_tick(['install_name_tool', '-add_rpath', newpath, filename]) + subprocess.run( + ['install_name_tool', '-add_rpath', newpath, filename], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True + ) if ad_hoc_sign: replace_signature(filename, '-') -def zip2dir(zip_fname, out_dir): +def zip2dir(zip_fname: str, out_dir: str) -> None: """ Extract `zip_fname` into output directory `out_dir` Parameters @@ -402,7 +436,12 @@ def zip2dir(zip_fname, out_dir): """ # Use unzip command rather than zipfile module to preserve permissions # http://bugs.python.org/issue15795 - back_tick(['unzip', '-o', '-d', out_dir, zip_fname]) + subprocess.run( + ['unzip', '-o', '-d', out_dir, zip_fname], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True + ) def dir2zip(in_dir, zip_fname): @@ -489,7 +528,7 @@ def cmp_contents(filename1, filename2): return contents1 == contents2 -def get_archs(libname): +def get_archs(libname: str) -> FrozenSet[str]: """ Return architecture types from library `libname` Parameters @@ -506,8 +545,15 @@ def get_archs(libname): if not exists(libname): raise RuntimeError(libname + " is not a file") try: - stdout = back_tick(['lipo', '-info', libname]) - except RuntimeError: + lipo = subprocess.run( + ['lipo', '-info', libname], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + check=True, + ) + stdout = lipo.stdout.strip() + except subprocess.CalledProcessError: return frozenset() lines = [line.strip() for line in stdout.split('\n') if line.strip()] # For some reason, output from lipo -info on .a file generates this line @@ -522,15 +568,16 @@ def get_archs(libname): re.escape(libname) ) ): - reggie = re.compile(reggie) - match = reggie.match(line) + match = re.match(reggie, line) if match is not None: return frozenset(match.groups()[0].split(' ')) raise ValueError("Unexpected output: '{0}' for {1}".format( stdout, libname)) -def lipo_fuse(in_fname1, in_fname2, out_fname, ad_hoc_sign=True): +def lipo_fuse( + in_fname1: str, in_fname2: str, out_fname: str, ad_hoc_sign: bool = True +) -> str: """ Use lipo to merge libs `filename1`, `filename2`, store in `out_fname` Parameters @@ -543,17 +590,29 @@ def lipo_fuse(in_fname1, in_fname2, out_fname, ad_hoc_sign=True): filename to which to write new fused library ad_hoc_sign : {True, False}, optional If True, sign file with ad-hoc signature + + Raises + ------ + RuntimeError + If the lipo command exits with an error. """ - out = back_tick(['lipo', '-create', - in_fname1, in_fname2, - '-output', out_fname]) + try: + lipo = subprocess.run( + ['lipo', '-create', in_fname1, in_fname2, '-output', out_fname], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True, + universal_newlines=True, + ) + except subprocess.CalledProcessError as exc: + raise RuntimeError(f"Command {exc.cmd} failed with error: {exc.stderr}") if ad_hoc_sign: replace_signature(out_fname, '-') - return out + return lipo.stdout.strip() @ensure_writable -def replace_signature(filename, identity): +def replace_signature(filename: str, identity: str) -> None: """ Replace the signature of a binary file using `identity` See the codesign documentation for more info @@ -565,11 +624,15 @@ def replace_signature(filename, identity): identity : str The signing identity to use. """ - back_tick(['codesign', '--force', '--sign', identity, filename], - raise_err=True) + subprocess.run( + ['codesign', '--force', '--sign', identity, filename], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=True + ) -def validate_signature(filename): +def validate_signature(filename: str) -> None: """ Remove invalid signatures from a binary file If the file signature is missing or valid then it will be ignored @@ -582,11 +645,15 @@ def validate_signature(filename): filename : str Filepath to a binary file """ - out, err = back_tick(['codesign', '--verify', filename], - ret_err=True, as_str=True, raise_err=False) - if not err: + codesign = subprocess.run( + ['codesign', '--verify', filename], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + universal_newlines=True, + ) + if not codesign.stderr: return # The existing signature is valid - if 'code object is not signed at all' in err: + if 'code object is not signed at all' in codesign.stderr: return # File has no signature, and adding a new one isn't necessary # This file's signature is invalid and needs to be replaced