Skip to content

add PythonABI struct and use it for pyo3-build-config InterpreterConfig#5924

Open
ngoldbaum wants to merge 5 commits intoPyO3:mainfrom
ngoldbaum:abi-tag-refactor
Open

add PythonABI struct and use it for pyo3-build-config InterpreterConfig#5924
ngoldbaum wants to merge 5 commits intoPyO3:mainfrom
ngoldbaum:abi-tag-refactor

Conversation

@ngoldbaum
Copy link
Copy Markdown
Contributor

@ngoldbaum ngoldbaum commented Mar 30, 2026

Towards #5786.

Refactor pyo3_build_config::impl_::InterpreterConfig to use an enum to represent the kinds of stable ABI instead of boolean abi3 flag. 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.

@ngoldbaum ngoldbaum force-pushed the abi-tag-refactor branch 2 times, most recently from 2e92e57 to 4f2177f Compare March 30, 2026 18:37
messense added a commit to PyO3/maturin that referenced this pull request Mar 31, 2026
#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>
Copy link
Copy Markdown
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, some hazy thoughts here, not sure if I'm being helpful throwing these out there.

Comment thread pyo3-build-config/src/impl_.rs Outdated
pub abi3: bool,
/// Serialized to `stable_abi`.
pub stable_abi: CPythonABI,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, ok, I'll try to take a look at removing this field from this struct.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In light of #5960 I wonder whether really what's needed is that instead of an abi3 boolean field, or an enum with just abi3 / abi3t, we want the enum to also have versioned forms, e.g. cp38-abi3, cp315-abi3t etc. (chosen in this form to match the wheel naming convention, see e.g. cryptography wheel names)

That might be good enough to detach "ABI Version" from "Host Interpreter Version".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense. That's actually the design I used in Maturin and it probably makes sense to use more-or-less the same struct wrapping two enums over here:

https://github.com/PyO3/maturin/blob/f19867f332398b8d90c06464406be6762a4e1cac/src/bridge/mod.rs#L117-L179

I'll look at updating this and see if a fix to #5960 automatically falls out of that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just pushed another even bigger refactor to abstract the implementation, ABI "kind", and ABI target version, into a PythonAbi struct. I'm not sure whether #5690 is fixed. I do like how this simplifies the interfaces a little. I also like how the builder pattern makes certain states become errors.

It looks like I still need to clean up CI but I'd also appreciate your opinion on what I have so far.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davidhewitt @Icxolu gentle ping for feedback on this approach

