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

fix: avoid forcing the macOS version #607

Merged
merged 1 commit into from
Dec 28, 2021

Conversation

henryiii
Copy link
Contributor

@henryiii henryiii commented Nov 10, 2021

This fixes #596 by removing the custom logic that is lowering the macOS version incorrectly. This was originally there because Python defaults to whatever it was built with, and 10.6 was too low for modern macOS. However, all normal Pythons (everything except the dead 3.5) have 10.9 or better installers now, and this is causing the (correct) calculation of the version to be lost. Users should always set MACOSX_DEPLOYMENT_TARGET, and most wheel building tools will do this for you just in case.

Draft until I'm sure it works or can add tests.

@henryiii henryiii marked this pull request as ready for review December 11, 2021 03:20
@henryiii henryiii marked this pull request as draft December 11, 2021 03:20
@henryiii
Copy link
Contributor Author

Before:

$ MACOSX_DEPLOYMENT_TARGET=10.13 _PYTHON_HOST_PLATFORM=macosx-10.11-universal2 pipx run build --wheel
...
Successfully built scikit_build_example-0.0.1-cp310-cp310-macosx_10_11_universal2.whl

After:

$ MACOSX_DEPLOYMENT_TARGET=10.13 _PYTHON_HOST_PLATFORM=macosx-10.11-universal2 pipx run build --wheel
...
Successfully built scikit_build_example-0.0.1-cp310-cp310-macosx_10_13_universal2.whl

I'm happy with it! :)

@henryiii henryiii marked this pull request as ready for review December 13, 2021 23:19
@henryiii henryiii merged commit 6933aa9 into scikit-build:master Dec 28, 2021
@henryiii henryiii deleted the henryiii/fix/macos branch December 28, 2021 17:32
@henryiii
Copy link
Contributor Author

Ouch, this bites PyPy because it defaults to 10.7, instead of an actual proper minimum (10.9+). @mattip I'm sure I've mentioned this before - do you know if there's been any progress on that (or where we discussed it before)? Setuptools (and therefore Scikit-Build) require MACOSX_DEPLOYMENT_TARGET (cibuildwheel sets it by default) to build a PyPy extension on macOS, while CPython is fine because they support 10.9+ instead of 10.7+, so they get stdc++ instead of stdlibc++, which was removed some time ago.

I'm still in favor of this patch since it's exactly the same issue with exactly the same fix as vanilla setuptools, but it would be nice to fix it properly in PyPy. We could also consider setting MACOSX_DEPLOYMENT_TARGET if unset and PyPy is detected, which would be a less problematic workaround for now.

@mattip
Copy link
Contributor

mattip commented Jan 20, 2022

This will be 10.9 for the upcoming release.

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.

bug: compute correct deployment target
2 participants