Merge pull request #216 from jameslamb/fix/error-message

Fix string formatting in DelocationError message

Changelog.md

@@ -21,6 +21,12 @@ rules on making a good Changelog.
   version to `delocate==0.11.0`.
   [#215](https://github.com/matthew-brett/delocate/pull/215)
 
+### Fixed
+
+- Existing libraries causing DelocationError were not shown due to bad string
+  formatting.
+  [#216](https://github.com/matthew-brett/delocate/pull/216)
+
 ## [0.11.0] - 2024-03-22
 
 ### Added

delocate/delocating.py

@@ -992,7 +992,7 @@ def delocate_wheel(
         )
         if copied_libs and lib_path_exists_before_delocate:
             raise DelocationError(
-                "f{lib_path} already exists in wheel but need to copy "
+                f"{lib_path} already exists in wheel but need to copy "
                 + "; ".join(copied_libs)
             )
         if len(os.listdir(lib_path)) == 0:

delocate/tests/test_delocating.py

@@ -114,7 +114,9 @@ def test_delocate_tree_libs(
         sys_lib = EXT_LIBS[0]
         lib_dict = without_system_libs(tree_libs_func(subtree))
         lib_dict.update({"/unlikely/libname.dylib": {}})
-        with pytest.raises(DelocationError):
+        with pytest.raises(
+            DelocationError, match=r".*/unlikely/libname.dylib.*does not exist"
+        ):
             delocate_tree_libs(lib_dict, copy_dir, subtree)
 
         lib_dict = without_system_libs(tree_libs_func(subtree))
@@ -171,7 +173,11 @@ def test_delocate_tree_libs(
         # out-of-tree will raise an error because of duplicate library names
         # (libc and slibc both named <something>/libc.dylib)
         lib_dict2 = without_system_libs(tree_libs_func(subtree2))
-        with pytest.raises(DelocationError):
+        with pytest.raises(
+            DelocationError,
+            match=r"Already planning to copy library with same basename as: "
+            r"libc.dylib",
+        ):
             delocate_tree_libs(lib_dict2, copy_dir2, "/fictional")
         # Rename a library to make this work
         new_slibc = pjoin(dirname(slibc), "libc2.dylib")
@@ -365,7 +371,10 @@ def filt_func(libname: Text) -> bool:
         shutil.copy2(libb, "subtree")
         # If liba is already present, barf
         shutil.copy2(liba, "subtree")
-        assert_raises(DelocationError, copy_recurse, "subtree", filt_func)
+        with pytest.raises(
+            DelocationError, match=r".*liba.dylib .*already exists"
+        ):
+            copy_recurse("subtree", filt_func)
         # Works if liba not present
         os.unlink(pjoin("subtree", "liba.dylib"))
         copy_recurse("subtree", filt_func)

delocate/tests/test_libsana.py

@@ -463,7 +463,9 @@ def test_wheel_libs_ignore_missing() -> None:
     # Test wheel_libs ignore_missing parameter.
     with InTemporaryDirectory() as tmpdir:
         shutil.copy(RPATH_WHEEL, pjoin(tmpdir, "rpath.whl"))
-        with pytest.raises(DelocationError):
+        with pytest.raises(
+            DelocationError, match=r"Could not find all dependencies."
+        ):
             wheel_libs("rpath.whl")
         wheel_libs("rpath.whl", ignore_missing=True)
 

delocate/tests/test_wheelies.py

@@ -34,7 +34,7 @@
 )
 from ..wheeltools import InWheel
 from .env_tools import _scope_env
-from .pytest_tools import assert_equal, assert_false, assert_raises, assert_true
+from .pytest_tools import assert_equal, assert_false, assert_true
 from .test_install_names import DATA_PATH, EXT_LIBS
 from .test_tools import ARCH_BOTH, ARCH_M1
 
@@ -153,7 +153,12 @@ def test_fix_plat() -> None:
         assert exists(dylibs)
         assert os.listdir(dylibs) == ["libextfunc.dylib"]
         # Test check for existing output directory
-        with pytest.raises(DelocationError):
+        with pytest.raises(
+            DelocationError,
+            match=r".*wheel/fakepkg1/subpkg "
+            r"already exists in wheel but need to copy "
+            r".*libextfunc.dylib",
+        ):
             delocate_wheel(fixed_wheel, "broken_wheel.ext", "subpkg")
         # Test that `wheel unpack` works
         fixed_wheel, stray_lib = _fixed_wheel(tmpdir)
@@ -301,9 +306,8 @@ def _fix_break_fix(arch_):
             )
             # Now we check, and error raised
             _fix_break(arch)
-            assert_raises(
-                DelocationError, delocate_wheel, fixed_wheel, require_archs=()
-            )
+            with pytest.raises(DelocationError, match=r".*(x86_64|arm64)"):
+                delocate_wheel(fixed_wheel, require_archs=())
             # We can fix again by thinning the module too
             fixed_wheel2 = _fix_break_fix(arch)
             assert_equal(
@@ -317,21 +321,17 @@ def _fix_break_fix(arch_):
                 ARCH_BOTH.difference([arch]),
             ):
                 fixed_wheel3 = _fix_break_fix(arch)
-                assert_raises(
-                    DelocationError,
-                    delocate_wheel,
-                    fixed_wheel3,
-                    require_archs=req_arch,
-                )
+                with pytest.raises(DelocationError, match=r".*(x86_64|arm64)"):
+                    delocate_wheel(fixed_wheel3, require_archs=req_arch)
         # Can be verbose (we won't check output though)
         _fix_break("x86_64")
-        assert_raises(
+        with pytest.raises(
             DelocationError,
-            delocate_wheel,
-            fixed_wheel,
-            require_archs=(),
-            check_verbose=True,
-        )
+            match=r".*missing architectures in wheel\n"
+            r"fakepkg1/subpkg/module2.abi3.so needs arch arm64 missing from "
+            r".*/libextfunc.dylib",
+        ):
+            delocate_wheel(fixed_wheel, require_archs=(), check_verbose=True)
 
 
 def test_patch_wheel() -> None: