python: Add test cases for PEP 803/820 ft-compatible limited API#15695
python: Add test cases for PEP 803/820 ft-compatible limited API#15695mgorny wants to merge 1 commit intomesonbuild:masterfrom
Conversation
|
Thanks for working on this @mgorny.
That [1] was intended as a link it looks like - python/cpython#148267. |
|
Indeed, thanks for noticing. Fixed now. |
rgommers
left a comment
There was a problem hiding this comment.
This LGTM module a few very small things. I also verified that abi3.so or the abi3 name isn't used explicitly anywhere, so it's expected that there is no need to explicitly check for abi3t.so either.
| default_options : ['buildtype=release', 'werror=true']) | ||
|
|
||
| py_mod = import('python') | ||
| py = py_mod.find_installation() |
There was a problem hiding this comment.
Nit: more idiomatic to collapse these 2 lines to 1 line and not use a py_mod variable.
There was a problem hiding this comment.
I was copying the existing test, but I suppose I can change that.
| py_mod = import('python') | ||
| py = py_mod.find_installation() | ||
|
|
||
| if py.get_variable('Py_GIL_DISABLED', 0) == 1 and py.language_version().version_compare('<3.15') |
There was a problem hiding this comment.
Shouldn't this stay? This is not supported on 3.14 Maybe add a code comment saying that limited.c doesn't use PyModExport, hence this test case doesn't support use of the limited API under free-threading? Or extend the error message below to say that. Because I was slightly confused about why this wouldn't work for a second.
| } | ||
|
|
||
| PyMODINIT_FUNC PyInit_limited(void) { | ||
| PyErr_SetString(PyExc_NotImplementedError, "legacy init not supported"); |
There was a problem hiding this comment.
Is this better than not defining the PyInit_ symbol at all?
There was a problem hiding this comment.
To be honest, I don't recall anymore why I did it that way in the stable-abi-testing repo, but there was probably a reason. Yes, it's one of these things :-).
There was a problem hiding this comment.
Checked upstream, it's done the same way there in the one test package they have:
so resolving.
b8cad09 to
016b197
Compare
Add a new test case that exercises the limited API code in Python 3.15.0b1. It is run with Python 3.15+, both GIL-enabled (where `.abi3.so` extension is built) and freethreading (`.abi3t.so`). The "old" limited API test case is now restricted to GIL-enabled builds only, as the code is not compatible with the new API, and it is cleaner to have separate test cases rather than add conditions to that code. Signed-off-by: Michał Górny <mgorny@quansight.com>
|
Okay, so Python 3.15.0b1 is out. The bug we were working around is fixed, so I've removed the hack for now. We're still going to need something to switch to PEP 820 changed the limited API, so I've updated the test cases. I've also rebased on top of master. |
dcbaker
left a comment
There was a problem hiding this comment.
This looks good to me. I think @eli-schwartz wanted to look at it as well?
Yup, I was worried about that, and turns out for good reason... |
| } | ||
|
|
||
| PyMODINIT_FUNC PyInit_limited(void) { | ||
| PyErr_SetString(PyExc_NotImplementedError, "legacy init not supported"); |
There was a problem hiding this comment.
Checked upstream, it's done the same way there in the one test package they have:
so resolving.
| py_mod = import('python') | ||
| py = py_mod.find_installation() | ||
|
|
||
| if py.get_variable('Py_GIL_DISABLED', 0) == 1 and py.language_version().version_compare('<3.15') |
Add a new test case that exercises the limited API code in Python 3.15.0b1. It is run with Python 3.15+, both GIL-enabled (where
.abi3.soextension is built) and freethreading (.abi3t.so). The "old" limited API test case is now restricted to GIL-enabled builds only, as the code is not compatible with the new API, and it is cleaner to have separate test cases rather than add conditions to that code.