Repository navigation
Prefer resolved executable symlinks in POSIX virtual environments #9058
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,10 @@ | ||
| import os | ||
| import shutil | ||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
| import warnings | ||
| from pathlib import Path | ||
|
|
||
| from testutils import assert_raises | ||
|
|
||
|
|
@@ -192,3 +195,132 @@ def test_getframemodulename(): | |
| del os.environ["PYTHONBREAKPOINT"] | ||
| else: | ||
| os.environ["PYTHONBREAKPOINT"] = saved_breakpoint_env | ||
|
|
||
|
|
||
| # A venv's symlink target takes precedence over home/<invoked name>. | ||
| if os.name == "posix" and sys.platform not in ("wasi", "emscripten"): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This block adds synthetic test data, control flow, and assertions to an existing test file, but the repository policy permits test-file changes only for adding or removing expected-failure decorators; the regression coverage therefore needs to be removed from AGENTS.md reference: AGENTS.md:L273-L279 Useful? React with 👍 / 👎. |
||
| with tempfile.TemporaryDirectory() as directory: | ||
| root = Path(directory).resolve() | ||
| interpreter = Path(sys.executable).resolve() | ||
| # A framework build can also run its embedded app directly. Match the | ||
| # installed launcher's file identity before applying launcher semantics. | ||
| framework_base = ( | ||
| Path(sys.base_prefix) | ||
| / "bin" | ||
| / f"python{sys.version_info.major}.{sys.version_info.minor}" | ||
| ) | ||
| framework_launcher = ( | ||
| sys.platform == "darwin" | ||
| and bool(getattr(sys, "_framework", "")) | ||
| and framework_base.is_file() | ||
| and interpreter.samefile(framework_base) | ||
| ) | ||
| home = root / "home" | ||
| home.mkdir() | ||
| venv_env = dict(os.environ) | ||
| for name in ("PYTHONHOME", "PYTHONEXECUTABLE", "__PYVENV_LAUNCHER__"): | ||
| venv_env.pop(name, None) | ||
| # Keep startup imports available even with the synthetic home directory. | ||
| venv_env["PYTHONPATH"] = os.pathsep.join(sys.path) | ||
| path_code = ( | ||
| "import sys; print(sys.executable); " | ||
| "print(sys._base_executable); print(sys.prefix)" | ||
| ) | ||
|
|
||
| def executable_paths(executable, *, cwd=None, env=venv_env): | ||
| result = subprocess.run( | ||
| [str(executable), "-B", "-S", "-c", path_code], | ||
| cwd=cwd, | ||
| env=env, | ||
| capture_output=True, | ||
| text=True, | ||
| timeout=10, | ||
| ) | ||
| assert result.returncode == 0, result.stderr | ||
| return result.stdout.splitlines() | ||
|
|
||
| for kind in ("absolute", "relative", "chain"): | ||
| bin_dir = root / kind / "bin" | ||
| bin_dir.mkdir(parents=True) | ||
| # Exercise both parent and adjacent pyvenv.cfg discovery. | ||
| prefix = bin_dir if kind == "chain" else bin_dir.parent | ||
| (prefix / "pyvenv.cfg").write_text(f"home = {home}\n") | ||
| executable = bin_dir / kind | ||
| # This regular file must never be preferred to a symlink's target. | ||
| (home / kind).touch() | ||
| if kind == "absolute": | ||
| executable.symlink_to(interpreter) | ||
| elif kind == "relative": | ||
| executable.symlink_to(os.path.relpath(interpreter, bin_dir)) | ||
| else: | ||
| (bin_dir / "target").symlink_to(interpreter) | ||
| executable.symlink_to("target") | ||
| invocation = f"./bin/{kind}" if kind == "relative" else executable | ||
| assert executable_paths(invocation, cwd=bin_dir.parent) == [ | ||
| str(executable), | ||
| str(framework_base if framework_launcher else interpreter), | ||
| str(prefix), | ||
| ] | ||
|
|
||
| # Framework launchers preset their base; ordinary binaries keep argv[0]. | ||
| plain = root / "plain" / "python" | ||
| plain.parent.mkdir() | ||
| plain.symlink_to(interpreter) | ||
| assert executable_paths(plain)[:2] == [ | ||
| str(plain), | ||
| str(framework_base if framework_launcher else plain), | ||
| ] | ||
|
|
||
| # Without a launcher preset, copies fall back to home/<invoked name>. | ||
| copied = root / "copied" / "python" | ||
| copied.parent.mkdir() | ||
| shutil.copy2(interpreter, copied) | ||
| (home / copied.name).touch() | ||
| (copied.parent / "pyvenv.cfg").write_text(f"home = {home}\n") | ||
| assert executable_paths(copied) == [ | ||
| str(copied), | ||
| str(framework_base if framework_launcher else home / copied.name), | ||
| str(copied.parent), | ||
| ] | ||
|
|
||
| # A directory alias does not turn a copied executable into a symlink. | ||
| directory_alias = root / "directory-alias" | ||
| directory_alias.symlink_to(copied.parent, target_is_directory=True) | ||
| aliased_copy = directory_alias / copied.name | ||
| # The framework launcher canonicalizes its directory before dispatch. | ||
| assert executable_paths(aliased_copy) == [ | ||
| str(copied if framework_launcher else aliased_copy), | ||
| str(framework_base if framework_launcher else home / copied.name), | ||
| str(copied.parent if framework_launcher else directory_alias), | ||
| ] | ||
|
|
||
| # Without a launcher preset, final-link resolution preserves target aliases. | ||
| target_link = root / "linked" / "python" | ||
| target_link.parent.mkdir() | ||
| (target_link.parent / "pyvenv.cfg").write_text(f"home = {home}\n") | ||
| target_link.symlink_to("../directory-alias/python") | ||
| assert executable_paths(target_link) == [ | ||
| str(target_link), | ||
| str(framework_base if framework_launcher else aliased_copy), | ||
| str(target_link.parent), | ||
| ] | ||
|
|
||
| # macOS framework builds use a framework-derived base for overrides. | ||
| if sys.platform == "linux": | ||
| # Relative targets also preserve a double-leading-slash directory. | ||
| double_target = root / "double-target" | ||
| double_target.symlink_to("directory-alias/python") | ||
| target_link.unlink() | ||
| target_link.symlink_to("/" + str(double_target)) | ||
| assert executable_paths(target_link) == [ | ||
| str(target_link), | ||
| "/" + str(aliased_copy), | ||
| str(target_link.parent), | ||
| ] | ||
|
|
||
| override_env = dict(venv_env, __PYVENV_LAUNCHER__=str(executable)) | ||
| assert executable_paths(plain, env=override_env) == [ | ||
| str(executable), | ||
| str(plain), | ||
| str(prefix), | ||
| ] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a venv executable is directly under
/(for example,/pythonwith/pyvenv.cfg) and the executable symlink has a relative target such asusr/bin/python,rpositionreturns0and this slice drops the only root separator. The kernel resolves that target as/usr/bin/python, butcalculate_base_executablenow publishesusr/bin/pythonassys._base_executable, so its meaning depends on the startup working directory and consumers such asvenvcan select the wrong interpreter; retain the root separator when the final component starts immediately after/(and likewise for a//root).Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This edge is deliberate CPython 3.14 parity. Both 3.14.7 and 3.14.8 truncate the final separator before joining a relative target: getpath.c. The join helper adds no separator for an empty dirname: fileutils.c. Consequently, /python with target usr/bin/python becomes usr/bin/python, while //python becomes /usr/bin/python. The current helper reproduces that behavior; retaining these roots would diverge from the reference contract. This particular edge was checked against the source, without creating or testing root-level filesystem paths. I am keeping the implementation unchanged.