Comment thread pyo3-build-config/src/impl_.rs Outdated
Comment on lines +232 to +238
match self.stable_abi {
CPythonABI::ABI3 => {
if !self.is_free_threaded() {
out.push("cargo:rustc-cfg=Py_LIMITED_API".to_owned());
}
}
CPythonABI::VersionSpecific => {}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, is there a reason that is_free_threaded() && abi3 is not good enough to imply abi3t?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes complete sense

@ngoldbaum ngoldbaum changed the title add CPythonABI enum for pyo3-build-config InterpreterConfig add PythonABI struct and use it for pyo3-build-config InterpreterConfig Apr 15, 2026
@ngoldbaum ngoldbaum force-pushed the abi-tag-refactor branch 2 times, most recently from 51ff27b to dc5c1d1 Compare April 17, 2026 21:18
/// Python `X.Y` version. e.g. `3.9`.
///
/// Serialized to `version`.
pub version: PythonVersion,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is putting implementation and version into the ABI struct like I've done here OK? In particular, is the way I've changed the serialization for the config OK?

Notably, I'm pretty sure this change would break this code in maturin:

https://github.com/PyO3/maturin/blob/2582ab3763cf7b9d4b9dce25bc4c4299f074d914/src/python_interpreter/config.rs#L459-L501

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if there's potentially a case to have two version fields - one is the host interpreter version (if known) and one is the output abi3 version. (e.g. can discover a Python 3.14 interpreter but build for 3.11 stable ABI).
I wonder if there's a similar argument for implementation, though it seems rare that users want to build for a different interpreter than the host?

If we decide not to have two fields, it may be desirable to mark the old ones #[deprecated] rather than immediate removal.

Copy link
Copy Markdown
Contributor Author

@ngoldbaum ngoldbaum Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to play around with leaving the version and implementation fields in the InterpreterConfig struct, representing the "host" interpreter, and use the ABI struct to represent the target ABI and see if that's cleaner. Thanks for taking a look!

Copy link
Copy Markdown
Member

@Icxolu Icxolu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't worked too much inside the build code, so I'm not sure that I can help much here, but I gave this a brief read and left some comments.

I do like the builder pattern. I think it's nice to have a different type between the abi configuring phase and the usage phase (even tho it does not protect us from forgetting to configure something)

/// The original stable ABI, supporting Python 3.2 and up
Abi3,
/// Version specific ABI, which may be different on the free-threaded build (true) or gil-enabled build (false)
VersionSpecific(bool),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really love using bools (especially unnamed ones) for these things. It tends to get hard to follow what a true and false mean. I'd prefer to just split this into two separate variants.
Also: Should the version specific variant even have this distinction, given that we also have the interpreter build flag to determine if we should enable Py_GIL_disabled? If they serve the same function, we should settle for one I would say, and if they serve different function, that difference is not clear to me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also: Should the version specific variant even have this distinction, given that we also have the interpreter build flag to determine if we should enable Py_GIL_disabled? If they serve the same function, we should settle for one I would say, and if they serve different function, that difference is not clear to me.

I think I'd prefer to keep this information in the ABI struct and I guess given what David is saying, we can think of the ABI struct as containing information about the target ABI rather than the ABI of the host interpreter. The build flags are populated from the host interpreter and with the advent of a free-threaded stable ABI, it makes more sense to talk about cross-compiling for free-threaded builds rather than always requiring a free-threaded host interpreter to build.

Copy link
Copy Markdown
Member

@Icxolu Icxolu Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense to me. However I think it would be great to have a better distinction between host and target abi, so that it is always clear which one I'm looking at. Additionally I think this means that we should emit any cfgs purely based on the target abi, and only use the host abi to initialize the target if neccessary. Py_GIL_DISABLED is for example emitted based on host abi currently if i'm reading this correctly

if self.version < PythonVersion::PY313 {
let version = self.version;
bail!(
"Free-threaded builds on Python versions before 3.13, tried to build for {version}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not really read like a sentence to me

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum PythonAbiKind {
/// The original stable ABI, supporting Python 3.2 and up
Abi3,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: It's kind of inconsistent how we name this now. On one hand you renamed a bunch of previous "abi3" fields to "stable abi", but also introduced a lot of new "abi3" naming.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is we'd grow an Abi3t variant alongside this one to support PEP 803. I'll go ahead and implement David's suggestion to add build support for abi3t in this PR, which should make it clearer what the end state we're going for is.

Copy link
Copy Markdown
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have taken a look through some parts, I think I have a couple of pieces that I'd like clarifying in this PR:

  • I can't decide if we want to separate "host version" from target abi version. That seemed like it might help in #5960, but maybe it adds complexity for no practical benefit.
  • I think it'd be helpful to introduce the possibility to configure for abi3t in this PR through a config file, even if the full end-to-end build with abi3t is not done here (we could maybe just halt the build if abi3t is selected for now). I think that'd make it easier to see the full end state possible states we're heading towards, plus help understand where "stable abi" vs "abi3 and abi3t" are the right names.

/// Python `X.Y` version. e.g. `3.9`.
///
/// Serialized to `version`.
pub version: PythonVersion,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure if there's potentially a case to have two version fields - one is the host interpreter version (if known) and one is the output abi3 version. (e.g. can discover a Python 3.14 interpreter but build for 3.11 stable ABI).
I wonder if there's a similar argument for implementation, though it seems rare that users want to build for a different interpreter than the host?

If we decide not to have two fields, it may be desirable to mark the old ones #[deprecated] rather than immediate removal.

Comment on lines -110 to -113
/// Whether linking against the stable/limited Python 3 API.
///
/// Serialized to `abi3`.
pub abi3: bool,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably the case that we can leave this field in-place and mark it #[deprecated], to give users a chance to migrate even if we no longer use it internally.

Comment on lines -540 to -543
"implementation" => parse_value!(implementation, value),
"version" => parse_value!(version, value),
"abi" => parse_value!(abi, value),
"shared" => parse_value!(shared, value),
"abi3" => parse_value!(abi3, value),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid breaking maturin I guess we could handle these backwards-compatibly somehow?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants