| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Assisted-by: OpenAI Codex:model identifier unavailable
Assisted-by: OpenAI Codex:model identifier unavailable
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to f29bb No actionable issue is identified in this review; the change is mergeable after normal CI checks. Pre-merge checks |
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands. |
Sorry, something went wrong.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Sorry, something went wrong.
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: 68ea9412c4
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sorry, something went wrong.
| let end = bytes.iter().rposition(|&b| b == b'/'); | ||
| let dirname = &bytes[..end.unwrap_or(bytes.len())]; | ||
| let mut joined = PathBuf::from(std::ffi::OsStr::from_bytes(dirname)); |
There was a problem hiding this comment.
Preserve the root when joining relative link targets
When a venv executable is directly under / (for example, /python with /pyvenv.cfg) and the executable symlink has a relative target such as usr/bin/python, rposition returns 0 and this slice drops the only root separator. The kernel resolves that target as /usr/bin/python, but calculate_base_executable now publishes usr/bin/python as sys._base_executable, so its meaning depends on the startup working directory and consumers such as venv can select the wrong interpreter; retain the root separator when the final component starts immediately after / (and likewise for a // root).
Useful? React with 👍 / 👎.
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Identify the installed framework launcher by build configuration and file identity, then retain exact path assertions for launcher presets and directory canonicalization. Keep all cases and the production resolver unchanged. Assisted-by: OpenAI Codex:model identifier unavailable
Head branch was pushed to by a user without write access
There was a problem hiding this comment.
Here are some automated review suggestions for this pull request.
Reviewed commit: f29bb731e2
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Sorry, something went wrong.
|
|
||
|
|
||
| # 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.
Remove the prohibited regression-test body
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 stdlib_sys.py rather than added here.
AGENTS.md reference: AGENTS.md:L273-L279
Useful? React with 👍 / 👎.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Extract the Python 3.14-compatible venv executable-symlink behavior from #8954 (43065e79ddc46eafeca58cb39398ac007b5d25f7), without copying its broader custom metadata traversal.
When pyvenv.cfg supplies a home directory, follow final-component executable symlinks before falling back to home/<invoked name>. Preserve directory aliases, copied-executable fallback, non-venv invocation paths, Windows behavior, and existing launcher-override precedence.
The resolver follows CPython 3.14.7's getpath helper, including its bounded traversal, rather than using filesystem canonicalization, which would incorrectly resolve directory aliases. The selection contract is in getpath.py.
Validation
All new assertions are Python-side. Children only print sys attributes, run with -B -S and timeouts, and execute the trusted interpreter or its copy inside owned temporary directories. No package installation, configuration-provided executable, dangling/cyclic launch, or root-path probe is part of the tests.
Boundaries
The local character-count rule follows RustPython's existing UTF-8/surrogateescape filesystem contract; arbitrary non-UTF-8 CPython locale parity is not claimed. Root-level edge behavior was source-reviewed only. WASI/Emscripten are excluded from new runtime coverage; non-Linux native execution remains unverified.
No cfg parser, custom base-executable/executable metadata, PYTHONHOME handling, shared path converter, or canonical test assertion/marker is changed. Canonical test_getpath.py is absent from this checkout; no canonical-suite pass is claimed. Hosted CI remains required.
Summary by CodeRabbit
macOS framework reference correction
Follow-up f29bb73 corrects the new test fixture's expectations for the installed macOS framework launcher. The launcher recreates __PYVENV_LAUNCHER__, presets the framework base, and canonicalizes the invocation directory (launcher source, getpath precedence). The fixture now combines the configured framework feature with exact installed-launcher file identity; direct embedded-app binaries keep the ordinary path expectations. All cases and exact assertions remain; no implementation-name guard or production change is added.
The corrected full snippet passes native RustPython and both CPython 3.14.7/3.14.8 references on Linux; the reference/native pytest harness also passes. Independent source review and normal hooks passed. Production files are identical to 68ea941, whose native binary was reused for this test-only validation. Actual macOS framework validation requires the new hosted run; it has not been reproduced locally. An arbitrary copied framework stub as the original parent interpreter remains outside this fixture's pre-existing scope.