Skip to content
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

RFC: make abi3 builds the default, add feature for unlimited ABI #2865

Open
davidhewitt opened this issue Jan 8, 2023 · 16 comments
Open

RFC: make abi3 builds the default, add feature for unlimited ABI #2865

davidhewitt opened this issue Jan 8, 2023 · 16 comments

Comments

@davidhewitt
Copy link
Member

No description provided.

@davidhewitt
Copy link
Member Author

davidhewitt commented Jan 8, 2023

(posted issue without content originally, sorry)

I'm wondering if we should change features around so that abi3 aka Py_LIMITED_API is set by default. To use the unlimited API functionality we then can have an unlimited-api (or unstable-python-api or any other bikeshedded name) feature.

Advantages:

  • Helps users build abi3 compatible builds by default, which I think is a good habit if the more complex version-specific builds aren't necessary
  • The unlimited-api feature would be additive, which fits normal Rust idioms
  • Additive features are easier to test on CI, so we might be able to speed up CI runs a fair bit (because we'll need to run fewer feature combinations)

Disadvantage would be churn induced on users and tools like maturin. We could make the unstable-python-api a default feature for a few versions to make the migration easier.

@messense I'd be interested to hear what you think of this from a packaging / builds side.
@encukou perhaps you'd be willing to share Python core dev opinion on whether we should be encouraging stable api compatibility as a default?

@messense
Copy link
Member

messense commented Jan 8, 2023

IMO it's not a big issue for maturin, we can detect pyo3 version and change the abi3 detection logic to

  • unstable-python-api/unlimited-api features means non-abi3
  • otherwise it's abi3

It might be an issue for setuptools-rust? Since you need to build with --py-limited-api=cpXY for abi3.

@birkenfeld
Copy link
Member

I'm not against changing the default, as long as any change in behavior is clearly visible with new compile errors.

Please don't add "unstable" to the feature name, the API should be perfectly stable within the Python minor version that the extension is compiled for.

@alex
Copy link
Contributor

alex commented Jan 8, 2023

One note: it'd be useful if pyo3 still supported specifying the abi3 minimum version if it differs from the current python.

For cryptography on Windows and macOS we build abi-py36 wheels using newer Pythons.

@davidhewitt
Copy link
Member Author

One note: it'd be useful if pyo3 still supported specifying the abi3 minimum version if it differs from the current python.

Definitely; I was thinking the existing abi3-py3x features would all remain, the meaning of them being unchanged. The abi3 feature could exist as a no-op for a while for backwards compatibility too.

@encukou
Copy link

encukou commented Jan 9, 2023

@encukou perhaps you'd be willing to share Python core dev opinion on whether we should be encouraging stable api compatibility as a default?

Well I'm a biased CPython developer, so you might want other opinions, but as far as I'm concerned, go for it :)

Don't use the term unstable API, that might mean something slightly different soon. Unlimited sounds good.

@konstin
Copy link
Member

konstin commented Jan 17, 2023

I'd also like to see abi3 be the default. For naming, what about a feature called no-abi3 and features py38, py39, etc. to select the minimum target version in either case?

@messense
Copy link
Member

what about a feature called no-abi3

I feel that no-abi3 seems to imply that it's opt-out of some feature rather than opt-in to version specific feature, given that features in Rust is additive it's a bit weird.

and features py38, py39, etc. to select the minimum target version in either case?

I like this!

@adamreichold
Copy link
Member

Considering the apparent bike-sheddability of this, I deferring such a policy decision to Maturin wouldn't be a simpler first step that would help a lot to move the ecosystem into this direction. Especially for newcomers who have not yet found about the PyO3 features themselves.

@davidhewitt
Copy link
Member Author

NB #2901 has indicated that if we did this, users embedding Python would get no benefit (they would effectively be tied to a version-specific Python anyway).

Not a blocker, just something to document I think.

@davidhewitt
Copy link
Member Author

A reason to want this: libraries can accidentally rely on features from the unlimited API and then have users fail when running abi3 builds. If the default build mode was abi3 then libraries wouldn't run into this footgun.

davidhewitt/pythonize#59 (comment)

@LilyFoote
Copy link
Contributor

LilyFoote commented Apr 3, 2024

I think adding this would cause less friction with #4008. I'm using apis that aren't available with abi3, so being able to gate behind unlimited-api would make sense.

@LilyFoote
Copy link
Contributor

@davidhewitt I'm open to doing a bunch of the work implementing this, since I'd like to be able to use it. Do you have any thoughts about when we should aim to land this? Is it something that can happen parallel to the gil-refs work or should it wait until after?

@davidhewitt
Copy link
Member Author

I'm also reasonably keen for this but given it creates churn for user's builds when upgrading I think either:

  • we need to find ways to attach carrots to this (e.g. make sure their CI changes automatically to use a single build, if they're using maturin), or
  • we need to pick a tactical moment to do this when there's not a load of other necessary churn

I think with the soundness adjustments in 0.22 it's probably a bit mean, but maybe in 0.23?

@LilyFoote
Copy link
Contributor

I'll make a start with a draft PR, but I'll not expect it to land for 0.22 then.

@davidhewitt
Copy link
Member Author

Just realised that at the moment we have a divergence between Py_LIMITED_API and feature = "abi3" cfg flags, because it's possible to set abi3=false through the PyO3 config file and then the state of --feature=abi3 gets ignored.

#4185 (comment)

Might influence how we want to redesign things.

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

No branches or pull requests

8 participants