-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
GH-114809: Add support for macOS multi-arch builds with the JIT enabled #131751
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
GH-114809: Add support for macOS multi-arch builds with the JIT enabled #131751
Conversation
Hmmmmm, will investigate why everything is failing tomorrow.
|
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.
Thanks, looks good! Just the few things we discussed offline:
- Maybe trying to get this in CI.
- Indentation nitpick for preprocessor directives in
jit_stencils.h
. - The possibility of just using the second code path in
build.py
for single-arch builds too.
4b518e5
to
c54250b
Compare
Co-authored-by: Brandt Bucher <brandtbucher@gmail.com>
@savannahostrowski Thanks for working on this! I'll review and test this in the next day or two. |
Thanks @ned-deily! It would be great if we could have the JIT built-but-off ( The remaining tests seem to be failing because the external introspection and remote exec features doesn't work on fat builds, and this PR switches the JIT CI over to Universal2 to make sure we don't break releases. The existing tests don't skip themselves on these builds, and are just failing. @pablogsal, is it intended that fat builds aren't supported by the internal introspection and remote exec features? The error message we're running into here says I think the correct thing to do here is for the |
@savannahostrowski, I spoke to @pablogsal and he'll look into the universal issue separately. Let's roll back the CI change and land that later after the feature is fixed for universal builds. |
I think I can get something today today or tomorrow if you want to wait until then |
Yep, we're fine waiting. |
Fastest gun in the west folks: #132892 |
The other test needs #132852 to reuse the implementation |
Both are landed. Rebasing should fix the CI |
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.
LGTM. With these additions, I was able to produce a python.org-style macOS universal2 installer containing both traditional (GIT-enabled) and free-treaded release builds both with optional JIT (yes-off
). Installer smoke tests of all four options on both Apple Silicon (running current macOS 15.4) and Intel Macs (with macOS 11.7.10) did not show any obvious regressions.
The only unexpected issue was the JIT build's use of pthread_jit_write_protect_np
(added in #126195). That function is only available as of macOS 11, which is when Apple Silicon Mac support was introduced. The macOS installer build supports a range of operating system releases through the use of Apple's preferred method of weak-linking and run-time testing for a feature's availability. We've tried to cover a very broad range of releases and architecture, occasionally upping the lower bound. For 3.13, we have been supporting down to macOS 10.13 but, for various reasons, have been planning to bump that to at least 10.15 for 3.14 and it likely wouldn't affect too many users if we went to macOS 11 (which was the next feature release after 10.15) instead. For b1, I'll plan to target macOS 11+ and make a final recommendation as part of a macOS support policy PEP during the beta phase. There is more than one approach we could take should we decide such support is needed but my initial reaction is that it won't be. (Also, note that this is only an issue when building to support a range of operating systems with one universal binary. Building with the jit enabled for, say, just macOS 10.15 with a macOS 10.15 SDK should work as is.)
Thanks @ned-deily!
One thing I forgot to mention is that I'm planning on making it an error in 3.14 to pass both So we would only pass
Thanks for the explanation. If it turns out to be too much trouble, we can use the "older" |
Thanks @ned-deily for the review and @pablogsal for the quick turnaround on the test fixes! (This is also my last CPython contribution as Savannah Ostrowski, as I'm getting married this weekend, so it feels doubly special 🥹) |
This work patches up #115742.