PyO3 / maturin

Build and publish crates with pyo3, cffi and uniffi bindings as well as rust binaries as python packages
https://maturin.rs
Apache License 2.0
3.96k stars 275 forks source link

Add free-threaded Python support #2310

Open messense opened 1 week ago

messense commented 1 week ago

Implementation details:

Closes #2298 Closes #2315

messense commented 1 week ago

This change is part of the following stack:

Change managed by git-spice.

davidhewitt commented 4 days ago

For testing it's possible to use https://github.com/Quansight-Labs/setup-python as a drop-in replacement for the GitHub one

ngoldbaum commented 3 days ago

Happy to help with this if needed. The windows free-threaded packaging situation is "fun".

messense commented 3 days ago

Happy to help with this if needed.

Thanks, feel free to send PR to this branch!

ngoldbaum commented 2 days ago

It looks like a few of the tests hardcode using abi3:

○  rg "abi3" **/Cargo.toml
pyo3-pure/Cargo.toml
10:pyo3 = { version = "0.22.0", features = ["abi3-py37", "extension-module", "generate-import-lib"] }

pyo3-abi3-without-version/Cargo.toml
2:name = "pyo3-abi3-without-version"
8:pyo3 = { version = "0.22.4", features = ["abi3", "extension-module"] }
11:name = "pyo3_abi3_without_version"

workspace-inverted-order/path-dep-with-root/Cargo.toml
13:pyo3 = { version = "0.22.5", features = ["abi3", "abi3-py38", "extension-module"] }

pyo3-ffi-pure/Cargo.toml
7:pyo3-ffi = { version = "0.18.1", features = ["abi3-py37", "extension-module"] }

Since free-threading and abi3 are incompatible, would it be better to skip over the tests that use these crates? For the ones that are testing whether the limited api support is working correctly I think skipping them makes sense, but some of these seem to be more generic integration tests for maturin that happen to use the limited API.

I don't see an easy way to optionally turn on and off the abi3 feature in the test crates. Am I missing something? If not, any guidance?

davidhewitt commented 2 days ago

We should probably try to support this case because cryptography probably hard-codes the abi3 build in the same way. It's useful to do that because then the whole system limits you to the limited API.

My gut says that it'd be good enough for now if Py_GIL_DISABLED being set caused pyo3-build-config to no-op the whole lot of abi3 features. So maybe there's a PyO3 fix required here? 🤔

ngoldbaum commented 2 days ago

My gut says that it'd be good enough for now if Py_GIL_DISABLED being set caused pyo3-build-config to no-op the whole lot of abi3 features. So maybe there's a PyO3 fix required here?

Hmm, that's sort of in the vein of https://github.com/PyO3/pyo3/pull/4719, but maybe what I'm doing there making more things hard errors is bad and it would be better to warn and just turn off limited API and set the ABI to Py_3_13 if someone is trying to build for the free-threaded ABI but asks for an earlier ABI.

alex commented 2 days ago

We should probably try to support this case because cryptography probably hard-codes the abi3 build in the same way. It's useful to do that because then the whole system limits you to the limited API.

We have a convoluted setup where Cargo.toml specifies the abi3 feature, pyproject.toml specifies the pyo3/abi3-py37 feature, and then some individual wheel building jobs specify higher abi3 ABIs.

davidhewitt commented 2 days ago

👍 It seems to me that we probably don't want cryptography to have to remove abi3 feature from Cargo.toml to build against the freethreaded build, so silently overriding (like we do for PyPy & Graalpy) seems more correct. I guess we can probably also make that work for the abi3-py37 too, I'll check how that works...

davidhewitt commented 2 days ago

Yep, if I add abi3-py37 to Cargo.toml and then try to build with PyPy, the build succeeds (with a warning about PyPy not supporting abi3), I guess we should match here.

ngoldbaum commented 2 days ago

It looks like three of the crates use old PyO3 APIs and need to be updated:

test-crates/sdist_with_target_path_dep/src/lib.rs
18:    let pool = ::pyo3::GILPool::new();
116:    let pool = ::pyo3::GILPool::new();

test-crates/lib_with_path_dep/src/lib.rs
18:    let pool = ::pyo3::GILPool::new();
116:    let pool = ::pyo3::GILPool::new();

test-crates/sdist_with_path_dep/src/lib.rs
18:    let pool = ::pyo3::GILPool::new();
116:    let pool = ::pyo3::GILPool::new();

Other errors from trying to build one of these crates:

