matthew-brett / delocate

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

delocate-merge does not merge Qt Framework binaries #228

Open MAKOMO opened 2 months ago

MAKOMO commented 2 months ago

Describe the bug While delocate-merge succeeds to merge PyQt6-Qt6 arm and x86 wheels the resulting wheel contains Qt Frameworks that are not universal2.

To Reproduce

# delocate-merge PyQt6_Qt6-6.7.2-py3-none-macosx_10_14_x86_64.whl PyQt6_Qt6-6.7.2-py3-none-macosx_11_0_arm64.whl -w .
# unzip PyQt6_Qt6-6.7.2-py3-none-macosx_11_0_universal2.whl.zip
# cd PyQt6/Qt6/lib/QtCore.framework/Versions/A
# file QtCore
QtCore: Mach-O 64-bit dynamically linked shared library arm64

Expected behavior A full universal2 wheel.

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/7e/9d/517b12a42b0692c909ed348545114dae7d0b4014ef9075e18f6bf48834a1/PyQt6_Qt6-6.7.2-py3-none-macosx_11_0_arm64.whl

https://files.pythonhosted.org/packages/10/38/ba0313442c5e4327d52e6c48d2bb4b39099bf1d191bd872edfd8bb1392ef/PyQt6_Qt6-6.7.2-py3-none-macosx_10_14_x86_64.whl

Platform (please complete the following information):

Additional context

Only files with the extensions

lib_exts=(".so", ".dylib", ".a")

are fused by fuse_trees but this Qt Framework binary has no extension at all.

Patching fuse_trees as follows works for those Qt packages, but is not be general enough to catch other cases.

def fuse_trees(to_tree, from_tree, lib_exts=(".so", ".dylib", ".a")):
...
        for fname in filenames:
            root, ext = splitext(fname)
            from_path = pjoin(from_dirpath, fname)
            to_path = pjoin(to_dirpath, fname)
            if not exists(to_path):
                _copyfile(from_path, to_path)
            elif cmp_contents(from_path, to_path):
                pass
            elif ext in lib_exts or fname.startswith('Qt'):
                # existing lib that needs fuse
                lipo_fuse(from_path, to_path, to_path)
...
HexDecimal commented 2 months ago

The lib_exts parameter isn't even used by the caller. This could be replaced with a more reliable filter than the file suffix.

HexDecimal commented 2 months ago

When I check to see what the main analysis function uses, _is_macho_file shows up again. Could probably use that here.

justvanrossum commented 1 month ago

I can confirm that using _is_macho_file() fixes the problem.