crates/vm/src/getpath.rs (1)

123-132: Consider validating the launcher path before use.

The code reads __PYVENV_LAUNCHER__ and uses it directly without verifying that the path is valid or exists. While this mirrors CPython's behavior, consider whether RustPython should add defensive validation to prevent potential issues with malformed or non-existent launcher paths.

🔎 Potential validation approach

 let exe_dir = if let Ok(launcher) = env::var("__PYVENV_LAUNCHER__") {
+    let launcher_path = PathBuf::from(&launcher);
+    if !launcher_path.exists() {
+        // Log warning or fallback to non-launcher behavior
+    }
     paths.executable = launcher.clone();
     paths.base_executable = real_executable;
     PathBuf::from(&launcher).parent().map(PathBuf::from)

crates/venvlauncher/Cargo.toml (1)

19-19: Clarify or remove misleading comment about free-threading.

The comment mentions "Free-threaded variants (RustPython uses Py_GIL_DISABLED=true)" but the launcher implementation doesn't contain any free-threading logic. The launcher simply delegates to the Python executable, so the 't' suffix in the binary name is purely conventional (to match CPython's naming). Consider clarifying that these launchers merely forward to free-threaded Python builds rather than implementing free-threading themselves.

🔎 Suggested clarification

-# Free-threaded variants (RustPython uses Py_GIL_DISABLED=true)
+# Launcher variants for free-threaded Python builds (suffix 't' matches CPython convention)

crates/venvlauncher/src/main.rs (3)

82-86: Consider handling inline comments in configuration values.

The read_home() function doesn't handle inline comments (e.g., home = /path # this is the Python home). While this may not be common in pyvenv.cfg, it could cause issues if users add comments.

🔎 Enhanced parsing with inline comment support

         if let Some(rest) = line.strip_prefix("home") {
             let rest = rest.trim_start();
             if let Some(value) = rest.strip_prefix('=') {
-                return Ok(value.trim().to_string());
+                let value = value.trim();
+                // Strip inline comments
+                let value = value.split('#').next().unwrap_or(value).trim();
+                return Ok(value.to_string());
             }
         }

---

99-99: Redundant type casting in exit code handling.

The code casts Option<i32> to u32 after unwrapping. Since status.code() returns Option<i32>, the pattern could be simplified.

🔎 Simplified version

-    Ok(status.code().unwrap_or(1) as u32)
+    Ok(status.code().unwrap_or(1).max(0) as u32)

Also consider handling negative exit codes (though rare) by clamping to 0.

---

112-120: Test cleanup may fail on panic.

If the test panics before fs::remove_file(), the temporary file will be left behind. Consider using RAII or test fixtures for cleanup.

🔎 Use drop guard for cleanup

struct TempFile(PathBuf);
impl Drop for TempFile {
    fn drop(&mut self) {
        let _ = fs::remove_file(&self.0);
    }
}

#[test]
fn test_read_home() {
    let cfg_path = std::env::temp_dir().join("test_pyvenv.cfg");
    let _guard = TempFile(cfg_path.clone());
    
    let mut file = fs::File::create(&cfg_path).unwrap();
    // ... rest of test
    // No manual cleanup needed
}

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Run `cargo test --workspace --exclude rustpython_wasm` to execute Rust unit tests

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Use `cargo run -- script.py` instead of the standard `python` command to run and test Python code with RustPython

Learnt from: arihant2math
Repo: RustPython/RustPython PR: 5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.

crates/venvlauncher/src/main.rs (2)

15-23: Exit code truncation may lose information.

Casting u32 to u8 on line 17 silently truncates exit codes > 255. Consider clamping to preserve meaningful error indication.

---

62-67: Prefer Command::env over unsafe env::set_var.

The unsafe block for env::set_var is risky—the single-threaded assumption isn't guaranteed by Rust's runtime. The safer approach is to set the environment variable on the child process via Command::env().

🔎 Proposed fix

Modify launch_process to accept the launcher path and set the env var there:

-fn launch_process(exe: &Path, args: &[String]) -> Result<u32, Box<dyn std::error::Error>> {
+fn launch_process(exe: &Path, args: &[String], launcher_path: &Path) -> Result<u32, Box<dyn std::error::Error>> {
     use std::process::Command;
 
-    let status = Command::new(exe).args(args).status()?;
+    let status = Command::new(exe)
+        .args(args)
+        .env("__PYVENV_LAUNCHER__", launcher_path)
+        .status()?;
 
     Ok(status.code().unwrap_or(1) as u32)
 }

Then in run():

-    // 6. Set __PYVENV_LAUNCHER__ environment variable
-    // This tells Python it was launched from a venv
-    // SAFETY: We are in a single-threaded context (program entry point)
-    unsafe {
-        env::set_var("__PYVENV_LAUNCHER__", &exe_path);
-    }
-
     // 7. Launch Python with same arguments
     let args: Vec<String> = env::args().skip(1).collect();
-    launch_process(&python_path, &args)
+    launch_process(&python_path, &args, &exe_path)

crates/venvlauncher/src/main.rs (1)

106-125: Test isolation could be improved for parallel execution.

The tests use fixed temp file names (test_pyvenv.cfg, test_pyvenv2.cfg, etc.) in the shared temp directory. If tests run in parallel, they could interfere with each other. Consider using tempfile crate or unique names.

🔎 Proposed improvement using tempfile

use tempfile::NamedTempFile;

#[test]
fn test_read_home() {
    let mut file = NamedTempFile::new().unwrap();
    writeln!(file, "home = C:\\Python313").unwrap();
    writeln!(file, "include-system-site-packages = false").unwrap();
    writeln!(file, "version = 3.13.0").unwrap();

    let home = read_home(file.path()).unwrap();
    assert_eq!(home, "C:\\Python313");
    // File is automatically deleted when `file` goes out of scope
}

This requires adding tempfile as a dev-dependency in Cargo.toml.

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.

Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.

Learnt from: arihant2math
Repo: RustPython/RustPython PR: 5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to Lib/test/**/*.py : When modifying test files in `Lib/test/`, use `unittest.skip("TODO: RustPython <reason>")` or `unittest.expectedFailure` with `# TODO: RUSTPYTHON <reason>` comment to mark unsupported features

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Run `cargo test --workspace --exclude rustpython_wasm` to execute Rust unit tests

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Always run clippy to lint Rust code (`cargo clippy`) before completing tasks and fix any warnings or lints introduced by changes

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Use `cargo run -- script.py` instead of the standard `python` command to run and test Python code with RustPython

Learnt from: reactive-firewall
Repo: RustPython/RustPython PR: 6176
File: .github/workflows/Check_Tests.yml:133-141
Timestamp: 2025-09-28T22:22:55.921Z
Learning: In the RustPython project's CI-5974-Test-RustPython-Integration action, the override-rustpython-path input is marked as required but has runtime fallback logic that defaults to RUSTPYTHONPATH environment variable or "Lib" if neither is provided, making explicit specification unnecessary in most cases.

Learnt from: arihant2math
Repo: RustPython/RustPython PR: 5790
File: build.rs:2-2
Timestamp: 2025-06-28T16:31:03.991Z
Learning: In Cargo build scripts (build.rs), the environment variable CARGO_CFG_TARGET_OS is guaranteed to exist and is automatically set by Cargo during the build process, making unwrap() safe to use when accessing this variable.

Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust