Skip to content

Update to CMake 3.20.2 #135

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

Merged
merged 4 commits into from
May 13, 2021
Merged

Update to CMake 3.20.2 #135

merged 4 commits into from
May 13, 2021

Conversation

mayeut
Copy link
Contributor

@mayeut mayeut commented May 1, 2021

Besides the CMake update, the goal of this PR is to get universal2 support on macOS.

Fix #132

@mayeut mayeut marked this pull request as ready for review May 1, 2021 23:53
@mayeut mayeut marked this pull request as draft May 2, 2021 10:54
@mayeut
Copy link
Contributor Author

mayeut commented May 2, 2021

Converted to draft while waiting on #130 so that I can rebase this one on master once #130 is merged.

@mayeut
Copy link
Contributor Author

mayeut commented May 2, 2021

PS: I don't know why the macOS 10.10 binary of CMake is built as universal given that a 10.13+ is available. Probably an oversight when splitting in 2 macOS deliveries. The wheel for 10.10 will only be uploaded as macosx_10_10_x86_64 while the 10.13 one will be uploaded as macosx_10_13_x86_64.macosx_10_13_universal2.macosx_11_0_universal2.macosx_11_0_arm64.
I opened an issue at https://gitlab.kitware.com/cmake/cmake/-/issues/22137 if we can get some MB saved on this one but not a showstopper at all.

@henryiii
Copy link
Contributor

henryiii commented May 7, 2021

@jcfr Can we trigger a release? I'd like to get 3.18 out before adding this.

@jcfr
Copy link
Contributor

jcfr commented May 7, 2021

I'd like to get 3.18 out before adding this.

I suggest to move forward with integration of this, create the tag for 3.20.2 to trigger the release.

Once done, we then create a branch for releasing 3.18.6 based of master.

@henryiii What do you think ?

@jcfr
Copy link
Contributor

jcfr commented May 7, 2021

I opened an issue at https://gitlab.kitware.com/cmake/cmake/-/issues/22137 if we can get some MB saved on this one but not a showstopper at all.

For future reference, here was the answer:

The primary differences between the 10.13+ binary and the 10.10+ binary are in cmake-gui and Qt behavior. Ideally we would have only the 10.10+ binary but it is impossible to support Dark Mode and a few other things on modern macOS without requiring 10.13+.
For command-line usage (e.g. in CI scripts) one can us the 10.10+ binary to support all architectures.

@jcfr
Copy link
Contributor

jcfr commented May 7, 2021

we can get some MB saved on this one

Also since we do not distribute cmake-gui in the wheel, we could probably package the content of the 10.10 package in a wheel called macosx_10_13_x86_64.macosx_10_13_universal2.macosx_11_0_universal2.macosx_11_0_arm64.macosx_10_0_universal2.macosx_10_0_arm64

That said, I am wondering if it is worth the hassle .. and potential future complications.

@mayeut mayeut marked this pull request as ready for review May 8, 2021 09:51
@mayeut
Copy link
Contributor Author

mayeut commented May 8, 2021

That said, I am wondering if it is worth the hassle .. and potential future complications.

Given the answer on the CMake issue & @jcfr remark about cmake-gui, I can revisit the PR to target only 1 MACOSX_DEPLOYMENT_TARGET target.

Furthermore, I'm not sure the PR will allow for seamless build from source (i.e. not CMake source but cmake-python-distributions package source) with the current use of MACOSX_DEPLOYMENT_TARGET. I will push 2 new commits to give some different options.

@mayeut mayeut marked this pull request as draft May 8, 2021 09:57
@mayeut mayeut marked this pull request as ready for review May 8, 2021 13:16
@mayeut
Copy link
Contributor Author

mayeut commented May 8, 2021

The build from source issue is fixed & squashed into 2f96a87
The c86767c commit switches to distributing only one macOS 10.10 universal wheel (well, 2 wheels, py2 & py3)

I find the single macOS 10.10 version a bit less hacky, especially, it allows to have something a bit more generic in scikit-ci.yml.

ppc64le builds are still flaky....

@henryiii
Copy link
Contributor

henryiii commented May 8, 2021

We will probably want to distribute two wheels (Edit: at least two wheel tags); a classically named x86 wheel, and a universal wheel. You need pip 21.0.1+ to install 10.9+ universal2 wheels (or a slightly older pip for 11+ universal2 wheels), so the x86 wheel is needed for older versions of pip (like the default pip even on macOS 11, etc). Since this is already broken up into two wheels, 10.13+ for the Universal one seems to be fine, I'd think? Though it could wait until/if we add GUI support, perhaps, and remain 10.10?

@henryiii
Copy link
Contributor

henryiii commented May 8, 2021

I suggest to move forward with integration of this, create the tag for 3.20.2 to trigger the release.

Once done, we then create a branch for releasing 3.18.6 based of master.

@henryiii What do you think ?

Fine with me; I'm mostly thinking the 3.18 release is simpler and can be pushed out sooner, then we have a little more time to get 3.20 right. But branching is fine too. I don't know if we want to get a 3.19 out also (I'd rather like to so users can select any CMake version they want), but it's a weird mix of 3.18 and 3.20 changes.

@mayeut
Copy link
Contributor Author

mayeut commented May 8, 2021

The wheels generated by the last commit are (editing the version part so that one can focus on the platform part):

cmake-3.20.2-py3-none-macosx_10_10_universal2.macosx_10_10_x86_64.macosx_11_0_arm64.macosx_11_0_universal2.whl
cmake-3.20.2-py2-none-macosx_10_10_universal2.macosx_10_10_x86_64.macosx_11_0_arm64.macosx_11_0_universal2.whl

Those should be installable by any pip that supports either one of the following platform tags:

  • macosx_10_10_x86_64
  • macosx_10_10_universal2
  • macosx_11_0_arm64
  • macosx_11_0_universal2

@henryiii
Copy link
Contributor

I suggest to move forward with integration of this, create the tag for 3.20.2 to trigger the release.

Okay, let's put this in.

@henryiii henryiii merged commit 3bf06c7 into scikit-build:master May 13, 2021
@mayeut mayeut deleted the cmake-3.20.2 branch May 14, 2021 08:21
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.

Universal2 wheel on macOS
3 participants