-
Notifications
You must be signed in to change notification settings - Fork 239
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
Set SYSTEM_VERSION_COMPAT=0 during pip install on macos #1768
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're modifying the wrong environment here, it should be the previous invocation labelled 'install the wheel'. Having said that it's probably worth setting the env var for this invocation too.
for more information, see https://pre-commit.ci
Done, please check! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be good to limit this to CPython 3.8, it's not required to do this on CPython 3.9+ and on 3.6 and 3.7 this is an error that users will also get, so it's worth alerting packagers to.
Sorry can you clarify? Do you mean on python 3.6 and 3.7, we also have to set SYSTEM_VERSION_COMPAT=0 as user would also encounter the same problem (e.g. On arm64 runner building x86_64 wheels, triggering the problem when they install wheel with target >=11.0)? Or only set SYSETEM_VERSION_COMPAT=0 on python 3.8? |
Python 3.6 and 3.7 were never ported to ARM on macOS. (3.8 was only partially ported). |
@henryiii theoretically, can someone crosscompile cp36/cp37 x86_64 wheel on arm64 macos runner, and trigger the bug we are discussing? (Probably nobody would actually do this irl though) Edit: |
Done adding the additional check, please review. |
Turns out the bug can indeed occur if someone crosscompile cp36/37 x86_64 wheels on arm64 macos runner: https://github.com/laggykiller/rlottie-python/actions/runs/8086369591/job/22095975016 I have changed to checking if python version <= 3.8, please review. |
It's a complex area, but here's my understanding:
Actually this issue isn't to do with arm64 at all, it's to do with the version numbers. macOS was 10.x for many many years, so when they made the transition to 11.0, Apple was concerned that some old software wouldn't bother checking the major version and do checks like So they coded the OS with a special rule, if the software was built with an SDK for a previous version of macOS, instead of returning the real version number for the OS, (something like 12.1), they return a fake version, 10.16. 10.16 doesn't exist. CPython 3.6 and 3.7 were released before this SDK was released, so they are running in this compatibility mode by default (i.e. they default to Users of CPython 3.8 can make pip install a package targeting macOS 11+ by updating their Python. Users of 3.6 and 3.7 cannot - they'd have to set SYSTEM_VERSION_COMPAT=0 to do so.
If my reasoning above is correct, the same issue would happen on an x86_64 runner too - the issue is the MACOSX_DEPLOYMENT_TARGET is set too high, so we hit this pip bug. |
Can confirm about this |
Co-authored-by: Joe Rickerby <[email protected]>
Co-authored-by: Joe Rickerby <[email protected]>
All agreed and committed, please check! |
for more information, see https://pre-commit.ci
@@ -579,17 +579,37 @@ def build(options: Options, tmp_path: Path) -> None: | |||
shell_with_arch(before_test_prepared, env=virtualenv_env) | |||
|
|||
# install the wheel | |||
if is_cp38 and python_arch == "x86_64": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this change only fixed the MacOS builds for the cp38
case. My builds for cp36
and cp37
, e.g.: https://github.com/alexlancaster/pypop/actions/runs/9866159045/job/27244457994#step:4:298 fail.
I had to modify my pyproject.toml
to include the SYSTEM_VERSION_COMPAT=0
as a workround, but I feel this should be in cibuildwheel, and the comment here: #1768 (comment) seems to imply that the fix should also apply to <cp38
builds too, but it doesn't appear to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users of Python 3.6 and 3.7 can't install wheels that target macOS newer than 11.0. So you can build them, but users can't install them - well, they technically could, but they'd have to set SYSTEM_VERSION_COMPAT=0 too when they install them, otherwise pip will see the wheels as targeting a macOS that's newer than what's running.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, but isn't it possible that Python 3.6 or 3.7 could be running on MacOS 12 or later? or did Python drop packages for those Python versions after 11.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See this comment for further explanation- #1768 (comment)
Fixes #1767