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

Setting configs incompatible with Py_GIL_DISABLED should error harder #4709

Open
ngoldbaum opened this issue Nov 15, 2024 · 2 comments
Open

Comments

@ngoldbaum
Copy link
Contributor

ngoldbaum commented Nov 15, 2024

I noticed working on cryptography, which tries to build against the Python 3.7 limited API, that it has this pair of configs set:

cargo:rustc-cfg=Py_3_7
cargo:rustc-cfg=Py_GIL_DISABLED

This leads to confusing compiler errors due to the conditional compilation in pyo3-ffi not being written to handle this case since it's impossible:

DEBUG    Compiling cryptography-x509 v0.1.0 (/Users/goldbaum/Documents/cryptography/src/rust/cryptography-x509)
DEBUG error[E0432]: unresolved import `crate::PyMutex`
DEBUG  --> /Users/goldbaum/Documents/pyo3/pyo3-ffi/src/object.rs:3:5
DEBUG   |
DEBUG 3 | use crate::PyMutex;
DEBUG   |     ^^^^^^^^^^^^^^ no `PyMutex` in the root
DEBUG
DEBUG error[E0425]: cannot find value `_Py_IMMORTAL_REFCNT` in this scope
DEBUG    --> /Users/goldbaum/Documents/pyo3/pyo3-ffi/src/object.rs:162:20
DEBUG     |
DEBUG 29  | const _Py_IMMORTAL_REFCNT_LOCAL: u32 = u32::MAX;
DEBUG     | ------------------------------------------------ similarly named constant `_Py_IMMORTAL_REFCNT_LOCAL` defined here
DEBUG ...
DEBUG 162 |             return _Py_IMMORTAL_REFCNT;
DEBUG     |                    ^^^^^^^^^^^^^^^^^^^ help: a constant with a similar name exists: `_Py_IMMORTAL_REFCNT_LOCAL`
DEBUG
DEBUG error[E0560]: struct `object::PyObject` has no field named `ob_refcnt`
DEBUG   --> /Users/goldbaum/Documents/pyo3/pyo3-ffi/src/object.rs:55:5
DEBUG    |
DEBUG 55 |     ob_refcnt: 1,
DEBUG    |     ^^^^^^^^^ `object::PyObject` does not have this field
DEBUG    |
DEBUG    = note: all struct fields are already assigned

We should panic earlier because this configuration will never work.

@davidhewitt
Copy link
Member

Hmm, I wonder, does this imply that it's actually really hard for cryptography to build a free-threaded wheel because they enabled the abi3 features?

@ngoldbaum
Copy link
Contributor Author

No, I think it just means they'll need to upload a cp313t wheel alongside the abi3 wheel. This is true for any project that normally uploads version-independent wheels but wants to support free-threaded 3.13.

The limited API is a subset of the full API and I don't think normally compiling against the 3.7 limited ABI prevents compiling for a newer non-limited ABI.

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

2 participants