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

RLS/BLD: use native Apple Silicon macOS for arm64 wheels; remove universal2 #1990

Merged
merged 1 commit into from Feb 16, 2024

Conversation

mwtoews
Copy link
Member

@mwtoews mwtoews commented Feb 12, 2024

Use native Apple Silicon macOS runners (described here) to build arm64 wheels.

Also, remove universal2 wheels, since I think it's fine to have separate x86_64 builds for macOS.

@mwtoews
Copy link
Member Author

mwtoews commented Feb 12, 2024

Up to @jorisvandenbossche to decide about these changes for the upcoming maintenance release version 2.0.3 described in #1742

I'm working on a separate PR for main.

@jorisvandenbossche
Copy link
Member

It's still showing the same warning / not actually running tests:

Warning: While cibuildwheel can build CPython 3.8 universal2/arm64 wheels, we cannot test the arm64 part of them, even when running on an Apple Silicon machine. This is because we use the x86_64 installer of CPython 3.8. See the discussion in pypa/cibuildwheel#1169 for the details. To silence this warning, set CIBW_TEST_SKIP: "cp38-macosx_*:arm64".

@jorisvandenbossche
Copy link
Member

(I think it is also fine to directly target this PR to main, can still backport the eventual commit to 2.0.x if we want)

@mwtoews mwtoews changed the base branch from maint-2.0 to main February 13, 2024 08:55
@mwtoews
Copy link
Member Author

mwtoews commented Feb 13, 2024

Switched to main and attempted to silence the issue with cibuildwheel and CPython 3.8 universal2/arm64 wheels. Note that warning is a new one not shown before. The older warnings related to cross-compiling and testing are now gone.

@coveralls
Copy link

coveralls commented Feb 13, 2024

Pull Request Test Coverage Report for Build 7884170432

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on maint-2.0-macos-14 at 88.076%

Totals Coverage Status
Change from base Build 7874085015: 88.1%
Covered Lines: 2600
Relevant Lines: 2952

💛 - Coveralls

@mwtoews
Copy link
Member Author

mwtoews commented Feb 13, 2024

Warnings all cleared up now, ready for final review.

I should elaborate more on why to remove universal2. They are nearly twice the size, since they contain both x86_64 and arm64. E.g. with the shapely 2.0.2 files:

$ file universal2/shapely/_geos.cpython-39-darwin.so 
universal2/shapely/_geos.cpython-39-darwin.so: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit x86_64 bundle, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL>] [arm64:Mach-O 64-bit arm64 bundle, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL>]

which is cool, but there are individual wheels for each architecture, so pip should be able to fetch the correct built wheel for the host platform. But also be warned that I'm not a macOS user, so I don't have any direct experience with this stuff.

@martinfleis
Copy link
Contributor

Do you want me to test something here? I have access to both M1 and Intel Mac.

@mwtoews
Copy link
Member Author

mwtoews commented Feb 13, 2024

@martinfleis My main curiosity is with pip install --dry-run --force shapely==2.0.2, which binary wheel is selected? universal2 or arm64?

@martinfleis
Copy link
Contributor

martinfleis commented Feb 13, 2024

That would pull arm64, while on M1.

@jorisvandenbossche
Copy link
Member

I should elaborate more on why to remove universal2. They are nearly twice the size, since they contain both x86_64 and arm64

Yes, certainly on board with that. In most other projects where I am involved in, we also dropped the universal wheels the last years (eg pandas, and also numpy did the same I see)

@jorisvandenbossche jorisvandenbossche changed the title RLS: use native Apple Silicon macOS for arm64 wheels; remove universal2 RLS/BLD: use native Apple Silicon macOS for arm64 wheels; remove universal2 Feb 16, 2024
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good!

@jorisvandenbossche jorisvandenbossche merged commit 6a56c36 into shapely:main Feb 16, 2024
31 checks passed
@mwtoews mwtoews deleted the maint-2.0-macos-14 branch February 16, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants