Skip to content

gh-106045: Fix venv path discovery for symlinked executables - #156483

Open
jjhelmus wants to merge 2 commits into
python:mainfrom
jjhelmus:getpath_use_base_executable
Open

gh-106045: Fix venv path discovery for symlinked executables#156483
jjhelmus wants to merge 2 commits into
python:mainfrom
jjhelmus:getpath_use_base_executable

Conversation

@jjhelmus

@jjhelmus jjhelmus commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Use the resolved base executable when locating the base installation instead of relying on the home directory in pyvenv.cfg.

Add tests covering relocated Python installations and misleading standard-library landmarks near executable symlinks.

Use the resolved base executable when locating the base installation
instead of relying on the home directory in pyvenv.cfg.

Add tests covering relocated Python installations and misleading
standard-library landmarks near executable symlinks.
@jjhelmus

Copy link
Copy Markdown
Contributor Author

This will likely also address: #127440
Which has another proposed fix: #127441

Comment thread Lib/test/test_getpath.py
ns.add_known_file("/venv/pyvenv.cfg", [
"home = /public/bin",
])
expected = dict(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why the dict() calls when a dict literal would have worked?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Good catch! A dictionary literal {...} is cleaner, more pythonic, and slightly faster than calling dict(). I'll update this line to use a literal instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The dict() call matches the pattern established in the existing tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(agree it makes sense to match all the other tests in the file instead)

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