-
Notifications
You must be signed in to change notification settings - Fork 760
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
pypy: support 7.3.8 #2217
pypy: support 7.3.8 #2217
Conversation
cc @mattip |
src/impl_/pymodule.rs
Outdated
@@ -71,6 +71,21 @@ impl ModuleDef { | |||
panic_result_into_callback_output( | |||
py, | |||
std::panic::catch_unwind(move || -> PyResult<_> { | |||
#[cfg(all(PyPy, not(Py_3_8)))] | |||
{ | |||
const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 9]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I don't understand?
const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 9]; | |
const PYPY_GOOD_VERSION: [u8; 3] = [7, 3, 8]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha good catch, I set it higher when manually testing 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Shouldn't v7.3.8 be clean, and the warning only emitted for v7.3.7 and under? |
88ad9f4
to
5f61d8e
Compare
Yes, I was running 7.3.8 locally so fiddled with versions to verify the warning. Force-pushed a corrected implementation which should now only warn on 7.3.7 and older ;) |
According to https://foss.heptapod.net/pypy/pypy/-/issues/3688#note_181074 the official PyPy line will be that PyPy 3.7 versions older than 7.3.8 should be treated as bugged.
Hence here I'm updating definitions to support 7.3.8 and add a warning if a user tries to import a PyO3 module with an older PyPy version.