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

Pass pyo3/abi3-pyXX feature to pyo3 automatically #137

Merged
merged 7 commits into from
Mar 28, 2021
Merged

Pass pyo3/abi3-pyXX feature to pyo3 automatically #137

merged 7 commits into from
Mar 28, 2021

Conversation

messense
Copy link
Member

No description provided.

@davidhewitt
Copy link
Member

Thank you for all of these PRs! I have some time later this afternoon so will review all of them then.

Copy link
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! I took a long time to think about this, and decided to create a PR to add some extra functionality on top of this commit. See https://github.com/messense/setuptools-rust/pull/1

I made it a separate PR onto your branch because I wanted to give you a chance to review it.

Once that is merged into this branch, I'm happy to push a CHANGELOG entry and more docs here.

setuptools_rust/build.py Outdated Show resolved Hide resolved
setuptools_rust/setuptools_ext.py Outdated Show resolved Hide resolved
@davidhewitt
Copy link
Member

@alex based off #132 (comment), I'd be interested in hearing what you think of this PR.

Essentially if you set py_limited_api to True or False, the behaviour will be backwards-compatible and continue to match setuptools - you'll need to define the features as currently done.

With the new default "auto" mode, the command-line --py-limited-api option to bdist_wheel will be used to infer the build mode for the extension too, and will pass the correct feature to pyo3. I'd be curious if you'd want to use this in cryptography.

IMO it seems like a win for everyone: package maintainers can still build and distribute abi3 wheels, and users installing the package directly can build a version-specific extension. (PyO3 already uses a few optimizations when building version-specific, and I'd consider adding more.)

@alex
Copy link
Contributor

alex commented Mar 24, 2021

I think we'd probably use this in pyca/cryptography, sure.

One note: I think you'll want to have auto -> not try to enable abi3 on PyPy still.

@davidhewitt
Copy link
Member

Great!

One note: I think you'll want to have auto -> not try to enable abi3 on PyPy still.

Given that PyPy is sounding like it'll support a "stable api" for PyPy 3.8, my thought was that we should allow this in setuptools-rust so that it's future-compatible with when PyPy does add support.

PyO3 0.14 will emit a warning when abi3 is enabled for PyPy; we opted to do that so that it wouldn't cause a hard error in builds if the feature is set. PyO3 0.13.2 will silently complete the build, which imo is still fine because it'll be a working build. Setuptools just won't actually package it as an abi3 wheel.

@davidhewitt davidhewitt merged commit 934d688 into PyO3:main Mar 28, 2021
@messense messense deleted the py-limited-api branch March 28, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants