GH-126985: move pyvenv.cfg detection from site to getpath

[FFY00]

• Issue: Make pyvenv.cfg-relocation part of the interpreter initialization instead of the site module #126985

[zooba]

The change looks reasonable to me, but I'm still terrified of changing anything at all in getpath 😆 Better to do it early in the release cycle though.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit fa3adba 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[FFY00]

I think the implementation is ready if the tests are passing on other platforms and buildbots. The documentation still needs to be updated though.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit 3c1fb29 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit 3c24f44 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[FFY00]

Alright, the results from the buildbots seem positive. There are 3 failures, but I think they're unrelated.

1. test_import leaks

Example: https://buildbot.python.org/#/builders/474/builds/1754/steps/6/logs/stdio

----------------------------------------------------------------------
Ran 115 tests in 4.995s
OK (skipped=3)
X
test_import leaked [52, 3, 51] references, sum=106
test_import leaked [33, 2, 32] memory blocks, sum=67
1 test failed again:
    test_import
== Tests result: FAILURE then FAILURE ==
10 slowest tests:
- test_shelve: 8 min 5 sec
- test.test_multiprocessing_spawn.test_processes: 7 min 9 sec
- test.test_multiprocessing_forkserver.test_processes: 5 min 44 sec
- test_socket: 5 min 24 sec
- test.test_concurrent_futures.test_wait: 4 min 56 sec
- test_tarfile: 4 min 41 sec
- test_io: 4 min 26 sec
- test.test_multiprocessing_spawn.test_misc: 4 min 19 sec
- test_regrtest: 3 min 53 sec
- test_zipfile: 3 min 42 sec
15 tests skipped:
    test.test_asyncio.test_windows_events
    test.test_asyncio.test_windows_utils test_android test_devpoll
    test_free_threading test_ioctl test_kqueue test_launcher
    test_msvcrt test_startfile test_winapi test_winconsoleio
    test_winreg test_winsound test_wmi
4 tests skipped (resource denied):
    test_peg_generator test_tkinter test_ttk test_zipfile64
1 re-run test:
    test_import
1 test failed:
    test_import
460 tests OK.
Total duration: 21 min 38 sec
Total tests: run=45,618 skipped=1,894
Total test files: run=477/480 failed=1 skipped=15 resource_denied=4 rerun=1
Result: FAILURE then FAILURE

2. test_gc failures on Android and iOS

Android: https://buildbot.python.org/#/builders/1593/builds/71/steps/8/logs/stdio
iOS: https://buildbot.python.org/#/builders/1382/builds/180/steps/10/logs/stdio

3. PEG Generator failures on Windows 10

Example: https://buildbot.python.org/#/builders/577/builds/1691

[FFY00]

This is now ready for review. @zooba would be able to have a look?

[FFY00]

I am gonna sync with main and re-trigger the buildbots to see if any of the failures I saw before went away.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @FFY00 for commit caf5984 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

[FFY00]

The 1) and 2) failures seem to be gone, leaving only the PEG Generator failures on Windows 10, which definitely seem unrelated.

[zooba]

I'm not 100% confident this will be smooth, but I don't think it can be made any better.

Let's keep an eye on whether the -S behaviour needs to be brought back. I hope not, but it might be too much trouble.

[zooba]

If we need to, we should be able to make them affected by the option again. This seems like the most likely change to impact people, so it's worth being aware of how we might cleanly roll it back without undoing all the work.

[FFY00]

Right, this should be relatively easy, as PyConfig already has a site_import field, which we can use to check to change the behavior.

I think the specifics of a possible rollback would depend on the reported issues. I would like to still keep the new behavior long term, if there are any issues, I think we should consider rolling back with a deprecation period, but depending on how critical and in what way the new behavior is problematic, we may just want to roll back permanently.

[zooba]

These warnings are also worth watching out for, though hopefully they only ever indicate actual problems.

If the values don't match, should we update them anyway like we used to?

[FFY00]

Good point! We could run this through a deprecation period, setting the value for now and stopping in two versions, though, I think triggering these warnings likely highlights a bug in user code and giving how niche use-cases like these are, I am not sure if that's needed, and it's extra overhead on our part. I'm inclined to keep the code as-is, and if we see any issues during the pre-release phase, change to the deprecation approach before a final release.

@hugovk do you have any preference as the RM?

[hugovk]

No preference as RM.

You could have the warnings for one release, then based on feedback (if any!) decide if you want to deprecate/change behaviour.

[FFY00]

Let's go with the warnings and re-evaluate based on feedback, then.

[zooba]

I like all these changes! Makes the intent much clearer. The overall change it is worth it for these, IMHO.

[FFY00]

Yes, and from playing with the getpath, I believe a significant portion of the code could be simplified and better written to show intent by strictly adhering to the defined input/output parameters, as listed in https://docs.python.org/3/c-api/init_config.html#python-path-configuration. I found a lot of the complexity comes from taking into account non-input parameters, which as far as I can tell, is done to make writing the tests easier, by allowing them to overwrite certain parameters, mostly to get around file system checks. This feels wrong, and makes the getpath extremely difficult to parse and understand what is supposed to be happening. It could be avoided by improving the test suite, maybe by making the helper functions operate on a mocked file system.

[zooba]

I found a lot of the complexity comes from taking into account non-input parameters, which as far as I can tell, is done to make writing the tests easier, by allowing them to overwrite certain parameters, mostly to get around file system checks.

It's certainly intended to be a pure module, I'd hate to see "real" file system checks showing up in too many places. Those should be done first and passed in as data, or at least handled through the functions made available for executing getpath (which are easily mocked out for tests).

But other than that, it basically follows the flow of the getpath.c and getpathp.c files that I translated to get here. So yeah, it's a mess, but only because they were a worse mess! With controlled input/output parameters and very controlled external state, it should be possible to safely refactor the whole thing now.