Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

delocate-merge fails to merge numpy #227

Open
MAKOMO opened this issue Sep 7, 2024 · 4 comments
Open

delocate-merge fails to merge numpy #227

MAKOMO opened this issue Sep 7, 2024 · 4 comments
Labels

Comments

@MAKOMO
Copy link

MAKOMO commented Sep 7, 2024

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

  • OS version: macOS 14.6.1
  • Delocate version: 0.12.0
@MAKOMO MAKOMO added the bug label Sep 7, 2024
@HexDecimal
Copy link
Collaborator

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
Copy link
Author

MAKOMO commented Sep 7, 2024

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
Copy link
Collaborator

_is_macho_file performs a very simple magic number check so I wonder how easily this can be a false positive.

def _is_macho_file(filename: str | os.PathLike[str]) -> bool:

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.

jonathanperret added a commit to jonathanperret/ayab-desktop that referenced this issue Sep 8, 2024
Ideally we'd upgrade to delocate 0.12.0 and use the new delocate-merge command, but
it fails on numpy (matthew-brett/delocate#227), so we'll stick to
0.11.0 for now.
jonathanperret added a commit to jonathanperret/ayab-desktop that referenced this issue Sep 8, 2024
Ideally we'd upgrade to delocate 0.12.0 and use the new delocate-merge command, but
it fails on numpy (matthew-brett/delocate#227), so we'll stick to
0.11.0 for now.
dl1com pushed a commit to AllYarnsAreBeautiful/ayab-desktop that referenced this issue Sep 9, 2024
Ideally we'd upgrade to delocate 0.12.0 and use the new delocate-merge command, but
it fails on numpy (matthew-brett/delocate#227), so we'll stick to
0.11.0 for now.
@HexDecimal
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants