Fix for bigsur

[Czaki]

Fix for issue #385

@fxcoudert @ronaldoussoren could you test it?

This PR still does not have tests for extract_macosx_min_system_version function. To add such a test I need some files compiled for the BigSur system.

[fxcoudert]

@Czaki what information do you need?

>>> a.extract_macosx_min_system_version("/usr/local/opt/openconnect/lib/libopenconnect.5.dylib")
(11, 0, 0)

Do you need a dylib compiled for Big Sur? With any specific settings?

[Czaki]

@Czaki what information do you need?

>>> a.extract_macosx_min_system_version("/usr/local/opt/openconnect/lib/libopenconnect.5.dylib")
(11, 0, 0)

Do you need a dylib compiled for Big Sur? With any specific settings?

I would like to have such a file to add it to tests/testdata/macosx_minimal_system_version.
Best it will be to have also file for version 11.1 to check if it is properly changed to 11.0.

But from your response, I see that they not change binary representation.

[fxcoudert]

$ cat toto.c 
int foo(int x) { return x >= 0; }
$ clang -shared -fPIC toto.c -o libtoto.dylib

The file produced is available (for a couple of days) at: https://filesender.renater.fr/?s=download&token=f2be417a-d8f1-40c6-a984-fb89fc679426

It's produced on Big Sur 11.0.1 with Xcode 12.2. It has:

Load command 7
       cmd LC_BUILD_VERSION
   cmdsize 32
  platform 1
       sdk 11.0
     minos 11.0
    ntools 1
      tool 3
   version 609.7
Load command 8
      cmd LC_SOURCE_VERSION
  cmdsize 16
  version 0.0

[fxcoudert]

A file compiled on macOS 11.1 (beta) with Xcode 12.3 (beta) and the 11.1 (beta) SDK has:

      cmd LC_BUILD_VERSION
  cmdsize 32
 platform 1
    minos 11.0
      sdk 11.1
   ntools 1
     tool 3
  version 609.8
Load command 8
      cmd LC_SOURCE_VERSION
  cmdsize 16
  version 0.0

You will find 3 files compiled on that platform here: one for arm, one of Intel, one fat.

% ls libtoto_*
libtoto_arm64.dylib	libtoto_fat.dylib	libtoto_x86_64.dylib
% file libtoto_*                                                        
libtoto_arm64.dylib:  Mach-O 64-bit dynamically linked shared library arm64
libtoto_fat.dylib:    Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit dynamically linked shared library x86_64] [arm64:Mach-O 64-bit dynamically linked shared library arm64]
libtoto_fat.dylib (for architecture x86_64):	Mach-O 64-bit dynamically linked shared library x86_64
libtoto_fat.dylib (for architecture arm64):	Mach-O 64-bit dynamically linked shared library arm64
libtoto_x86_64.dylib: Mach-O 64-bit dynamically linked shared library x86_64

There are available here: https://filesender.renater.fr/?s=download&token=d22c471a-7db4-4094-985c-d9dcafc1ce63

I hope that helps. If you have more specific requirements, please let me know.

[Czaki]

if fat is now selected as architecture for macos x86_64 and arm packed wheel? Up to this moment, wheel does not support arm detection from wheel.

[ronaldoussoren]

See also #387.

[ronaldoussoren]

if fat is now selected as architecture for macos x86_64 and arm packed wheel? Up to this moment, wheel does not support arm detection from wheel.

arm64 is already handled correctly by wheel/macosx_libfile.py because that code doesn't look at the CPU architecture but does handle fat binaries.

[Czaki]

arm64 is already handled correctly by wheel/macosx_libfile.py because that code doesn't look at the CPU architecture but does handle fat binaries.

Ok. I was work on this in matthew-brett/delocate#61 and matthew-brett/delocate#68. But it was not finished because of a lack of macOS knowledge and access to macOS hardware with different systems.

But I think that at this moment it should be added. If someone installs universal python but builds libraries only for arm it should not have a universal platform tag.

[ronaldoussoren]

I guess I miscommunicated, its getting late for me. Wheel already handles "universal2" binaries, except for the issue with the deployment target described in #387.

To demonstrate: https://pypi.org/project/pyobjc-core/#files. The latest release includes a "universal2" wheel that worked out of the box once the PR for macOS 11 was merged.

[Czaki]

I guess I miscommunicated, its getting late for me. Wheel already handles "universal2" binaries, except for the issue with the deployment target described in #387.

You can not calculate the minimum version per files. It is bad idea. Calculate maximum per file for given architecture and minimum per architecture seams good.

[agronholm]

Is this PR ready for merging? If not, mark it as a draft PR.

[Czaki]

@agronholm For me it is ready but @ronaldoussoren has some requests. Merging this will allow creating bugfix release which fixes build on macOS 11 bigsur.

But if you prefer to wait on a full fix (where the decision about calculates the platform tag needs to be done) I could change this to draft.

[agronholm]

How long do you think the full fix would take?

[Czaki]

I do not know. It depends on how long will take a discussion.
I think that the wheel should work on every machine that satisfies the platform tag. @ronaldoussoren suggest that wheel should inform about the minimal version of the system that has a chance to run (so it is possible that after successful pip install code will not work, which produce hard to debug errors, but some peoples could not need to install from sources).

I rather prefer to create more wheel than @ronaldoussoren approach.

[ronaldoussoren]

Wheels can already not work, I recently installed a wheel that required me to install homebrew and a particular homebrew package. IMHO that's not something this package should try to protect against.

My questions are in a separate issue, IMHO that discussion is not blocking for this PR.

[Czaki]

Wheels can already not work, I recently installed a wheel that required me to install homebrew and a particular homebrew package. IMHO that's not something this package should try to protect against.

The question is how this bug is handled. If it is an ugly long stack trace or a simple message prepared by the package maintainer.

@agronholm I suggest merging it and create a bugfix release. Then there be the next PR for the final solution.

[agronholm]

Thanks.