FIX: import internal cpython headers to fix for py311 #4667

tacaswell · 2022-02-26T23:13:58Z

In 18b5dd68c6b616257ae243c0b6bb965ffc885a23 /
python/cpython#31530 /
https://bugs.python.org/issue46836

The _frame struct was moved to an internal header, however the public API is
primarily read-only, and cython needs to build _frame objects so still import
the internal headers.

The version bounding is wrong (the problem commit won't be out until a6, but this condition will compile against the current CPYthon main branch.

I'm not at all sure that this is the right fix, but it compiled!

tacaswell · 2022-02-26T23:41:03Z

I was fooled by faliing to fully rebuild before and missed in include (I had just removed the line number setting while testing 🤦🏻 ).

I am very confused by I could not get the version getting in ModuleSetup.c to work so this is currently broken for every other version 😞 so made a draft.

Cython/Utility/ModuleSetupCode.c

tacaswell · 2022-02-27T21:49:45Z

@scoder I can confirm that this works and squashed it down to one commit.

vstinner

Oh nice, I didn't know that there was already a PR! I wrote a simpler change: https://github.com/cython/cython/pull/4671/files (closed).

vstinner · 2022-03-02T22:36:48Z

Cython/Utility/Coroutine.c

@@ -499,6 +499,9 @@ static int __pyx_Generator_init(PyObject *module); /*proto*/
 //@requires: ModuleSetupCode.c::IncludeStructmemberH

 #include <frameobject.h>
+#if PY_VERSION_HEX >= 0x030b00a5


Technically, the include will only be needed since 0x030b00a6 but it's fine to include it in Python 3.11 alpha 5. I suggest to define the Py_BUILD_CORE macro to include an internal C API header file. Right now, it's not needed, but I consider requiring it later.

# ifndef Py_BUILD_CORE # define Py_BUILD_CORE 1 # endif

Same remarks for the other modified files.

Fair, but then it would not work for me locally. What is the best way to test this sort of thing in the "interstitial" time? I guess I could just locally patch my CPython source to be the next release prematurely?

Should I also include the !CYTHON_COMPILING_IN_PYSTON check everywhere?

3.11a6 will hopefully be out quite soon. Maybe faster than this gets merged. Not sure when it will become available on Github Actions, though.

In 18b5dd68c6b616257ae243c0b6bb965ffc885a23 / python/cpython#31530 / https://bugs.python.org/issue46836 The `_frame` struct was moved to an internal header, however the public API is primarily read-only, and cython needs to build _frame objects so still import the internal headers.

tacaswell · 2022-03-05T00:00:11Z

I fixed the version gating and addeng the Py_BUILD_CORE define in a second commit (to make it easy to strip if needed).

Should I also include the !CYTHON_COMPILING_IN_PYSTON check everywhere?

I did not add this check as it looks like PYSTON is pinned to 3.8?

scoder · 2022-03-05T03:41:20Z

I fixed the version gating and addeng the Py_BUILD_CORE define in a second commit (to make it easy to strip if needed).

I'm still not happy about the fact that we now have to do that. We currently set the macro at a somewhat late point, and since we do it in more than one place, it might happen that other CPython header files get included afterwards (e.g. traceback.h in this case, although we could at least move that before the frame header include).

@vstinner, I think we should unset it directly after including the frame header file. In that case, the #ifndef check seems counterproductive. Although, I guess a (very) few users might actually want to access CPython internals themselves, in their own code… Can we somehow save the old define and restore it? I would basically like to have a way to say "define it for this specific header and don't interfere with anything else". Or would you consider it "not a problem" if it's defined?

Should I also include the !CYTHON_COMPILING_IN_PYSTON check everywhere?

I did not add this check as it looks like PYSTON is pinned to 3.8?

Pyston should be fine. It's based on CPython now. If they need something different in a later version, we'll adapt at that point.

Cython/Utility/Exceptions.c

Cython/Utility/Profile.c

vstinner

LGTM.

vstinner · 2022-03-11T16:37:17Z

@scoder @da-woods: Python 3.11 alpha6 has been released, can this PR be merged now?

scoder · 2022-03-12T11:06:03Z

Thanks

In python/cpython#31530 https://bugs.python.org/issue46836 the `_frame` struct was moved to an internal header, however the public API is primarily read-only, and Cython needs to build PyFrameObjects so still import the internal headers. Also sets the Py_BUILD_CORE define for py311a6, trying to restrict it to the frame header.

scoder · 2022-03-12T11:15:17Z

Backported to 0.29.x in afc00fc
(Let's see if I missed something)

vstinner · 2022-03-14T09:56:19Z

Great, thank you :-)

ncoghlan · 2022-03-19T16:29:38Z

Please comment on https://discuss.python.org/t/proposal-rename-pyinterpreterframe-struct-as-py-framedata/14213/7 if any of the adjustments proposed there would affect Cython.

ncoghlan · 2022-03-20T03:21:10Z

Don't worry about my previous comment - we'll make sure code that works against 3.11a6 keeps working until faster-cpython/ideas#309 provides a proper public API for the relevant frame manipulation tasks (although feel free to chime in there on the public API that you'd like to see to avoid having to delve into frame object internals)

tacaswell marked this pull request as draft February 26, 2022 23:38

scoder marked this pull request as ready for review February 27, 2022 14:49

scoder reviewed Feb 27, 2022

View reviewed changes

Cython/Utility/ModuleSetupCode.c Outdated Show resolved Hide resolved

tacaswell force-pushed the fix_py311_internals branch from 4130215 to 952fc89 Compare February 27, 2022 21:49

scoder mentioned this pull request Mar 2, 2022

Update to Python 3.11 alpha 6 #4671

Closed

vstinner reviewed Mar 2, 2022

View reviewed changes

tacaswell added 2 commits March 4, 2022 18:50

MNT: add Py_BUILD_CORE define for py311a6

fb678f3

tacaswell force-pushed the fix_py311_internals branch from 952fc89 to fb678f3 Compare March 4, 2022 23:52

brandtbucher mentioned this pull request Mar 7, 2022

Broken pyperformance benchmarks faster-cpython/ideas#311

Closed

2 tasks

scoder reviewed Mar 7, 2022

View reviewed changes

Cython/Utility/Exceptions.c Outdated Show resolved Hide resolved

Avoid impact of Py_BUILD_CORE on "traceback.h".

d06bd65

scoder reviewed Mar 7, 2022

View reviewed changes

Cython/Utility/Profile.c Outdated Show resolved Hide resolved

Avoid impact of Py_BUILD_CORE on "traceback.h".

dbcd9ec

mjpieters mentioned this pull request Mar 11, 2022

Install Error on Python 3.11 aio-libs/frozenlist#285

Closed

vstinner approved these changes Mar 11, 2022

View reviewed changes

vstinner mentioned this pull request Mar 11, 2022

Use PyFloat_Pack8() on Python 3.11a7 msgpack/msgpack-python#499

Merged

scoder merged commit 7537910 into cython:master Mar 12, 2022

scoder added defect Code Generation labels Mar 12, 2022

scoder added this to the 0.29.29 milestone Mar 12, 2022

tacaswell deleted the fix_py311_internals branch March 14, 2022 14:31

mattip mentioned this pull request Apr 7, 2022

MAINT: Start testing with Python 3.11. numpy/numpy#21308

Merged

cjwatson mentioned this pull request Apr 23, 2022

10336: Implement twisted.python.failure._Code.co_positions twisted/twisted#1723

Merged

7 tasks

vstinner mentioned this pull request May 2, 2022

[C API] Move PyFrameObject to the internal C API python/cpython#90992

Closed

isc-adang mentioned this pull request May 17, 2022

[autoupdate] Update Cython from 0.29.28 to 0.29.30 intersystems/iknow#249

Merged

arcondello mentioned this pull request Oct 31, 2022

Build fails on Python3.11 dwavesystems/dimod#1280

Closed

sklam mentioned this pull request Jan 12, 2023

[WIP] Python 3.11 tracing numba/numba#8670

Merged

dimpase mentioned this pull request Mar 17, 2023

build/pkgs/cython: Update to 0.29.36 sagemath/sage#35084

Merged

4 tasks

scoder mentioned this pull request May 10, 2024

remove unused include pycore_frame #6188

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: import internal cpython headers to fix for py311 #4667

FIX: import internal cpython headers to fix for py311 #4667

tacaswell commented Feb 26, 2022

tacaswell commented Feb 26, 2022

tacaswell commented Feb 27, 2022

vstinner left a comment

vstinner Mar 2, 2022

tacaswell Mar 4, 2022

scoder Mar 5, 2022

tacaswell commented Mar 5, 2022

scoder commented Mar 5, 2022

vstinner left a comment

vstinner commented Mar 11, 2022

scoder commented Mar 12, 2022

scoder commented Mar 12, 2022

vstinner commented Mar 14, 2022

ncoghlan commented Mar 19, 2022 •

edited

Loading

ncoghlan commented Mar 20, 2022

FIX: import internal cpython headers to fix for py311 #4667

FIX: import internal cpython headers to fix for py311 #4667

Conversation

tacaswell commented Feb 26, 2022

tacaswell commented Feb 26, 2022

tacaswell commented Feb 27, 2022

vstinner left a comment

Choose a reason for hiding this comment

vstinner Mar 2, 2022

Choose a reason for hiding this comment

tacaswell Mar 4, 2022

Choose a reason for hiding this comment

scoder Mar 5, 2022

Choose a reason for hiding this comment

tacaswell commented Mar 5, 2022

scoder commented Mar 5, 2022

vstinner left a comment

Choose a reason for hiding this comment

vstinner commented Mar 11, 2022

scoder commented Mar 12, 2022

scoder commented Mar 12, 2022

vstinner commented Mar 14, 2022

ncoghlan commented Mar 19, 2022 • edited Loading

ncoghlan commented Mar 20, 2022

ncoghlan commented Mar 19, 2022 •

edited

Loading