gh-146636: Improve ABI/feature selection, add new header for it - #148302
gh-146636: Improve ABI/feature selection, add new header for it#148302encukou merged 11 commits into
Conversation
Sorry, something went wrong.
…ABLED Add a test that Python headers themselves don't use Py_GIL_DISABLED in abi3t: abi3 and abi3t ought to be the same except the _Py_OPAQUE_PYOBJECT differences. This is done using the GCC-only poison pragma.
| // whether they need extra synchronization. | ||
| # define Py_GIL_DISABLED | ||
| # endif | ||
| # if defined(_Py_IS_TESTCEXT) |
There was a problem hiding this comment.
You may move #ifdef __GNUC__ here:
| # if defined(_Py_IS_TESTCEXT) | |
| # if defined(_Py_IS_TESTCEXT) && defined(__GNUC__) |
Sorry, something went wrong.
There was a problem hiding this comment.
This subtle style detail is intentional: I'm inviting #elifs for other compilers.
Sorry, something went wrong.
|
|
||
| /* The internal C API must not be used with the limited C API: make sure | ||
| * that Py_BUILD_CORE* macros are not defined in this case. | ||
| * But, keep the "original" values, under different names, for "exports.h" |
There was a problem hiding this comment.
It's surprising that _PyEXPORTS_CORE and _PyEXPORTS_CORE_MODULE are defined even if Py_LIMITED_API is defined. I tried to undefine Py_BUILD_CORE if Py_LIMITED_API is defined before _PyEXPORTS_CORE code, but I got linker errors on Windows. Examples:
LINK : warning LNK4217: symbol 'PyType_GetFlags' defined in 'typeobject.obj' is imported by '_stat.obj' in fu nction '_PyLong_AsMode_t' [C:\victor\python\main\PCbuild\pythoncore.vcxproj] LINK : warning LNK4217: symbol '_Py_DecRef' defined in 'object.obj' is imported by '_stat.obj' in function '_ PyLong_AsMode_t' [C:\victor\python\main\PCbuild\pythoncore.vcxproj] LINK : warning LNK4286: symbol '_Py_DecRef' defined in 'object.obj' is imported by 'errnomodule.obj' [C:\vict or\python\main\PCbuild\pythoncore.vcxproj] LINK : warning LNK4217: symbol 'PyUnicode_FromStringAndSize' defined in 'unicodeobject.obj' is imported by '_ stat.obj' in function 'stat_filemode' [C:\victor\python\main\PCbuild\pythoncore.vcxproj]
Sorry, something went wrong.
There was a problem hiding this comment.
Yes; these select whether PyAPI_FUNC sumbols are exported or imported. Anything compiled into the main binary needs to be exported, regardless of the API it uses.
(This is currently done by including "exports.h" in the middle of pyport.h, right before Py_BUILD_CORE is undefined -- which is IMO confusing, as it makes the meaning of Py_BUILD_CORE depend on where you are in the header.)
Sorry, something went wrong.
Co-authored-by: Victor Stinner <vstinner@python.org>
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 7c9d920 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F148302%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
|
I ran into an issue similar to gh-148267 while working on CFFI (see https://github.com/python-cffi/cffi/actions/runs/24516133542/job/71660551119?pr=232) and was curious if this PR fixes it. It doesn't. The compiler error I see on this branch is: In file included from /Users/goldbaum/.pyenv/versions/3.15t-dev-encukoupr/include/python3.15t/Python.h:16:
/Users/goldbaum/.pyenv/versions/3.15t-dev-encukoupr/include/python3.15t/pyabi.h:55:24: error: invalid
token at start of a preprocessor expression
55 | # elif Py_LIMITED_API > Py_TARGET_ABI3T
| ^
I think this is happening because my CFFI branch was using _Py_OPAQUE_PYOBJECT directly and the header doesn't handle someone setting that outside of CPython's headers. Probably unlikely to happen in practice, not sure if it's worth explicitly guarding against so I thought I'd share. On the CFFI side I'll fix this by defining Py_TARGET_ABI3T. |
Sorry, something went wrong.
|
Looking again, maybe it's because CFFI's headers define Py_LIMITED_API but don't set a value: |
Sorry, something went wrong.
test_cext.test_build_abi3t() failed. All of these buildbot workers use Clang as the C compiler. I suppose that the issue can be reproduced with CC=clang CXX=clang++ ./python -m test test_cext -u all -v -m test_build_abi3t. Example on "buildbot/AMD64 FreeBSD PR": Building wheels for collected packages: internal__test_abi3t
Building wheel for internal__test_abi3t (pyproject.toml): started
Running command Building wheel for internal__test_abi3t (pyproject.toml)
CC env var: 'cc -pthread'
CFLAGS env var: <missing>
CPPFLAGS env var: <missing>
extra_compile_args: ['-Werror', '-D_Py_IS_TESTCEXT', '-Wcast-qual', '-pedantic-errors', '-Werror=declaration-after-statement', '-DMODULE_NAME=_test_abi3t', '-DPy_TARGET_ABI3T=0x30f00a8']
running bdist_wheel
running build
running build_ext
building '_test_abi3t' extension
creating build/temp.freebsd-13.5-RELEASE-p10-amd64-cpython-315-pydebug
cc -pthread -fno-strict-overflow -Wsign-compare -g -Og -Wall -fPIC -I/buildbot/buildarea/pull_request.ware-freebsd/build/build/test_python_81305æ/tempcwd/env/include -I/buildbot/buildarea/pull_request.ware-freebsd/build/Include -I/buildbot/buildarea/pull_request.ware-freebsd/build -c extension.c -o build/temp.freebsd-13.5-RELEASE-p10-amd64-cpython-315-pydebug/extension.o -Werror -D_Py_IS_TESTCEXT -Wcast-qual -pedantic-errors -Werror=declaration-after-statement -DMODULE_NAME=_test_abi3t -DPy_TARGET_ABI3T=0x30f00a8
In file included from extension.c:13:
In file included from /buildbot/buildarea/pull_request.ware-freebsd/build/Include/Python.h:79:
/buildbot/buildarea/pull_request.ware-freebsd/build/Include/object.h:126:16: error: attempt to use a poisoned identifier
126 | #elif !defined(Py_GIL_DISABLED)
| ^
In file included from extension.c:13:
In file included from /buildbot/buildarea/pull_request.ware-freebsd/build/Include/Python.h:80:
/buildbot/buildarea/pull_request.ware-freebsd/build/Include/refcount.h:342:15: error: attempt to use a poisoned identifier
342 | #elif defined(Py_GIL_DISABLED) && defined(Py_REF_DEBUG)
| ^
In file included from extension.c:13:
In file included from /buildbot/buildarea/pull_request.ware-freebsd/build/Include/Python.h:128:
/buildbot/buildarea/pull_request.ware-freebsd/build/Include/modsupport.h:117:15: error: attempt to use a poisoned identifier
117 | #elif defined(Py_GIL_DISABLED)
| ^
3 errors generated.
error: command '/usr/local/libexec/ccache/cc' failed with exit code 1
error: subprocess-exited-with-error
I can reproduce the issue on Fedora with Clang on this short program: #define Py_TARGET_ABI3T 0x30f00a8
#pragma GCC poison Py_GIL_DISABLED
#ifdef Py_TARGET_ABI3T
# define _Py_OPAQUE_PYOBJECT
#endif
int main()
{
#ifdef _Py_OPAQUE_PYOBJECT
// All good
return 0;
#elif defined(Py_GIL_DISABLED)
// Not good
return 1;
#endif
return 0;
}Clang output: x.c:13:15: error: attempt to use a poisoned identifier
13 | #elif defined(Py_GIL_DISABLED)
| ^
1 error generated.
The workaround is to write: #ifdef _Py_OPAQUE_PYOBJECT
// All good
return 0;
#else
# if defined(Py_GIL_DISABLED)
// Not good
return 1;
# endif
#endif
test.test_io.test_fileio failed: waiting for #148648 fix. |
Sorry, something went wrong.
Oh, maybe we can set the Py_LIMITED_API macro value to 3 if the macro is empty or if its value is smaller than 3, with a warning: #if Py_LIMITED_API+0 < 3
# warning "Py_LIMITED_API macro value must be at least 3"
# undef Py_LIMITED_API
# define Py_LIMITED_API 3
#endif |
Sorry, something went wrong.
|
@mgorny and I were also looking at the stable-abi-testing repo and got into a situation where "weird" things happen if you define Py_TARGET_ABI3T but don't define Py_LIMITED_API. IMO it'd be more helpful if Python's headers could trigger a build error if you don't explicitly set Py_LIMITED_API when Py_TARGET_ABI3T is set. |
Sorry, something went wrong.
|
🤖 New build scheduled with the buildbot fleet by @encukou for commit 72b1637 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F148302%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
Sorry, something went wrong.
|
RHEL8 changes are unrelated. (I'll remove them from the 🔨 test-with-buildbots set, but probably not before beta.) I'm planning to merge this PR soon, and base Windows changes & further tests on it. |
Sorry, something went wrong.
…pythonGH-148302) Improve ABI/feature selection, add new header for it. Add a test that Python headers themselves don't use Py_GIL_DISABLED in abi3t: abi3 and abi3t ought to be the same except the _Py_OPAQUE_PYOBJECT differences. This is done using the GCC-only poison pragma. Co-authored-by: Victor Stinner <vstinner@python.org>
Feature/ABI selection headers (Py_BUILD_CORE, Py_LIMITED_API, Py_GIL_DISABLED) need to be defined early (so they affect all definitions), but after the version (patchlevel.h) and configuration (pyconfig.h).
Put them in their own header to make this more obvious, and easier for contributors to keep them in the proper place.
Add a test that Python headers themselves don't use Py_GIL_DISABLED, since abi3 and abi3t ought to be identical except for the _Py_OPAQUE_PYOBJECT differences.
This is done using the GCC-only poison pragma, which tells the compiler/preprocessor to error if it ever sees the identifier after the pragma.
This needs some rewriting: #if defined(Py_LIMITED_API) && defined(Py_GIL_DISABLED) needs to be split into two lines, as the poisoning doesn't honour short-circuiting.
This should fix gh-148267, but I'm attaching the PR to the abi3t issue, gh-146636, as it continues that effort (GH-148142 specifically).