-
-
Notifications
You must be signed in to change notification settings - Fork 101
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
Implement support for the free-threaded build of CPython 3.13 #1015
Implement support for the free-threaded build of CPython 3.13 #1015
Conversation
The changes look good to me and I didn't see any ref count issues, however I am by no means an expert on the C API |
Assuming the CI passes (might take a while to run since there is a lot of activity right now), I'd like another maintainer who has more experience with the C API to take a look before merging. |
Thanks @lysnikolaou |
It looks like one of the existing actions unrelated to this PR is using an old upload-artifact. It would be nice if github told us which one. |
Yeah, I saw that as well. Opened #1016 for it. |
Looks like its almost there 👍 |
Looks like it's good to go on my side! |
LGTM, but I'd still like another set of 👀 #1015 (comment) before merging |
Friendly ping here! |
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.
Here's my CI/CD feedback. I can't say I'm experienced with C-API, however. Still, I think we can merge this and start a release process to see if more rare architectures also succeed building the new wheels. It should be enough to see if cibuildwheel passes in those jobs that only run on release, since it also runs some testing. We can then abort the release once we know. Or we could just attempt a regular release.
899a644
to
41caba0
Compare
Thanks for the reviews everyone! I've addressed all of the feedback points. |
It looks like we don't have any benchmarks for this repository so can't check the performance impact. I guess I'll need to write some. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1015 +/- ##
=======================================
Coverage 94.86% 94.86%
=======================================
Files 23 23
Lines 2686 2686
Branches 254 254
=======================================
Hits 2548 2548
Misses 67 67
Partials 71 71
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Co-authored-by: J. Nick Koston <[email protected]>
41caba0
to
b302ec3
Compare
It looks like the testing fails because there is no cffi wheel for free-threaded 3.13 so it tries to build it and since libffi isn't installed in the cibuildwheel env, it fails |
CodSpeed Performance ReportMerging #1015 will not alter performanceComparing Summary
|
What do these changes do?
Implement support for the free-threaded build of CPython 3.13!
In more detail, this PR does the following:
pythoncapi_compat.h
and move away from all the borrowed reference APIs.pair_list_global_version
.Are there changes in behavior for the user?
There shouldn't be any.
Related issue number
Related to #1014.
Checklist