-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
gh-137210: Add a struct, slot & function for checking an extension's ABI #137212
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
Conversation
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.
Seems easier for us to just check this on our side than to expect users to get it right?
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.
And isn't the legacy value 1
rather than 3
?
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.
Well, users should use PyABIInfo_DEFAULT_ABI_VERSION
or PyABIInfo_VAR
-- those are our side too.
I prefer the data to be simpler, but if you want, we can keep the exception in.
The legacy value is 3. (Not that it matters that much; it's always compared to a Python version.)
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.
We're still planning to get rid of these options once FT is merged, right? Maybe we ought to use abiflags
here (i.e. a string) instead, for future extensibility? This isn't going to be checked anywhere that it needs to be bitflags.
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.
How would you encode being compatible with both GIL and FT?
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.
IMO, we can remove them when there are no GIL-only extensions around any more. That would be a lot of time after FT is the only option.
Include/modsupport.h
Outdated
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.
What are all of these warnings about? Does _Py_PACK_VERSION
do something funky?
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.
No idea. I'll need to check on a Windows box.
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.
I wasn't able to figure it out. Something seems to be compiling modsupport.h
outside of Python.h
-- that is, without pymacro.h
being included first: I get an error if I add this before the line:
#ifndef Py_PYMACRO_H #error this should not happen #endif
The error is currently quite impenetrable to me:
ResourceCompile:
C:\Program Files (x86)\Windows Kits10円\bin10円.0.26100.0\x86\rc.exe /D "ORIGINAL_FILENAME=\\\"python315_d.dll\\\"" /D FIELD3=100 /D _DEBUG /l"0x0409" /IC:\Users\encukou\dev\cpython\PC /IC:\Users\encu
kou\dev\cpython\Include /IC:\Users\encukou\dev\cpython\PCbuild\obj315円amd64_Debug\pythoncore\ /nologo /fo"C:\Users\encukou\dev\cpython\PCbuild\obj315円amd64_Debug\pythoncore\python_nt.res" ..\PC\pyth
on_nt.rc
3>C:\Users\encukou\dev\cpython\Include\modsupport.h(98): error RC2188: C:\Users\encukou\dev\cpython\PCbuild\obj315円amd64_Debug\pythoncore\RCa08340(52) : fatal error RC1116: RC terminating after preproce
ssor errors [C:\Users\encukou\dev\cpython\PCbuild\pythoncore.vcxproj]
3>Done Building Project "C:\Users\encukou\dev\cpython\PCbuild\pythoncore.vcxproj" (Build target(s)) -- FAILED.
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.
Ah, it's being referenced from PC\python_ver_rc.h
, which runs under the resource compiler rather than the C compiler. It's pretty much entirely compatible, but occasionally has some obscure/subtle differences.
I think it's just bypassing a lot of unnecessary includes here, assuming that modsupport.h
stands alone. If that's not true, then the header needs updating and whatever constant it's referencing will have to move elsewhere.
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.
I switched this PR to use 0x030f0000
; I plan to switch to _Py_PACK_VERSION(3, 15)
in a follow-up PR.
Co-authored-by: Steve Dower <steve.dower@microsoft.com>
Uh oh!
There was an error while loading. Please reload this page.
See the issue.
📚 Documentation preview 📚: https://cpython-previews--137212.org.readthedocs.build/