Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-85283: _stat extension uses the limited C API#108573
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner commented Aug 28, 2023 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Aug 29, 2023
I moved the Changelog entry to the Build category, and I documented the change in the Build Changes of What's New in Python 3.13. See #85283 (comment) about the choice of the Category. |
Hm, I'm not so sure about not building _stat if Py_TRACE_REFS are defined
vstinner commented Aug 29, 2023
It's a workaround for issue #108634: without this change, building Python with But. It's more complicated than expect: multiple tests fail when I'm going to:
|
AA-Turner commented Aug 29, 2023
Just to note a slight hesitation of intentionally breaking a buildbot &c, given that A |
vstinner commented Aug 30, 2023
serhiy-storchaka left a comment
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.
Caught a bug! PyModule_Add requires 3.13.
vstinner commented Aug 30, 2023
Oh! Only on Windows, so I didn't notice locally on Linux. I don't want to downgrade |
vstinner commented Aug 31, 2023
This PR will no longer break TraceRefs. I merged PR #108663 which adds limited C API support to Py_TRACE_REFS build. |
The _stat C extension is now built with the limited C API.
vstinner commented Aug 31, 2023
I rebased my PR to solve a merge conflict (Doc/whatsnew/3.13.rst). |
vstinner commented Aug 31, 2023
@erlend-aasland: Would you mind to review the latest version of the PR? |
vstinner commented Aug 31, 2023
On-going discussion about converting some stdlib C extensions to the limited C API: https://discuss.python.org/t/use-the-limited-c-api-for-some-of-our-stdlib-c-extensions/32465 |
erlend-aasland commented Aug 31, 2023
Sorry, I prefer to still stay out of the C API discussions. From the linked discussion, it seems to me these changes are far from non-controversial; perhaps considered marking this as draft until consensus have been reached. |
vstinner commented Aug 31, 2023
Ok, I marked my PR as a draft. I already marked the 3 other similar PRs as draft. |
vstinner commented Sep 13, 2023
I close this PR until a decision is taken on using the limited C API for a few Python stdlib extensions. Well, it's not that complicated to rewrite this PR (or reuse the patch of the closed PR). |
The _stat C extension is now built with the limited C API.