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

Add PyPy3.11 #4760

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Add PyPy3.11 #4760

wants to merge 10 commits into from

Conversation

mattip
Copy link
Contributor

@mattip mattip commented Dec 3, 2024

Support PyPy's python3.11. It is not officially released yet, but I wanted to be sure PyO3 works.
I am testing this branch elsewhere. Until there is an official release, I set the PR to use the nightly builds.

You may wish to hold off on merging this until the official release, that is fine with me.

Also I dropped PyPy3.9 since we no longer support it. Nothing has changed so it should JustWork(tm)

I had help from the discussions, thanks!

@mattip
Copy link
Contributor Author

mattip commented Dec 4, 2024

Hmm. There is a unused declaration error. Any suggestions how to fix that?

 error: unused import: `c_char`
   --> pyo3-ffi/src/cpython/abstract_.rs:2:20
    |
  2 | use std::os::raw::{c_char, c_int};
    |                    ^^^^^^
    |
    = note: `-D unused-imports` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(unused_imports)]`
  
  error: unused import: `std::os::raw::c_char`
   --> pyo3-ffi/src/cpython/genobject.rs:6:5
    |
  6 | use std::os::raw::c_char;
    |     ^^^^^^^^^^^^^^^^^^^^
  
  error: unused import: `libc::size_t`
   --> pyo3-ffi/src/cpython/objimpl.rs:1:5
    |
  1 | use libc::size_t;
    |     ^^^^^^^^^^^^
  
  error: could not compile `pyo3-ffi` (lib) due to 3 previous errors

@Icxolu
Copy link
Contributor

Icxolu commented Dec 4, 2024

These occur because with this there are new cfg combinations possible that weren't possible before (and therefore not tested for). To get rid of the warnings you need to add cfgs to them, so we only import them when they are needed.

In the order from the error above, I think these should be 🤞:

  • #[cfg(any(all(Py_3_8, not(any(PyPy, GraalPy))), not(Py_3_11)))]
  • #[cfg(all(not(PyPy), not(GraalPy), Py_3_11))]
  • #[cfg(any(all(not(PyPy), not(GraalPy)), not(Py_3_11)))]

@mattip
Copy link
Contributor Author

mattip commented Dec 5, 2024

Thanks, let's try it

@mattip
Copy link
Contributor Author

mattip commented Dec 5, 2024

Yup, that seems to have done the trick. Is there anything else I should try? I will mark this as "draft" since it can wait for the release to be merged.

@mattip mattip marked this pull request as draft December 5, 2024 15:03
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I added a label so that the full CI will trigger if you push again to this branch. This should execute more PyPy3.11 jobs and give better feedback.

Aside, if pypy/pypy#3836 ever gets resolved it will vastly increase the amount of testing of PyPy we can do here, because we have an absolute heap of tests which use Py_Initialize to spawn interpreters and run basic checks. I think it's out of scope for me to find time to work on that for the foreseeable future sadly.

@@ -242,8 +242,8 @@ jobs:
"3.11",
"3.12",
"3.13",
"pypy3.9",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Historically we've been slow to drop support of PyPy versions (typically we match the version of CPython EOL just to keep things simpler). Do you think it's better if we drop PyPy versions eagerly on EOL? Would certainly mean fewer CI runs 😂

Also, if we want to drop PyPy 3.9 support, I would rather we removed the CI jobs, started preventing compiles (because it's possible we might break it by accident once CI is removed), and added a newsfragment to announce, all in one step. Probably best to split into a separate PR, if we want to do this now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it's better if we drop PyPy versions eagerly on EOL?

It is up to you. I don't think we would do another 3.9 release even if a new issue were discovered. I would try to help you solve them though not with a high priority. Have you had many PyPy-specific issues reported? I will revert this change and let you do it at your tempo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename this to 4760.packaging.md - we put the packaging section at the top so easier for users to notice this.

@mattip
Copy link
Contributor Author

mattip commented Dec 8, 2024

Unfortunately we are also unlikely to implement Py_Initialize. To do it properly would require first refactoring the startup code in pypy/interpreter/app_main.py and pypy/module/sys to use a CPython-like startup sequence with a configuration object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants