Upgrade venv to v3.13.11 #6459
Upgrade venv to v3.13.11 #6459
Conversation
WalkthroughThe change adds virtual environment detection support to the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/vm/src/stdlib/sys.rs (1)
212-214: Consider edge case: config lines with empty values.The parsing uses
split_once('=')which correctly handles the first=, but doesn't validate that the value after trimming is non-empty. While this might work fine (an empty home path would fail later checks), explicitly handling this edge case would make the code more robust.🔎 Suggested enhancement for edge case handling
for line in content.lines() { if let Some((key, value)) = line.split_once('=') { let key = key.trim().to_lowercase(); let value = value.trim(); + + if value.is_empty() { + continue; + } if key == "home" {
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
Lib/test/test_venv.pyis excluded by!Lib/**Lib/venv/__init__.pyis excluded by!Lib/**Lib/venv/__main__.pyis excluded by!Lib/**Lib/venv/scripts/common/Activate.ps1is excluded by!Lib/**Lib/venv/scripts/common/activateis excluded by!Lib/**Lib/venv/scripts/common/activate.fishis excluded by!Lib/**Lib/venv/scripts/nt/activate.batis excluded by!Lib/**Lib/venv/scripts/posix/activate.cshis excluded by!Lib/**
📒 Files selected for processing (1)
crates/vm/src/stdlib/sys.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/stdlib/sys.rs
🔇 Additional comments (3)
crates/vm/src/stdlib/sys.rs (3)
167-178: LGTM! Clear priority order for executable detection.The logic correctly prioritizes
__PYVENV_LAUNCHER__environment variable first, then venv detection via config file, and finally falls back to the existing executable detection. This is a sensible implementation.
189-189: No action required—which::which()API usage is correct and compatible withwhichcrate v8.0.0.
218-222: Verify symlink resolution logic against Python's getpath.py implementation.The current code resolves symlinks independently and returns immediately if a different path is found, using the parsed
homedirectory value only as a fallback if symlink resolution doesn't produce a different path. This means the home value parsed from pyvenv.cfg is potentially ignored when symlink resolution succeeds.Clarify whether this matches Python's getpath.py behavior (lines 373-377 and 381):
- If Python's getpath.py also returns the resolved symlink path without applying the home value, the current logic is correct.
- If Python applies the home directory value regardless of symlink resolution, this is a logic error that should use the home value instead.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.