Merge pull request #126 from HexDecimal/subprocess-run

MRG: Deprecate and replace calls to back_tick.

Related to #125

The non-specific timeout check in `back_tick` has been removed.  It was that or provide a specific timeout time.

All internal functions and tests now use `subprocess.run`.  This might change some of the exceptions raised by those functions except for `lipo_fuse` which was specifically tested to raise `RuntimeError`.

Relevant tests have been cleaned up.

Relevant functions have had type annotations added/updated.

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)
 

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'))

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():

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')

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,31 +272,33 @@ 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
         out_fname = basename(PURE_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():

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():