Free-threaded build config fixes #4719
Conversation
pyo3-build-config/src/impl_.rs
Outdated
| major: 3, | ||
| minor: 13, | ||
| }; | ||
| #[allow(dead_code)] |
There was a problem hiding this comment.
this is leftover from a mistaken refactoring that removed the only internal use of this outside tests and should be deleted
pyo3-build-config/src/impl_.rs
Outdated
| @@ -1642,24 +1645,33 @@ fn default_lib_name_windows( | |||
| debug: bool, | |||
| gil_disabled: bool, | |||
| ) -> String { | |||
| if debug { | |||
| if debug && version.minor < 10 { | |||
There was a problem hiding this comment.
maybe define a PY310 constant?
|
👍 thanks, do you want me to review before the above comments are resolved? |
|
Go ahead. Tiny bit of context: I often look at PRs on my phone since it helps me catch things if I look over code in a different context from where I wrote it. |
pyo3-ffi/build.rs
Outdated
| if version < PythonVersion::PY313 { | ||
| panic!( |
There was a problem hiding this comment.
This should use ensure! rather than panic!, it leads to a friendlier presentation of the error to users.
|
See also PyO3/maturin#2310 (comment), can probably adjust here. |
|
Along with some changes to maturin on top of PyO3/maturin#2310 (which I'll create a separate PR for) this latest push lets more of the maturin tests pass on my local dev setup. |
There was a problem hiding this comment.
Thanks, can confirm locally that on the PyO3 side this looks correct. I just have a few copy suggestions.
Note that maturin will also need adjusting to not build abi3 wheels in the free-threaded case, i.e. at the moment I get
📦 Built wheel for abi3 Python ≥ 3.7 to /var/folders/z2/3t02d9cn6gxg11n5yd9yk3wm0000gn/T/.tmpM81f0O/pyo3_scratch-0.1.0-cp37-abi3-macosx_11_0_arm64.whl
💥 maturin failed
Caused by: pip install in /Users/david/.virtualenvs/pyo3-3.13t failed running ["-m", "pip", "--disable-pip-version-check", "install", "--no-deps", "--force-reinstall", "/var/folders/z2/3t02d9cn6gxg11n5yd9yk3wm0000gn/T/.tmpM81f0O/pyo3_scratch-0.1.0-cp37-abi3-macosx_11_0_arm64.whl"]: exit status: 1
--- Stdout:
--- Stderr:
ERROR: pyo3_scratch-0.1.0-cp37-abi3-macosx_11_0_arm64.whl is not a supported wheel on this platform.
---
(maybe you already did that in the maturin PR).
guide/src/free-threading.md
Outdated
|
|
||
| The free-threaded build uses a completely new ABI and there is not yet an | ||
| equivalent to the limited API for the free-threaded ABI. That means if your | ||
| crate depends on PyO3 using the `abi3` feature or an an `abi-pyxx` feature, PyO3 |
There was a problem hiding this comment.
| crate depends on PyO3 using the `abi3` feature or an an `abi-pyxx` feature, PyO3 | |
| crate depends on PyO3 using the `abi3` feature or an an `abi3-pyxx` feature, PyO3 |
pyo3-build-config/src/impl_.rs
Outdated
| .unwrap(), | ||
| "python3_d", | ||
| ); | ||
| // Python versions older than 3.13 panic if gil_disabled is true |
There was a problem hiding this comment.
| // Python versions older than 3.13 panic if gil_disabled is true | |
| // Python versions older than 3.13 don't support gil_disabled |
pyo3-ffi/build.rs
Outdated
| if !interpreter_config | ||
| .build_flags | ||
| .0 | ||
| .contains(&BuildFlag::Py_GIL_DISABLED) | ||
| { | ||
| ensure!( | ||
| interpreter_config.version <= versions.max || env_var("PYO3_USE_ABI3_FORWARD_COMPATIBILITY").map_or(false, |os_str| os_str == "1"), |
There was a problem hiding this comment.
This transformation disables the version check completely on the free-threaded build; I think this is risky given testing with 3.14 free-threading is likely to start happening soon.
I think better would be to do something like
if interpreter_config.version < versions.max {
if interpreter_config.build_flags.0.contains(&BuildFlags::Py_GIL_DISABLED) {
// free-threaded specific error
} else if !env_var("PYO3_USE_ABI3_FORWARD_COMPATIBILITY").map_or(false, |os_str| os_str == "1") {
// env var not set, raise the existing error
}
}There was a problem hiding this comment.
I did this and noticed we were using interpreter_config.build_flags.0.contains(&BuildFlags::Py_GIL_DISABLED) quite a bit, so I added that as a method to the InterpreterConfig struct to make the spelling a little clearer
I think that was fixed by PyO3/maturin#2316. I also had to update all the test crates to point at a local copy of pyo3. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* update build config logic and library name generation * fix free-threaded windows clippy with --features=abi3 * use constant * add release note * apply my self-review comments * use ensure and error handling instead of panicking * skip abi3 fixup on free-threaded build * don't support PYO3_USE_ABI3_FORWARD_COMPATIBILITY on free-threaded build * don't panic in pyo3-ffi in abi3 check * document lack of limited API support * add is_free_threaded() method to InterpreterConfig * implement David's code review suggestions * remove unused imports
* update build config logic and library name generation * fix free-threaded windows clippy with --features=abi3 * use constant * add release note * apply my self-review comments * use ensure and error handling instead of panicking * skip abi3 fixup on free-threaded build * don't support PYO3_USE_ABI3_FORWARD_COMPATIBILITY on free-threaded build * don't panic in pyo3-ffi in abi3 check * document lack of limited API support * add is_free_threaded() method to InterpreterConfig * implement David's code review suggestions * remove unused imports
Fixes #4709.
Trying to get a library name or build pyo3-ffi against a free-threaded ABI for Python earlier than 3.13 now panics.
Also adds tests for the panics and for the free-threaded library names as followup for #4690 where I ran out of steam to add new tests.
While working on this I noticed that there was code to work around python/cpython#101614 which was fixed in CPython for 3.10 and newer, so I modified the workaround code path to only apply for Python 3.9 and older.