Avoid exporting incorrect PyInit_* symbols
#12891
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Using the `#[pymodule]` derive macro in PyO3 0.21 always causes a `PyInit_*` symbol with a matching name to be exported in the output `cdylib`. This is required for the top-level module, in order for Python to import it---it needs to know which symbol in a shared library file it should call---but submodules must be manually initialised, so do not need it. Including it is typically harmless (and something we've been doing for a long time), but it is technically against the coding rules for CPython extensions[^1]. Recent versions of `abi3audit` (0.0.11+) have tightened their symbol checkers to better match the CPython guidelines, which causes our wheels to be rejected by their audits. This is, in theory, not a break of abi3 because CPython could never introduce an API-elvel `PyInit_*` function themselves without causing problems, so there ought to be no problems for our users, even with future Python versions. That said, we still want to pass the audit, because the coding guidelines are useful. This commit is not the cleanest way of doing things. PyO3 0.22 includes a `#[pymodule(submodule)]` option on the attribute macro, which lets us use all the standard code generation while suppressing the unnecessary `PyInit_*` symbol. When we are ready to move to PyO3 0.22, we probably want to revert this commit to switch to that form. [^1]: https://docs.python.org/3/c-api/intro.html
jakelishman
added
type: qa
|
One or more of the following people are relevant to this code:
|
mtreinish
approved these changes
Aug 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Using the
#[pymodule]derive macro in PyO3 0.21 always causes aPyInit_*symbol with a matching name to be exported in the outputcdylib. This is required for the top-level module, in order for Python to import it---it needs to know which symbol in a shared library file it should call---but submodules must be manually initialised, so do not need it. Including it is typically harmless (and something we've been doing for a long time), but it is technically against the coding rules for CPython extensions1.Recent versions of
abi3audit(0.0.11+) have tightened their symbol checkers to better match the CPython guidelines, which causes our wheels to be rejected by their audits. This is, in theory, not a break of abi3 because CPython could never introduce an API-elvelPyInit_*function themselves without causing problems, so there ought to be no problems for our users, even with future Python versions. That said, we still want to pass the audit, because the coding guidelines are useful.This commit is not the cleanest way of doing things. PyO3 0.22 includes a
#[pymodule(submodule)]option on the attribute macro, which lets us use all the standard code generation while suppressing the unnecessaryPyInit_*symbol. When we are ready to move to PyO3 0.22, we probably want to revert this commit to switch to that form.Details and comments
Manual backport of #12889 for the 1.1 branch.
Footnotes
https://docs.python.org/3/c-api/intro.html ↩