cryptography
cryptography copied to clipboard
pyca
Switch to maturin.
TODO before mergable:
- [x] Get this working in CI...
- [x] Figure out how to handle version numbers
- [x] Make sure the abi3 stuff works with PyPy
- [x] Update wheel builder to no longer use
setup.pydirectly (this should be done in a separate PR) - [x] Figure out what the correct behavior with respect to
MANIFEST.inis - [x] Maturin stable release
- [x] Figure out how maturin's MSRV interacts with our MSRV
Hey, maturin's sdist support is be a bit hacky so you may run into obscure problems, feel free to ping me or open an issue if something doesn't work.
Thanks!
On Thu, Apr 27, 2023 at 12:46 AM messense @.***> wrote:
Hey, maturin's sdist support is be a bit hacky https://github.com/PyO3/maturin/issues?q=is%3Aissue+is%3Aopen+label%3Asdist so you may run into obscure problems, feel free to ping me or open an issue if something doesn't work.
— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/8815#issuecomment-1524694203, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBB6XJP36EAGJRCWNK3XDH23ZANCNFSM6AAAAAAXKGUW3Y . You are receiving this because you authored the thread.Message ID: @.***>
-- All that is necessary for evil to succeed is for good people to do nothing.
@messense is there a way to disable the rewriting things to be local_dependencies? My preference is to just include everything at its normal location with include.
is there a way to disable the rewriting things to be
local_dependencies?
No way ATM, I do want to support it to avoid https://github.com/PyO3/maturin/issues/1442.
Would it be useful to file a bug for that, or should I just follow 1442?
On Tue, May 2, 2023 at 3:05 AM messense @.***> wrote:
is there a way to disable the rewriting things to be local_dependencies?
No way ATM, I do want to support it to avoid PyO3/maturin#1442 https://github.com/PyO3/maturin/issues/1442.
— Reply to this email directly, view it on GitHub https://github.com/pyca/cryptography/pull/8815#issuecomment-1530986121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBDG2335LUKBOQ772SLXECWZZANCNFSM6AAAAAAXKGUW3Y . You are receiving this because you authored the thread.Message ID: @.***>
-- All that is necessary for evil to succeed is for good people to do nothing.
Would it be useful to file a bug for that, or should I just follow 1442? …
Yeah, file a bug is useful.
@messense Is it intentional that maturin doesn't set the PYO3_PYTHON env var when running cargo? setuptools-rust does (https://github.com/PyO3/setuptools-rust/blob/main/setuptools_rust/build.py#L600), and we use it in our build.rs (I think you even wrote that code :-))
Specifically, this is happening when pip wheel invokes ['maturin', 'pep517', 'build-wheel', '-i', '/Users/alex_gaynor/projects/cryptography/.venv/bin/python3.11', '--compatibility', 'off'], so I'd expect the -i value to go into PYO3_PYTHON.
🔗 Found pyo3 bindings with abi3 support for Python ≥ 3.7
🐍 Not using a specific python interpreter
I think it has something to do with abi3, we can certainly pass down the interpreter path if it's specified. I can take a look this weekend.
Ok, we are green here, huzzah!
The remaining things we need are: a) Maturin stable release (this currently uses a beta), b) @reaperhulk we need to discuss MSRV implications:
maturin currently has an MSRV of 1.64, which is obviously higher than our MSRV. It also has wheels for every possible platform it seems, https://pypi.org/project/maturin/1.0.0b9/#files
After discussion with @reaperhulk, we think the MSRV is too aggressive. Going to hold this in draft for now.
maturin currently has an MSRV of 1.64, which is obviously higher than our MSRV.
I'm open to reduce MSRV to 1.63 to match Debian stable (bookworm) if it helps.
Maybe! We're still at 1.56, and are discussing going to 1.60 in our next release (https://github.com/pyca/cryptography/pull/9043), so either way it's going to be a little while for us. This is going to be a process.
@messense FWIW, looking at the data some more, I do think 1.63.0 is going to be easier for us than 1.64.0, because of Debian.
I do think 1.63.0 is going to be easier for us than 1.64.0, because of Debian.
Let me know when you plan to switch, I'll try to get a new maturin release out with MSRV of 1.63.0 for Debian.
BTW, consider upgrade to maturin 1.3.1 in this PR, it's currently using 1.2.0.
Our next release is going to have an MSRV of 1.63.
And yes, I should bump the pin, I've mostly been lazy -- I've only been rebasing to resolve conflicts, and I've been assuming dependabot would bump it for me when we finally merge :-)
And I should have said: Thank you for considering a release with a lower MSRV!
FYI, there is already a python-maturin package in Debian unstable, but it's a bit old, version is 1.1.0. Not sure when will it go to Debian stable.
The case we want to support is people who are installing packages with pip, and not using wheels.
The case we want to support is people who are installing packages with
pip, and not using wheels.
In that case I think lowering MSRV of maturin only helps a bit but creates too much maintenance burden for maturin. If people are installing packages with pip on Debian, they are also fine to use rustup to install a recent version of Rust I think?
I definitely understand the maintenance burden concern. Thanks for considering it.
This is probably worth a release note? (and should be remilestoned to 43?)
There's no public API implicated here -- the only installation method we have ever documented support for is pip (and perhaps by extension other installers that are PEP517 compatible).
Good catch re:milestone though.
Your call of course. There are cases where someone might hit this (e.g. installation without build isolation, as distributors are wont to do), but it's not hard to find out and fix. Still, I'd tend to add it for completeness 🤷♂️
Am I correct in understanding that now maturin is used it is no longer possible to build non-abi3 wheels if you're building locally for a single python version?
I think it may be possible with build-args (ala https://github.com/pyca/cryptography/blob/main/.github/workflows/wheel-builder.yml#L130), by simply not setting abi3. But I haven't tested, nor did we ever previously test building non-abi3 artifacts.
If there's something we can do to improve this, let us know.
It assumed it did work before, because when you simply run the build backend without any particular options, the previous version installed
/usr/lib/python3.12/site-packages/cryptography/hazmat/bindings/_rust.cpython-312-x86_64-linux-gnu.so
whereas the new version installs
/usr/lib/python3.12/site-packages/cryptography/hazmat/bindings/_rust.abi3.so
I futzed around a bit in the current source, and was able to convince maturin to build cpython-312-x86_64-linux-gnu.so extensions by deleting all the features = ["abi3"] from various toml files and removing pyo3/abi3-py37 from pyproject.toml, so maybe I'm wrong to think the old version was actually optimized for a specific python version (and instead it was building against the abi3 API/ABI but ignoring the "abi3" extension name?).
As far as I can tell, it's not actually possible to disable a single feature in cargo, though there's an open issue from 2016 asking for the ability. (?)
I know when I helped someone implement Limited API support in meson, the resulting implementation took:
- an attribute on the config for an extension module, asking for Limited API support and specifying the minimum version, which internally handles
-DPy_LIMITED_API=0x3...........for you - a builtin (not project-specific) build option
python.allow_limited_apiwhich can be manually toggled by the person doing a local build to opt out of the whole thing entirely
which I figured was a good balance.
I think it may be possible with
build-args(ala https://github.com/pyca/cryptography/blob/main/.github/workflows/wheel-builder.yml#L130), by simply not setting abi3.
It appears this is set in pyproject.toml anyway. :shrug: If I try to remove it on its own, maturin spawns an error message:
💥 maturin failed
Caused by: You have selected the `abi3` feature but not a minimum version (e.g. the `abi3-py36` feature). maturin needs a minimum version feature to build abi3 wheels.