matthew-brett / delocate

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

Fat binaries of static libraries are not supported in the wheel #229

Open hoisunng opened 6 days ago

hoisunng commented 6 days ago

Describe the bug The new check to ensure the minimum macOS version of all MachO generates an error when fat binaries of static libraries are included in the wheel.

Our python wheel includes JIT compilation support using LLVM. For this we need to bundle some static libraries in the wheel that are used by the JIT linker. One of these static libraries libclang_rt.osx.a is a fat binary containing multiple architectures (e.g. arm64 and arm64e). The other "simple" static libraries are correctly ignored.

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/delocate/cmd/delocate_wheel.py", line 131, in <module>
    main()
  File "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/delocate/cmd/delocate_wheel.py", line 116, in main
    copied = delocate_wheel(
             ^^^^^^^^^^^^^^^
  File "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/delocate/delocating.py", line 1090, in delocate_wheel
    out_wheel_fixed = _check_and_update_wheel_name(
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/aictx/.pyenv/versions/3.12.6/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 "/Users/aictx/.pyenv/versions/3.12.6/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 "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/delocate/delocating.py", line 619, in _get_macos_min_version
    for header in MachO(dylib_path).headers:
                  ^^^^^^^^^^^^^^^^^
  File "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/macholib/MachO.py", line 121, in __init__
    self.load(fp)
  File "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/macholib/MachO.py", line 131, in load
    self.load_fat(fh)
  File "/Users/aictx/.pyenv/versions/3.12.6/lib/python3.12/site-packages/macholib/MachO.py", line 148, in load_fat
    self.load_header(fh, arch.offset, arch.size)
  File "/Users/aictx/.pyenv/versions/3.12.6/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/z_/1_rkwk513p75p8gsy53mjb8r0000gn/T/tmp77ji4upg/wheel/samna/libclang_rt.osx.a'>

To Reproduce Steps used to reproduce the behavior, such as the delocate commands used.

Expected behavior Delocating the wheel is successful, as in version 0.10.7.

Proposals:

  1. Only check the files that were touched by delocate.
  2. Add a parameter for files to ignore, the current filters don't influence the check.
  3. Add a parameter to skip the check.

Wheels used Unfortunately the wheel is private by my company.

Platform (please complete the following information):

Additional context My understanding of the problem is as follows. The _calculate_minimum_wheel_name looks at all the files in the wheel. It sees the static library and because it's a fat binary it contains the MachO magic number (0xCAFEBABF). Now it assumes that it's a dynamic library and looks for the magic number (0xFEEDFACF) for single binaries at the offsets given by the header. This magic number is missing because it's a static library, not a dynamic one.

njzjz commented 6 days ago

Got the same error here: https://github.com/njzjz/mdtraj/actions/runs/10816173115/job/30006767453

njzjz commented 6 days ago

Oh, it seems that it has been discussed in #227

HexDecimal commented 6 days ago

This is a duplicate of #227, but this issue was better researched and written than the other one.

This check was specifically to figure out compatibility of the whole wheel, so it makes sense to check every file, even those not touched by Delocate.

macholib is not as reliable as I'd hoped. Does otool -l show the version requirements of static libraries? A better otool parser might solve this issue.

hoisunng commented 5 days ago

Sorry for opening a new issue, I didn't see that it was the same.

I ran otool -l on my static library and it gives ~19000 lines of output. Basically for every object file in the static library it prints such an entry:

./pypi/samna/libclang_rt.osx.a(outline_atomic_ldset8_3.S.o):
Load command 0
      cmd LC_SEGMENT_64
  cmdsize 232
  segname
   vmaddr 0x0000000000000000
   vmsize 0x0000000000000050
  fileoff 392
 filesize 80
  maxprot 0x00000007
 initprot 0x00000007
   nsects 2
    flags 0x0
Section
  sectname __text
   segname __TEXT
      addr 0x0000000000000000
      size 0x000000000000002c
    offset 392
     align 2^4 (16)
    reloff 472
    nreloc 2
     flags 0x80000400
 reserved1 0
 reserved2 0
Section
  sectname __compact_unwind
   segname __LD
      addr 0x0000000000000030
      size 0x0000000000000020
    offset 440
     align 2^3 (8)
    reloff 488
    nreloc 1
     flags 0x02000000
 reserved1 0
 reserved2 0
Load command 1
      cmd LC_BUILD_VERSION
  cmdsize 24
 platform 1
    minos 11.0
      sdk n/a
   ntools 0
Load command 2
     cmd LC_SYMTAB
 cmdsize 24
  symoff 496
   nsyms 4
  stroff 560
 strsize 64
Load command 3
            cmd LC_DYSYMTAB
        cmdsize 80
      ilocalsym 0
      nlocalsym 2
     iextdefsym 2
     nextdefsym 1
      iundefsym 3
      nundefsym 1
         tocoff 0
           ntoc 0
      modtaboff 0
        nmodtab 0
   extrefsymoff 0
    nextrefsyms 0
 indirectsymoff 0
  nindirectsyms 0
      extreloff 0
        nextrel 0
      locreloff 0
        nlocrel 0

Of interest is probably minos in the LC_BUILD_VERSION load command.

Note that otool by default only prints information for the host architecture, so if all object files should be checked we should parse the output of otool -arch all -l.

HexDecimal commented 5 days ago

It'd just need to replace _get_macos_min_version with a new implementation parsing otool instead of using macholib:

https://github.com/matthew-brett/delocate/blob/b0c37814ae51a0f8ccef0678b4d61173f9a59919/delocate/delocating.py#L602

I was hoping that there might be some easy way to parse the output into a Python object. So that it can be used as easily as macholib, but I guess it will have to be parsed more manually.

Of interest is probably minos in the LC_BUILD_VERSION load command.

Sometimes it's also version from the LC_VERSION_MIN_MACOSX command.

Note that otool by default only prints information for the host architecture, so if all object files should be checked we should parse the output of otool -arch all -l.

When you use -arch all will it always show the architecture in the output alongside the filename? Such as on the first line? This flag is missing from the rpaths parser.

hoisunng commented 1 day ago

Command otool -arch all -l libclang_rt.osx.a | head -n 100

Archive : libclang_rt.osx.a (architecture arm64)
libclang_rt.osx.a(comparetf2.c.o) (architecture arm64):
Load command 0
      cmd LC_SEGMENT_64
  cmdsize 152
  segname
   vmaddr 0x0000000000000000
   vmsize 0x0000000000000000
  fileoff 312
 filesize 0
  maxprot 0x00000007
 initprot 0x00000007
   nsects 1
    flags 0x0
Section
  sectname __text
   segname __TEXT
      addr 0x0000000000000000
      size 0x0000000000000000
    offset 312
     align 2^0 (1)
    reloff 0
    nreloc 0
     flags 0x80000000
 reserved1 0
 reserved2 0
Load command 1
      cmd LC_BUILD_VERSION
  cmdsize 24
 platform 1
    minos 11.0
      sdk 12.3
   ntools 0
Load command 2
     cmd LC_SYMTAB
 cmdsize 24
  symoff 312
   nsyms 1
  stroff 328
 strsize 8
Load command 3
            cmd LC_DYSYMTAB
        cmdsize 80
      ilocalsym 0
      nlocalsym 1
     iextdefsym 1
     nextdefsym 0
      iundefsym 1
      nundefsym 0
         tocoff 0
           ntoc 0
      modtaboff 0
        nmodtab 0
   extrefsymoff 0
    nextrefsyms 0
 indirectsymoff 0
  nindirectsyms 0
      extreloff 0
        nextrel 0
      locreloff 0
        nlocrel 0
libclang_rt.osx.a(divtc3.c.o) (architecture arm64):
Load command 0
      cmd LC_SEGMENT_64
  cmdsize 312
  segname
   vmaddr 0x0000000000000000
   vmsize 0x00000000000002e8
  fileoff 472
 filesize 744
  maxprot 0x00000007
 initprot 0x00000007
   nsects 3
    flags 0x0
Section
  sectname __text
   segname __TEXT
      addr 0x0000000000000000
      size 0x000000000000027c
    offset 472
     align 2^2 (4)
    reloff 1216
    nreloc 5
     flags 0x80000400
 reserved1 0
 reserved2 0
Section
  sectname __compact_unwind
   segname __LD
      addr 0x0000000000000280
      size 0x0000000000000020
    offset 1112
     align 2^3 (8)
    reloff 1256
    nreloc 1
     flags 0x02000000
 reserved1 0
 reserved2 0
Section
  sectname __eh_frame