gh-156495: Compare normalized prefixes in site._venv() - #156496
Open
Gronoxx wants to merge 2 commits into
Open
Conversation
subprocess only resolves a relative executable against its cwd argument on POSIX, so the test failed with FileNotFoundError on Windows. Change directory in the test process instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #156495.
site._venv()comparessys.prefixagainst asite_prefixbuilt withos.path.abspath(), using string equality.abspath()normalizes andgetpathdoes not, so a venv interpreter started through a path such as../venv/bin/pythonreports two spuriousRuntimeWarnings about prefixes that name the same directory.This compares normalized forms instead. No observable value changes:
sys.prefix,sys.exec_prefixandsys.executableare untouched, and the message still reports the original value. Only the condition for warning changes, and genuine mismatches still warn, including the one from #154160.The check itself came from GH-126987 (gh-126985), by Filipe Laíns, which replaced the earlier
sys.prefix = sys.exec_prefix = site_prefixassignment. Theabspath()on the other side has been there since #60723 (bpo-16519), reported by Christian Heimes and fixed by Vinay Sajip, added so that invoking the interpreter through a relative path would work. Both do what they were meant to do. Only the string comparison between them is too strict.Two tests are added, since there was no coverage of this warning in
test_site.pyortest_venv.py. The first fails without the change. The second pins down a genuine mismatch, so that a future change cannot silence the check altogether.Verification
Bisected on clean venvs, invoking
../v/bin/pythonfrom a sibling directory:sys.prefixsys.executable/tmp/v/tmp/sub/../v/bin/python/tmp/v/tmp/sub/../v/bin/python/tmp/v/tmp/sub/../v/bin/python/tmp/sub/../v/tmp/sub/../v/bin/python/tmp/sub/../v/tmp/sub/../v/bin/pythonsys.executableis identical throughout. Onlysys.prefixand the check are new.Every case, measured on this branch with and without the change:
../v/bin/pythonfrom a sibling directoryenv -i(noPATHin the environment)PATHentry (the #154160 symptom)//tmp/v/bin/python..in the middle of the pathpyvenv.cfgnext to the interpreterTest suite, before and after, on
test_venv test_site test_sys test_getpath test_embed test_sysconfig test_cmd_line test_cmd_line_script test_import test_frozen test_posixpath test_ntpath: 12 of 12 modules pass either way, 874 tests before and 876 after, the difference being the two added here.os.path.normpathis_path_normpath, a C builtin, at 549 ns per call. That is roughly 1 µs per interpreter start, and only inside a venv. I could not measure it above noise end to end.