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

Fix import under PyPy 3.8/3.9 on Windows #7850

Merged
merged 6 commits into from
Nov 19, 2023

Conversation

JelleZijlstra
Copy link
Contributor

@JelleZijlstra JelleZijlstra commented Nov 18, 2023

Fixes #7848.

@JelleZijlstra
Copy link
Contributor Author

OK https://github.com/aio-libs/aiohttp/actions/runs/6914844384/job/18813153994?pr=7850 reproduces the issue. Now pushing a commit to try to fix it.

Copy link

codecov bot commented Nov 18, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (6f76673) 97.40% compared to head (5fbfd6c) 97.40%.

Files Patch % Lines
aiohttp/cookiejar.py 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7850   +/-   ##
=======================================
  Coverage   97.40%   97.40%           
=======================================
  Files         106      106           
  Lines       32164    32164           
  Branches     3739     3739           
=======================================
  Hits        31329    31329           
  Misses        631      631           
  Partials      204      204           
Flag Coverage Δ
CI-GHA 97.31% <0.00%> (ø)
OS-Linux 96.99% <0.00%> (ø)
OS-Windows 95.49% <0.00%> (ø)
OS-macOS 96.66% <0.00%> (-0.01%) ⬇️
Py-3.10.11 95.41% <0.00%> (ø)
Py-3.10.13 96.85% <0.00%> (ø)
Py-3.11.6 96.50% <0.00%> (ø)
Py-3.12.0 96.57% <0.00%> (ø)
Py-3.8.10 95.38% <0.00%> (ø)
Py-3.8.18 96.77% <0.00%> (ø)
Py-3.9.13 95.38% <0.00%> (ø)
Py-3.9.18 96.81% <0.00%> (ø)
Py-pypy7.3.13 96.27% <0.00%> (ø)
VM-macos 96.66% <0.00%> (-0.01%) ⬇️
VM-ubuntu 96.99% <0.00%> (ø)
VM-windows 95.49% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dreamsorcerer
Copy link
Member

Well, we can probably merge the fix in here. But, maybe need to consider another PR to add Pypy-Windows to the CI, looks like there are a few other failing tests which will need to be fixed or skipped.

aiohttp/cookiejar.py Outdated Show resolved Hide resolved
@JelleZijlstra
Copy link
Contributor Author

@AlexWaygood pointed out that 3.8 is EOL so I switched CI back to pypy3.9. If we're lucky, that also makes some of the other errors go away.

I saw two categories of errors in the CI run with Windows PyPy 3.8:

  1. Overlapped
  File "C:\hostedtoolcache\windows\PyPy\3.8.16\x86\Lib\asyncio\windows_events.py", line 612, in sendfile
    ov.TransmitFile(sock.fileno(),
AttributeError: 'Overlapped' object has no attribute 'TransmitFile'

That seems like a bug in pypy. Possibly can be worked around by not using sendfile() on Windows PyPy.

  1. NotImplementedError: symlink() is not implemented for PyPy on Windows
    Those tests presumably just need to be skipped.

There are also some other errors with less clear causes (e.g. aiohttp.client_exceptions.ClientPayloadError: Response payload is not completed). Could be secondary effects of the above two, but could also be separate issues.

On 3.9 the symlink errors went away but the Overlapped one are still around.

I'll switch this PR back to running CI on Linux, and open an issue about Windows PyPy support.

@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Nov 18, 2023
@JelleZijlstra JelleZijlstra marked this pull request as ready for review November 18, 2023 20:27
@JelleZijlstra
Copy link
Contributor Author

OK this is ready. There are some other CI failures on 3.11 or 3.12 but they don't look related.

@AlexWaygood
Copy link
Contributor

The PR title should probably change ;)

@JelleZijlstra JelleZijlstra changed the title Run CI on PyPy 3.8 Fix import under PyPy 3.8/3.9 on Windows Nov 18, 2023
@Dreamsorcerer Dreamsorcerer merged commit 22170b2 into aio-libs:master Nov 19, 2023
29 of 34 checks passed
Copy link
Contributor

patchback bot commented Nov 19, 2023

Backport to 3.9: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.9/22170b21064be8fdf75b947d9c2930df7b2518e1/pr-7850

Backported as #7854

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 19, 2023
Copy link
Contributor

patchback bot commented Nov 19, 2023

Backport to 3.10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.10/22170b21064be8fdf75b947d9c2930df7b2518e1/pr-7850

Backported as #7855

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Nov 19, 2023
Dreamsorcerer pushed a commit that referenced this pull request Nov 19, 2023
…indows (#7855)

**This is a backport of PR #7850 as merged into master
(22170b2).**

Fixes #7848.

Co-authored-by: Jelle Zijlstra <[email protected]>
Dreamsorcerer pushed a commit that referenced this pull request Nov 19, 2023
…ndows (#7854)

**This is a backport of PR #7850 as merged into master
(22170b2).**

Fixes #7848.

Co-authored-by: Jelle Zijlstra <[email protected]>
xiangxli pushed a commit to xiangxli/aiohttp that referenced this pull request Dec 4, 2023
@webknjaz
Copy link
Member

webknjaz commented Apr 1, 2024

There are also some other errors with less clear causes (e.g. aiohttp.client_exceptions.ClientPayloadError: Response payload is not completed). Could be secondary effects of the above two, but could also be separate issues.

FTR there are a number of issues with this exception reported but unclear causes happening under normal CPython (like #4581). So I ended up releasing v3.9.4rc0 that attaches those causes to the exceptions when they are raised (via #8049 and #8089).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[3.9] Import fails on Windows PyPy
4 participants