``` Compiling sdist_with_path_dep v0.1.0 (/Users/goldbaum/Documents/maturin/test-crates/sdist_with_path_dep) error[E0432]: unresolved import `pyo3::derive_utils` --> src/lib.rs:113:15 | 113 | use pyo3::derive_utils::ModuleDef; | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `GILPool` in `pyo3` --> src/lib.rs:18:24 | 18 | let pool = ::pyo3::GILPool::new(); | ^^^^^^^ could not find `GILPool` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:26:47 | 26 | const PARAMS: &'static [pyo3::derive_utils::ParamDescription] = &[ | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:27:27 | 27 | pyo3::derive_utils::ParamDescription { | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:32:27 | 32 | pyo3::derive_utils::ParamDescription { | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:41:46 | 41 | let (_args, _kwargs) = pyo3::derive_utils::parse_fn_args( | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:53:44 | 53 | .map_err(|e| pyo3::derive_utils::argument_extraction_error(_py, "x", e))?, | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:61:44 | 61 | .map_err(|e| pyo3::derive_utils::argument_extraction_error(_py, "y", e))?, | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `callback` in `pyo3` --> src/lib.rs:85:17 | 85 | ::pyo3::callback::callback_error() | ^^^^^^^^ could not find `callback` in `pyo3` error[E0433]: failed to resolve: could not find `derive_utils` in `pyo3` --> src/lib.rs:89:27 | 89 | args: impl Into>, | ^^^^^^^^^^^^ could not find `derive_utils` in `pyo3` error[E0433]: failed to resolve: could not find `GILPool` in `pyo3` --> src/lib.rs:116:24 | 116 | let pool = ::pyo3::GILPool::new(); | ^^^^^^^ could not find `GILPool` in `pyo3` error[E0433]: failed to resolve: could not find `callback` in `pyo3` --> src/lib.rs:137:17 | 137 | ::pyo3::callback::callback_error() | ^^^^^^^^ could not find `callback` in `pyo3` error[E0433]: failed to resolve: could not find `callback` in `pyo3` --> src/lib.rs:22:17 | 22 | ::pyo3::callback::convert(_py, { | ^^^^^^^^ could not find `callback` in `pyo3` error[E0433]: failed to resolve: could not find `callback` in `pyo3` --> src/lib.rs:120:17 | 120 | ::pyo3::callback::convert(_py, MODULE_DEF.make_module("", pyo3_pure)) | ^^^^^^^^ could not find `callback` in `pyo3` warning: use of deprecated type alias `pyo3::PyMethodType`: PyO3 implementation detail --> src/lib.rs:97:22 | 97 | pyo3::class::PyMethodType::PyCFunctionWithKeywords(__pyo3_raw_add), | ^^^^^^^^^^^^ | = note: `#[warn(deprecated)]` on by default error[E0061]: this function takes 3 arguments but 5 arguments were supplied --> src/lib.rs:94:5 | 94 | pyo3::types::PyCFunction::internal_new( | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 95 | name, | ---- expected `Python<'_>`, found `&CStr` 96 | doc, | --- expected `&PyMethodDef`, found `&CStr` 97 | pyo3::class::PyMethodType::PyCFunctionWithKeywords(__pyo3_raw_add), 98 | pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS, | -------------------------------------------------- unexpected argument #4 of type `i32` 99 | args.into(), | ----------- unexpected argument #5 | = note: expected reference `&pymethods::PyMethodDef` found reference `&CStr` note: expected `Option<&Bound<'_, ...>>`, found `PyMethodType` --> src/lib.rs:97:9 | 97 | pyo3::class::PyMethodType::PyCFunctionWithKeywords(__pyo3_raw_add), | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: expected enum `Option<&pyo3::Bound<'_, pyo3::types::PyModule>>` found enum `PyMethodType` note: associated function defined here --> /Users/goldbaum/Documents/pyo3/src/types/function.rs:151:12 | 151 | pub fn internal_new<'py>( | ^^^^^^^^^^^^ help: remove the extra arguments | 95 ~ /* Python<'_> */, 96 ~ /* &pymethods::PyMethodDef */, 97 ~ /* Option<&pyo3::Bound<'_, pyo3::types::PyModule>> */, | error[E0308]: mismatched types --> src/lib.rs:94:5 | 90 | ) -> pyo3::PyResult<&'a pyo3::types::PyCFunction> { | -------------------------------------------- expected `Result<&'a pyo3::types::PyCFunction, PyErr>` because of return type ... 94 | / pyo3::types::PyCFunction::internal_new( 95 | | name, 96 | | doc, 97 | | pyo3::class::PyMethodType::PyCFunctionWithKeywords(__pyo3_raw_add), 98 | | pyo3::ffi::METH_VARARGS | pyo3::ffi::METH_KEYWORDS, 99 | | args.into(), 100 | | ) | |_____^ expected `Result<&PyCFunction, PyErr>`, found `Result, ...>` | = note: expected enum `Result<&'a pyo3::types::PyCFunction, _>` found enum `Result, _>` error[E0277]: the trait bound `Result<&pyo3::types::PyCFunction, PyErr>: IntoPyCallbackOutput<'_, Py>` is not satisfied --> src/lib.rs:104:7 | 104 | m.add_wrapped(&__pyo3_get_function_add)?; | ^^^^^^^^^^^ the trait `IntoPyCallbackOutput<'_, Py>` is not implemented for `Result<&pyo3::types::PyCFunction, PyErr>` | = help: the trait `IntoPyCallbackOutput<'py, U>` is implemented for `Result` note: required by a bound in `add_wrapped` --> /Users/goldbaum/Documents/pyo3/src/types/module.rs:311:12 | 309 | fn add_wrapped(&self, wrapper: &impl Fn(Python<'py>) -> T) -> PyResult<()> | ----------- required by a bound in this associated function 310 | where 311 | T: IntoPyCallbackOutput<'py, PyObject>; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `PyModuleMethods::add_wrapped` Some errors have detailed explanations: E0061, E0277, E0308, E0432, E0433. For more information about an error, try `rustc --explain E0061`. warning: `sdist_with_path_dep` (lib) generated 1 warning error: could not compile `sdist_with_path_dep` (lib) due to 17 previous errors; 1 warning emitted ```

I'm confused why the update to PyO3 0.23 last week didn't break tests using these crates on CI.

messense commented 4 hours ago

I'm confused why the update to PyO3 0.23 last week didn't break tests using these crates on CI.

Some of these tests are not built but only used to test generating source distribution, will fix them later in a separate PR.