Use a common helper to determine applicable stable ABI builds#6192
Use a common helper to determine applicable stable ABI builds#6192ngoldbaum wants to merge 3 commits into
Conversation
ngoldbaum
force-pushed
the
fix-stable-abi-3.14t
branch
from
4e66a95 to
acc5145
Compare
ngoldbaum
force-pushed
the
fix-stable-abi-3.14t
branch
from
acc5145 to
41d139c
Compare
|
Updated with a force-push so the first commit can still be backported. |
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, sorry to post a second round of review.
| /// `abi3-py3*`/`abi3t-py3*` cargo features, ignoring them (in favor of a | ||
| /// version-specific build) when the interpreter cannot target the | ||
| /// requested stable ABI. Must be called from a PyO3 crate build script. | ||
| pub fn from_cargo_features( |
There was a problem hiding this comment.
Possibly private
| pub fn from_cargo_features( | |
| fn from_cargo_features( |
| let target_abi = PythonAbi::from_build_env(implementation, version, None, gil_disabled)?; | ||
| let stable_abi = | ||
| applicable_stable_abi_at_interpreter_version(implementation, version, gil_disabled); | ||
| let target_abi = | ||
| PythonAbi::from_stable_abi(implementation, version, stable_abi, gil_disabled)?; |
There was a problem hiding this comment.
Looking again, I wonder if this function from_sysconfigdata (is public) should be purely about parsing that sysconfigdata and selecting a version-specific ABI with no pollution from pyo3 features (almost feels like a bug that it isn't).
Would mean deferring actual target abi selection and lib name to cross_compile_from_sysconfigdata, probably.
There was a problem hiding this comment.
It turns out make_cross_compile_config runs apply_build_env() on everything cross_compile_from_sysconfigdata returns so we can just add a dummy version-specific target ABI in this function.
It is a behavior change though so it doesn't go in the backport commit.
ngoldbaum
force-pushed
the
fix-stable-abi-3.14t
branch
from
41d139c to
c10afb8
Compare
ngoldbaum
force-pushed
the
fix-stable-abi-3.14t
branch
from
c10afb8 to
bb4d4ab
Compare
2 participants
Fixes #6176.
Replaces the inconsistent implementations of this logic with a call into a shared helper.
Unfortunately
PythonAbi::from_build_envis public API so I had to deprecate it. I landed on newPythonAbi::from_stable_abifunction to replace it. It accepts an explicitOption<(StableAbi, PythonVersion)argument. The explicitStableAbiin the tuple allows selecting the target stable ABI, which isn't passed intofrom_build_env.I also had to change the signature of a few functions to accept a
Option<StableAbiVersion>instead of anOption<PythonVersion>. The former allows disambiguating between "no stable ABI version requested" and "no stable ABI requested at all".I made this fix in two commits, the first can be backported for 0.29.1 (#6151) and fixes the issue reported in #6176.
AI disclosure
I used an AI harness to help debug this and iterate on the solution.