Conversation
|
I added full build, if you want to fixup merge conflict we can see what initial results are. |
|
You should bump cpython’s |
cpython 3.15 C-API is I think still moving, wouldn't it better to keep the error message and use |
Agreed, we should not declare support for 3.15 properly until 3.15 is in beta. In #5093 (comment) I wrote some thoughts about maybe how we could support 3.15 for development. |
0397ab6 to
8b88136
Compare
|
@clin1234 looks like it's that time of year again. Do you mind if I push to this branch to fixup things? |
|
Sure, considering I'm job hunting ATM |
Co-authored-by: Bas Schoenmaeckers <7943856+bschoenmaeckers@users.noreply.github.com>
pyo3-ffi/src/object.rs
Outdated
| // use a macro to substitute _PyObject_MIN_ALIGNMENT definition above? | ||
|
|
||
| #[cfg_attr(not(Py_3_15), repr(C))] | ||
| #[cfg_attr(Py_3_15, repr(C, align(4)))] |
There was a problem hiding this comment.
I guess I need a macro here to avoid hard-coding the value of _PyObject_MIN_ALIGNMENT in the cfg_attr macro?
There was a problem hiding this comment.
It feels like it should be possible to write a static assertion here to check
e.g.
const _: () =
assert!(std::mem::align_of::<PyObject>() == _PyObject_MIN_ALIGNMENT);There was a problem hiding this comment.
Good call! Just one note that it's the minimum alignment, so the assertion is >=, not ==.
|
It looks like CI is green here 🎉 |
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks for getting this moving!
pyo3-build-config/Cargo.toml
Outdated
| abi3-py313 = ["abi3-py314"] | ||
| abi3-py314 = ["abi3"] | ||
| abi3-py314 = ["abi3-py315"] | ||
| abi3-py315 = ["abi3"] |
There was a problem hiding this comment.
Is it too early to add this feature? My instinct is that we don't want users to be able to build for the 3.15 stable ABI until that's actually nailed down.
There was a problem hiding this comment.
Ultimately my whole goal here is to enable testing with the new opaque PyObject stable ABI. I could wait to do that until after finishing up the wrappers for the PEP 793 module initialization API, though.
I suspect this year we might need to enable this earlier than beta 1, but I'll think about how to handle that without possibly causing downstream users to ship abi3 wheels too early.
There was a problem hiding this comment.
I think we can set UNSAFE_PYO3_SKIP_VERSION_CHECK in the CI and start testing without exposing it to users.
pyo3-ffi/src/object.rs
Outdated
| // use a macro to substitute _PyObject_MIN_ALIGNMENT definition above? | ||
|
|
||
| #[cfg_attr(not(Py_3_15), repr(C))] | ||
| #[cfg_attr(Py_3_15, repr(C, align(4)))] |
There was a problem hiding this comment.
It feels like it should be possible to write a static assertion here to check
e.g.
const _: () =
assert!(std::mem::align_of::<PyObject>() == _PyObject_MIN_ALIGNMENT);
pyo3-ffi/src/object.rs
Outdated
| pub type PyObjectObRefcnt = Py_ssize_t; | ||
|
|
||
| const _PyGC_PREV_SHIFT: usize = 2; | ||
| pub const _PyObject_MIN_ALIGNMENT: usize = 1 << _PyGC_PREV_SHIFT; |
There was a problem hiding this comment.
Is there a need for public export of the private prefixed symbol?
| pub const _PyObject_MIN_ALIGNMENT: usize = 1 << _PyGC_PREV_SHIFT; | |
| const _PyObject_MIN_ALIGNMENT: usize = 1 << _PyGC_PREV_SHIFT; |
There was a problem hiding this comment.
Since it is declared in the public API header: https://github.com/python/cpython/blob/aa8578dc54df2af9daa3353566359e602e5905cf/Include/object.h#L117, it's probably safer to make it a public export
There was a problem hiding this comment.
Underscored symbols are not part of the public API, even if they're in public headers (they're present there as ABI implementation details).
We've generally been removing such exports across the whole of pyo3-ffi, e.g. #5592
pyo3-ffi/build.rs
Outdated
| max: PythonVersion { | ||
| major: 3, | ||
| minor: 14, | ||
| minor: 15, |
There was a problem hiding this comment.
I think it'd be worth briefly discussing what the alternatives to bumping this number are. Could we make a rule that max + 1 will always build and run with warnings (maybe with an env var to turn off the warnings)?
My worry is that packages built against PyO3 0.28 might not be using 3.15 yet, but in the future when 3.15's ABI has both changed and finalised, users might install packages built with PyO3 0.28 on 3.15.0, get no warnings, and then get an instant crash.
There was a problem hiding this comment.
Could we make a rule that max + 1 will always build and run with warnings (maybe with an env var to turn off the warnings)?
Let me see how painful this is to set up. Seems sensible.
By "build with warnings" are you talking about the same kind of warning we already generate when someone tries to build a stable ABI extension on the free-threaded build?
| warn!( | ||
| "Using experimental support for the Python {}.{} ABI. \ | ||
| Build artifacts may not be compatible with the final release of CPython, \ | ||
| so do not distribute them.", |
There was a problem hiding this comment.
Anyone have language tweaks here? Maybe I should also be warning that the build artifacts might lead to crashes with this python if it's ABI incompatible with the FFI definitions.
There was a problem hiding this comment.
I think experimental is probably good enough hint that is might crash. Let's not block this PR any further for getting testing running, we can edit this copy later trivially.
| warn!( | ||
| "Using experimental support for the Python {}.{} ABI. \ | ||
| Build artifacts may not be compatible with the final release of CPython, \ | ||
| so do not distribute them.", |
There was a problem hiding this comment.
I think experimental is probably good enough hint that is might crash. Let's not block this PR any further for getting testing running, we can edit this copy later trivially.
No description provided.