matthew-brett / delocate

Find and copy needed dynamic libraries into python wheels
BSD 2-Clause "Simplified" License
267 stars 59 forks source link

delocate-merge fails to merge numpy #227

Open MAKOMO opened 1 month ago

MAKOMO commented 1 month ago

Describe the bug A ValueError is thrown by macholib on trying to get the determine the macOS min version on applying delocate-merge to numpy 2.1.1 arm & x86_64 wheels downloaded from PyPip for macOS

To Reproduce

# delocate-merge numpy-2.1.1-cp312-cp312-macosx_10_9_x86_64.whl numpy-2.1.1-cp312-cp312-macosx_11_0_arm64.whl -w .
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.12/bin/delocate-merge", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/delocate/cmd/delocate_merge.py", line 39, in main
    fuse_wheels(wheel1, wheel2, out_wheel)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/delocate/fuse.py", line 170, in fuse_wheels
    out_wheel_name = _retag_wheel(to_wheel, from_wheel, to_wheel_dir)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/delocate/fuse.py", line 76, in _retag_wheel
    retag_name = _check_and_update_wheel_name(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/delocate/delocating.py", line 914, in _check_and_update_wheel_name
    new_name, problematic_files = _calculate_minimum_wheel_name(
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/delocate/delocating.py", line 816, in _calculate_minimum_wheel_name
    for arch, version in _get_macos_min_version(lib):
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/delocate/delocating.py", line 619, in _get_macos_min_version
    for header in MachO(dylib_path).headers:
                  ^^^^^^^^^^^^^^^^^
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/macholib/MachO.py", line 121, in __init__
    self.load(fp)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/macholib/MachO.py", line 131, in load
    self.load_fat(fh)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/macholib/MachO.py", line 148, in load_fat
    self.load_header(fh, arch.offset, arch.size)
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/macholib/MachO.py", line 170, in load_header
    raise ValueError("Unknown Mach-O header: 0x%08x in %r" % (header, fh))
ValueError: Unknown Mach-O header: 0x213c6172 in <_io.BufferedReader name='/private/var/folders/nz/h_yx09bn615395lst58hfn480000gn/T/tmppltawr0o/to_wheel/numpy/_core/lib/libnpymath.a'>

Expected behavior A successful merge

Wheels used If a wheel is involved then consider attaching the original wheel (before being delocated) or linking to the repository where the original wheel can be created.

https://files.pythonhosted.org/packages/6b/6c/a9fbef5fd2f9685212af2a9e47485cde9357c3e303e079ccf85127516f2d/numpy-2.1.1-cp312-cp312-macosx_11_0_arm64.whl

https://files.pythonhosted.org/packages/36/11/c573ef66c004f991989c2c6218229d9003164525549409aec5ec9afc0285/numpy-2.1.1-cp312-cp312-macosx_10_9_x86_64.whl

Platform (please complete the following information):

HexDecimal commented 1 month ago

Unsure what to do here. I don't have the tools to inspect these files. I might need to raise this issue upstream to ronaldoussoren/macholib.

In theory the native CLI otool could be used instead of macholib. _get_macos_min_version is small and could be rewritten by someone with experience.

MAKOMO commented 1 month ago

For now I added an except to cover this ValueError returning None. The reason for the troubles here might be that some of those numpy binaries are generated from Fortran code which might generate wired headers. However, those binary things are mostly an unknown field for me.

HexDecimal commented 1 month ago

_is_macho_file performs a very simple magic number check so I wonder how easily this can be a false positive. https://github.com/matthew-brett/delocate/blob/b0c37814ae51a0f8ccef0678b4d61173f9a59919/delocate/tools.py#L173

Could do what you did and rely on the exceptions from macholib instead of this _is_macho_file check, but I'm unsure about how reliable this would be. Would be too easy to suppress something important.

If the headers are unusual then that's definitely something I should report upstream.

HexDecimal commented 1 month ago

229 has identified the problem as being caused by fat static libraries. I'm likely to continue any discussion over there.