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 support for Python 3.13 #49970

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Add support for Python 3.13 #49970

wants to merge 15 commits into from

Conversation

jgraham
Copy link
Contributor

@jgraham jgraham commented Jan 8, 2025

Notably this involves vendoring a copy of the removed cgi module from stdlib.

This code has been dropped from Python 3.13, but we depend on
FieldStorage as part of the request API. The easiest option here is to
directly import the code that we are already using (the last version
on the 3.12 branch, but with the deprecation warning removed), along
with a copy of the PSF License which covers the cgi module code.
…ck API

This is required since the logging module started using the lock as a context manager,
and calling the acquire and release methods.
Remove code only needed for Python < 3.4 and ensure that we always use the spawn
form of multiprocessing to avoid problems with fork and locks.
Copy link
Member

@gsnedders gsnedders left a comment

Choose a reason for hiding this comment

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

I wonder if we should also vendor the cgi tests, for the sake of any changes we make to it?

tools/wptrunner/requirements.txt Outdated Show resolved Hide resolved
tools/wptrunner/wptrunner/wptlogging.py Outdated Show resolved Hide resolved
Just adding this to the excludes doesn't work, because we still check the imported
code.
Lightly modified from the version in Python 3.12 as follows:
* Inlined check__all__ function
* Converted the use of ignore_warnings helper to use warning module directly
This is no longer support in Python 3.13, and no longer required for Python>=3.8
@jgraham
Copy link
Contributor Author

jgraham commented Jan 9, 2025

I wonder if we should also vendor the cgi tests, for the sake of any changes we make to it?

I mildly regret doing this given that some changes were needed to make the tests themselves compatible with multiple Python versions, but done :)

The Azure test failures are:

  • Affected tests without changes - this tries to use use cgi module in the current master commit but with Python 3.13. It obviously fails, but I don't think we should be worried.
  • infrastructure/ tests - this looks unrelated to the current PR.

So I think this could land if no further changes are requested.

@jgraham jgraham requested a review from gsnedders January 9, 2025 16:20
@@ -90,6 +90,21 @@ def emit(self, record):
self.queue.put(data)



class NullLock:
Copy link
Member

Choose a reason for hiding this comment

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

Given this is brand new code, can we please actually add types for all of this? This should, I think, match: https://github.com/python/typeshed/blob/614e9499f145e9dccade16d906247276f1d5e6aa/stdlib/_thread.pyi#L16-L20

Can we also add a comment saying that this implements the interface of threading.RLock, and probably rename this to NullRLock, to clarify this doesn't match threading.Lock?

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.

4 participants