add CPythonABI enum for pyo3-build-config InterpreterConfig#5924
add CPythonABI enum for pyo3-build-config InterpreterConfig#5924
Conversation
ngoldbaum
force-pushed
the
abi-tag-refactor
branch
from
91eeba3 to
2e92e57
Compare
ngoldbaum
force-pushed
the
abi-tag-refactor
branch
from
2e92e57 to
4f2177f
Compare
#3110) Towards #3064. This is purely refactoring, there should be no functional changes as a result of this. Currently the build metadata special-cases ABI3 builds or more generally assumes stable ABI builds and ABI3 builds are the same thing. With PEP 803 and the new abi3t ABI in Python 3.15, that is no longer the case. This replaces the old `ABI3Version` enum with a new struct combining two enums: ```rust /// struct describing ABI layout to use for build #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct StableAbi { /// The "kind" of stable ABI. Either abi3 or abi3t currently. pub kind: StableAbiKind, /// The minimum Python version to build for. pub version: StableAbiVersion, } /// Python version to use as the abi3/abi3t target. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum StableAbiVersion { /// Stable ABI wheels will have a minimum Python version matching the /// version of the current Python interpreter CurrentPython, /// Stable ABI wheels will have a fixed user-specified minimum Python /// version Version(u8, u8), } /// The "kind" of stable ABI. Either abi3 or abi3t currently. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum StableAbiKind { /// The original stable ABI, supporting Python 3.2 and up Abi3, } ``` `StableAbiVersion` is just the old `Abi3Version` enum renamed since the concept of a minimum supported version is shared by abi3t. I have [a branch](main...ngoldbaum:maturin:abi3t) that adds an `Abi3t` variant for `StableAbiKind`. My goal with this PR is to make reviewing the subsequent PR adding abi3t support easier. Also see PyO3/pyo3#5924 where I made a similar change in PyO3. Here in Maturin I needed different types but in principle I could make the two implementations use shared code. I'm not sure if that's actually useful for anything in practice. --------- Co-authored-by: messense <messense@icloud.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, some hazy thoughts here, not sure if I'm being helpful throwing these out there.
| pub abi3: bool, | ||
| /// Serialized to `stable_abi`. | ||
| pub stable_abi: CPythonABI, | ||
|
|
There was a problem hiding this comment.
If we're changing this - I have a vague recollection that in the past I'd concluded having this field in the InterpreterConfig might be unecessary, as it's really just a property of the final build, not of the actual resolved interpreter. (I guess the idea would be to split "build config" from "discovered interpreter". But maybe that's not actually useful complexity.)
I can't remember exactly why I was thinking that. Maybe in relation to #4241.
There was a problem hiding this comment.
Hmm, ok, I'll try to take a look at removing this field from this struct.
There was a problem hiding this comment.
I'm not saying you should do so, I have no idea whether it was actually a sound thought and it doesn't seem like I wrote it down either.
I guess just observing that if we're making changes here, I vaguely recall wanting an alternative.
| match self.stable_abi { | ||
| CPythonABI::ABI3 => { | ||
| if !self.is_free_threaded() { | ||
| out.push("cargo:rustc-cfg=Py_LIMITED_API".to_owned()); | ||
| } | ||
| } | ||
| CPythonABI::VersionSpecific => {} |
There was a problem hiding this comment.
Just curious, is there a reason that is_free_threaded() && abi3 is not good enough to imply abi3t?
There was a problem hiding this comment.
That makes it impossible to build abi3t wheels using a GIL-enabled interpreter. We could choose to do that but that would be an entirely arbitrary restriction: there's nothing inherent about the free-threaded interpreter to make these builds possible.
It also makes the work in maturin easier, as I can just check for someone enabling an abi3t-py3xx or abi3t feature.
2 participants
Towards #5786.
Refactor
pyo3_build_config::impl_::InterpreterConfigto use an enum to represent the kinds of stable ABI instead of booleanabi3flag. Also replace names that contain "abi3" with "stable_abi".This is extracted from a branch that enables abi3t builds and Python 3.15 stable ABI support, where I add a third enum variant to represent abi3t. My goal here is to make upstreaming that change simpler. PEP 803 was accepted over the weekend so a new ABI is definitely happening.
I also personally find the enum clearer to understand and easier to read code that uses it instead of the boolean flag.