Merge pull request #235 from HexDecimal/fix-static-libs

Ignore unparseable static libraries

Changelog.md

@@ -33,6 +33,9 @@ rules on making a good Changelog.
   [#230](https://github.com/matthew-brett/delocate/pull/230)
 - `delocate-merge` now supports libraries with missing or unusual extensions.
   [#228](https://github.com/matthew-brett/delocate/issues/228)
+- Now supports library files ending in parentheses.
+- Fixed `Unknown Mach-O header` error when encountering a fat static library.
+  [#229](https://github.com/matthew-brett/delocate/issues/229)
 
 ### Removed
 

delocate/delocating.py

@@ -27,6 +27,7 @@
 from macholib.MachO import MachO  # type: ignore[import-untyped]
 from packaging.utils import parse_wheel_filename
 from packaging.version import Version
+from typing_extensions import deprecated
 
 from .libsana import (
     DelocationError,
@@ -39,7 +40,6 @@
 from .pkginfo import read_pkg_info, write_pkg_info
 from .tmpdirs import TemporaryDirectory
 from .tools import (
-    _is_macho_file,
     _remove_absolute_rpaths,
     dir2zip,
     find_package_dirs,
@@ -249,6 +249,7 @@ def _update_install_names(
                 set_install_name(requiring, orig_install_name, new_install_name)
 
 
+@deprecated("copy_recurse is obsolete and should no longer be called")
 def copy_recurse(
     lib_path: str,
     copy_filt_func: Callable[[str], bool] | None = None,
@@ -291,11 +292,6 @@ def copy_recurse(
         This function is obsolete.  :func:`delocate_path` handles recursive
         dependencies while also supporting `@loader_path`.
     """
-    warnings.warn(
-        "copy_recurse is obsolete and should no longer be called.",
-        DeprecationWarning,
-        stacklevel=2,
-    )
     if copied_libs is None:
         copied_libs = {}
     else:
@@ -587,12 +583,14 @@ def _make_install_name_ids_unique(
         validate_signature(lib)
 
 
-def _get_macos_min_version(dylib_path: Path) -> Iterator[tuple[str, Version]]:
+def _get_macos_min_version(
+    dylib_path: str | os.PathLike[str],
+) -> Iterator[tuple[str, Version]]:
     """Get the minimum macOS version from a dylib file.
 
     Parameters
     ----------
-    dylib_path : Path
+    dylib_path : str or PathLike
         The path to the dylib file.
 
     Yields
@@ -602,9 +600,16 @@ def _get_macos_min_version(dylib_path: Path) -> Iterator[tuple[str, Version]]:
     Version
         The minimum macOS version.
     """
-    if not _is_macho_file(dylib_path):
-        return
-    for header in MachO(dylib_path).headers:
+    try:
+        macho = MachO(dylib_path)
+    except ValueError as exc:
+        if str(exc.args[0]).startswith(
+            ("Unknown fat header magic", "Unknown Mach-O header")
+        ):
+            return  # Not a recognised Mach-O object file
+        raise  # pragma: no cover  # Unexpected error
+
+    for header in macho.headers:
         for cmd in header.commands:
             if cmd[0].cmd == LC_BUILD_VERSION:
                 version = cmd[1].minos
@@ -801,6 +806,8 @@ def _calculate_minimum_wheel_name(
     all_library_versions: dict[str, dict[Version, list[Path]]] = {}
 
     for lib in wheel_dir.glob("**/*"):
+        if lib.is_dir():
+            continue
         for arch, version in _get_macos_min_version(lib):
             all_library_versions.setdefault(arch.lower(), {}).setdefault(
                 version, []

delocate/libsana.py

@@ -399,6 +399,7 @@ def _allow_all(path: str) -> bool:
     return True
 
 
+@deprecated("tree_libs doesn't support @loader_path and has been deprecated")
 def tree_libs(
     start_path: str,
     filt_func: Callable[[str], bool] | None = None,
@@ -443,11 +444,6 @@ def tree_libs(
 
         :func:`tree_libs_from_directory` should be used instead.
     """
-    warnings.warn(
-        "tree_libs doesn't support @loader_path and has been deprecated.",
-        DeprecationWarning,
-        stacklevel=2,
-    )
     if filt_func is None:
         filt_func = _allow_all
     lib_dict: dict[str, dict[str, str]] = {}
@@ -559,7 +555,10 @@ def resolve_dynamic_paths(
     raise DependencyNotFound(lib_path)
 
 
-@deprecated("This function was replaced by resolve_dynamic_paths")
+@deprecated(
+    "This function doesn't support @loader_path "
+    "and was replaced by resolve_dynamic_paths"
+)
 def resolve_rpath(lib_path: str, rpaths: Iterable[str]) -> str:
     """Return `lib_path` with its `@rpath` resolved.
 
@@ -584,12 +583,6 @@ def resolve_rpath(lib_path: str, rpaths: Iterable[str]) -> str:
         This function does not support `@loader_path`.
         Use `resolve_dynamic_paths` instead.
     """
-    warnings.warn(
-        "resolve_rpath doesn't support @loader_path and has been deprecated."
-        "  Switch to using `resolve_dynamic_paths` instead.",
-        DeprecationWarning,
-        stacklevel=2,
-    )
     if not lib_path.startswith("@rpath/"):
         return lib_path
 

delocate/tests/test_delocating.py

@@ -1,5 +1,7 @@
 """Tests for relocating libraries."""
 
+from __future__ import annotations
+
 import os
 import shutil
 import subprocess
@@ -8,6 +10,7 @@
 from collections.abc import Iterable
 from os.path import basename, dirname, realpath, relpath, splitext
 from os.path import join as pjoin
+from pathlib import Path
 from typing import Any, Callable
 
 import pytest
@@ -16,6 +19,7 @@
 
 from ..delocating import (
     _get_archs_and_version_from_wheel_name,
+    _get_macos_min_version,
     bads_report,
     check_archs,
     copy_recurse,
@@ -33,7 +37,18 @@
 from ..tools import get_install_names, set_install_name
 from .env_tools import TempDirWithoutEnvVars
 from .pytest_tools import assert_equal, assert_raises
-from .test_install_names import EXT_LIBS, LIBA, LIBB, LIBC, TEST_LIB, _copy_libs
+from .test_install_names import (
+    A_OBJECT,
+    DATA_PATH,
+    EXT_LIBS,
+    ICO_FILE,
+    LIBA,
+    LIBA_STATIC,
+    LIBB,
+    LIBC,
+    TEST_LIB,
+    _copy_libs,
+)
 from .test_tools import (
     ARCH_32,
     ARCH_64,
@@ -747,3 +762,28 @@ def test_get_archs_and_version_from_wheel_name() -> None:
         _get_archs_and_version_from_wheel_name(
             "foo-1.0-py310-abi3-manylinux1.whl"
         )
+
+
+@pytest.mark.parametrize(
+    "file,expected_min_version",
+    [
+        # Dylib files
+        (LIBBOTH, {"ARM64": Version("11.0"), "x86_64": Version("10.9")}),
+        (LIBA, {"x86_64": Version("10.9")}),
+        # Shared objects
+        (
+            Path(DATA_PATH, "np-1.6.0_intel_lib__compiled_base.so"),
+            {"i386": Version("10.6"), "x86_64": Version("10.6")},
+        ),
+        # Object file
+        (A_OBJECT, {"x86_64": Version("10.9")}),
+        # Static file
+        (LIBA_STATIC, {}),
+        # Non library
+        (ICO_FILE, {}),
+    ],
+)
+def test_get_macos_min_version(
+    file: str | Path, expected_min_version: dict[str, Version]
+) -> None:
+    assert dict(_get_macos_min_version(file)) == expected_min_version

delocate/tests/test_install_names.py

@@ -9,6 +9,7 @@
 from collections.abc import Sequence
 from os.path import basename, dirname, exists
 from os.path import join as pjoin
+from pathlib import Path
 from subprocess import CompletedProcess
 from typing import (
     NamedTuple,
@@ -20,6 +21,7 @@
 from ..tmpdirs import InTemporaryDirectory
 from ..tools import (
     InstallNameError,
+    _get_install_ids,
     add_rpath,
     get_environment_variable_paths,
     get_install_id,
@@ -104,15 +106,30 @@ def test_parse_install_name():
 
 
 @pytest.mark.xfail(sys.platform != "darwin", reason="otool")
-def test_install_id():
+def test_install_id() -> None:
     # Test basic otool library listing
-    assert_equal(get_install_id(LIBA), "liba.dylib")
-    assert_equal(get_install_id(LIBB), "libb.dylib")
-    assert_equal(get_install_id(LIBC), "libc.dylib")
-    assert_equal(get_install_id(TEST_LIB), None)
+    assert get_install_id(LIBA) == "liba.dylib"
+    assert get_install_id(LIBB) == "libb.dylib"
+    assert get_install_id(LIBC) == "libc.dylib"
+    assert get_install_id(TEST_LIB) is None
     # Non-object file returns None too
-    assert_equal(get_install_id(__file__), None)
-    assert_equal(get_install_id(ICO_FILE), None)
+    assert get_install_id(__file__) is None
+    assert get_install_id(ICO_FILE) is None
+
+
+@pytest.mark.xfail(sys.platform != "darwin", reason="otool")
+def test_install_ids(tmp_path: Path) -> None:
+    # Test basic otool library listing
+    assert _get_install_ids(LIBA) == {"": "liba.dylib"}
+    assert _get_install_ids(LIBB) == {"": "libb.dylib"}
+    assert _get_install_ids(LIBC) == {"": "libc.dylib"}
+    assert _get_install_ids(TEST_LIB) == {}
+    # Non-object file returns None too
+    assert _get_install_ids(__file__) == {}
+    assert _get_install_ids(ICO_FILE) == {}
+    # Should work ending with parentheses
+    shutil.copy(LIBA, tmp_path / "liba(test)")
+    assert _get_install_ids(tmp_path / "liba(test)") == {"": "liba.dylib"}
 
 
 @pytest.mark.xfail(sys.platform != "darwin", reason="otool")
@@ -228,6 +245,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-L",
                     "example.so",
                 ): """\
@@ -240,6 +258,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-D",
                     "example.so",
                 ): """\
@@ -250,6 +269,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-l",
                     "example.so",
                 ): """\
@@ -271,6 +291,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-L",
                     "example.so",
                 ): """\
@@ -287,6 +308,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-D",
                     "example.so",
                 ): """\
@@ -299,6 +321,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-l",
                     "example.so",
                 ): """\
@@ -324,6 +347,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-L",
                     "example.so",
                 ): """\
@@ -340,6 +364,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-D",
                     "example.so",
                 ): """\
@@ -352,6 +377,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-l",
                     "example.so",
                 ): """\
@@ -377,6 +403,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-L",
                     "example.so",
                 ): """\
@@ -393,6 +420,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-D",
                     "example.so",
                 ): """\
@@ -405,6 +433,7 @@ def mock_subprocess_run(
                     "otool",
                     "-arch",
                     "all",
+                    "-m",
                     "-l",
                     "example.so",
                 ): """\