Fixes for 3.13

[A5rocks]

Fixes #3004, also refs #2885

@njsmith could you check my naive locals() -> sys._getframe().f_locals replacement? Unfortunately 3.13 makes locals() mutations not mutate actual local variables, as part of PEP 667. I'm especially not sure about the replacement in _run.py where locals() used to clear something (not sure what).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (3350c11) to head (f62e81e).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3005    +/-   ##
========================================
  Coverage   99.63%   99.63%            
========================================
  Files         120      120            
  Lines       17783    17963   +180     
  Branches     3197     3243    +46     
========================================
+ Hits        17718    17898   +180     
  Misses         46       46            
  Partials       19       19            

Files	Coverage Δ
src/trio/_core/_ki.py	100.00% <100.00%> (ø)
src/trio/_core/_run.py	99.38% <100.00%> (+0.04%)	⬆️
src/trio/_path.py	100.00% <100.00%> (ø)
src/trio/_tests/test_exports.py	99.61% <100.00%> (+<0.01%)	⬆️
src/trio/_tests/test_path.py	100.00% <100.00%> (ø)
src/trio/_tools/gen_exports.py	99.15% <100.00%> (ø)
src/trio/socket.py	100.00% <ø> (ø)

... and 6 files with indirect coverage changes

[richardsheridan]

Back when we were making reference cycle tests, calling locals() when an exception was around would make a reference cycle for every exception (i.e. every Cancelled) unless you did both del and removed it from the f_locals dict. if the test is passing on all versions without it, though, no reason to keep it around

[hroncok]

The test_cancel_scope_exit_doesnt_create_cyclic_garbage test fails on Fedora with Python 3.12 but passes with 3.13 :/

(However, we are stuck at 0.23.1, we are backporting this on top.)

[hroncok]

Perhaps something like:

if sys.version_info < (3, 13):
    locals()

[befeleme]

Replacing locals() with sys._getframe().f_locals also does the trick for both Python 3.12 and Python 3.13. The code is too magical for me to tell whether it's factually correct.

[A5rocks]

Just to confirm: latest state of the PR fails tests on fedora with Python 3.12? (That's not a platform in CI so very possible)

[A5rocks]

I'm a bit confused cause when I used Python 3.12.3 on Fedora (Docker, I built CPython 3.12.3 from source) and pytest --pyargs trio --skip-optional-imports on this PR, the tests pass. Am I missing something?

[hroncok]

As said, we are stuck at 0.23.1, we are backporting this PR on top of it, so perhaps that makes a difference.

[jakkdl]

As said, we are stuck at 0.23.1, we are backporting this PR on top of it, so perhaps that makes a difference.

what's the reason you're stuck at 0.23.1? Looking at the changelog of 0.23.2 doesn't look like it should be introducing anything controversial.

[hroncok]

python-trio/trio-websocket#187 and possibly others

[jakkdl]

that should only be affected from 0.25.0 though. If you're at the point where you're cherrypicking commits you could instead revert cfc0755

[richardsheridan]

Seems like with these changes CancelScope.__exit__ doesn't need to inline the KI protection decorator anymore, so I pushed a commit on that.

[A5rocks]

I think this is ready for review/merge. While this applied as a patch doesn't seem to work, I can't seem to reproduce that and I'm sure it's just some other PR that changed things just enough for this subtle locals stuff to work (which can be located probably just using a git bisect?).

[jakkdl]

I think you can get 3.13 to pass in CI if you explicitly install cffi==1.17.0rc1 as a dependency: https://github.com/python-cffi/cffi/releases/tag/v1.17.0rc1
Since it's a release candidate pip doesn't automatically target it.

I wrote a PR to get trio-websocket to support trio>0.25 python-trio/trio-websocket#188, so once merged that may fully resolve the issues encountered by the distro maintainers.

[A5rocks]

Alright, this is now running the whole test suite (rather than the part that doesn't require the test requirements).

[CoolCat467]

Makes sense, looks good as long as others are in agreement that the locals magic thing discussion is taken care of.

[jakkdl]

I also don't understand locals() magic, but the tests are passing and the other changes looks good.

[richardsheridan]

The locals()/garbage interaction isn't triggering some of the tests the way it was when I inserted it. I don't think it's worth bisecting python/trio to understand it or preemptively fixing, since it will be at worst a performance regression. If someone eventually stumbles on a reproducible regression to add to the tests, we know where to look for the fix.