-
Notifications
You must be signed in to change notification settings - Fork 1.4k
venvlauncher #6527
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
venvlauncher #6527
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| [package] | ||
| name = "rustpython-venvlauncher" | ||
| description = "Lightweight venv launcher for RustPython" | ||
| version.workspace = true | ||
| authors.workspace = true | ||
| edition.workspace = true | ||
| rust-version.workspace = true | ||
| repository.workspace = true | ||
| license.workspace = true | ||
|
|
||
| [[bin]] | ||
| name = "venvlauncher" | ||
| path = "src/main.rs" | ||
|
|
||
| [[bin]] | ||
| name = "venvwlauncher" | ||
| path = "src/main.rs" | ||
|
|
||
| # Free-threaded variants (RustPython uses Py_GIL_DISABLED=true) | ||
| [[bin]] | ||
| name = "venvlaunchert" | ||
| path = "src/main.rs" | ||
|
|
||
| [[bin]] | ||
| name = "venvwlaunchert" | ||
| path = "src/main.rs" | ||
|
Comment on lines
+11
to
+26
Contributor
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. 🧩 Analysis chain🏁 Script executed: # First, check if the file exists and its size
stat crates/venvlauncher/src/main.rs 2>/dev/null || echo "File not found, checking directory structure..."
ls -la crates/venvlauncher/src/ 2>/dev/null || find . -name "main.rs" -path "*/venvlauncher/*" 2>/dev/nullRepository: RustPython/RustPython Length of output: 280 🏁 Script executed: cat -n crates/venvlauncher/src/main.rsRepository: RustPython/RustPython Length of output: 6052 The main.rs implementation is incomplete—it detects console vs GUI subsystem but does not handle free-threaded mode variants. The code at lines 37-41 correctly distinguishes between console (python.exe) and GUI (pythonw.exe) variants by checking if the binary name contains "pythonw" or "venvw". However, it does not detect the "t" suffix in The current implementation will treat all four binary variants identically:
Add logic to detect the "t" suffix and select the appropriate free-threaded Python executable (e.g., |
||
|
|
||
| [target.'cfg(windows)'.dependencies] | ||
| windows-sys = { workspace = true, features = [ | ||
| "Win32_Foundation", | ||
| "Win32_System_Threading", | ||
| "Win32_System_Environment", | ||
| "Win32_Storage_FileSystem", | ||
| "Win32_System_Console", | ||
| "Win32_Security", | ||
| ] } | ||
|
|
||
| [lints] | ||
| workspace = true | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| //! Build script for venvlauncher | ||
| //! | ||
| //! Sets the Windows subsystem to GUI for venvwlauncher variants. | ||
| //! Only MSVC toolchain is supported on Windows (same as CPython). | ||
|
|
||
| fn main() { | ||
| let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap(); | ||
| let target_env = std::env::var("CARGO_CFG_TARGET_ENV").unwrap_or_default(); | ||
|
|
||
| // Only apply on Windows with MSVC toolchain | ||
| if target_os == "windows" && target_env == "msvc" { | ||
| let exe_name = std::env::var("CARGO_BIN_NAME").unwrap_or_default(); | ||
|
|
||
| // venvwlauncher and venvwlaunchert should be Windows GUI applications | ||
| // (no console window) | ||
| if exe_name.contains("venvw") { | ||
| println!("cargo:rustc-link-arg=/SUBSYSTEM:WINDOWS"); | ||
| println!("cargo:rustc-link-arg=/ENTRY:mainCRTStartup"); | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,155 @@ | ||||||
| //! RustPython venv launcher | ||||||
| //! | ||||||
| //! A lightweight launcher that reads pyvenv.cfg and delegates execution | ||||||
| //! to the actual Python interpreter. This mimics CPython's venvlauncher.c. | ||||||
| //! Windows only. | ||||||
|
|
||||||
| #[cfg(not(windows))] | ||||||
| compile_error!("venvlauncher is only supported on Windows"); | ||||||
|
|
||||||
| use std::env; | ||||||
| use std::fs; | ||||||
| use std::path::{Path, PathBuf}; | ||||||
| use std::process::ExitCode; | ||||||
|
|
||||||
| fn main() -> ExitCode { | ||||||
| match run() { | ||||||
| Ok(code) => ExitCode::from(code as u8), | ||||||
|
Contributor
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. Exit code truncation may lose information. Casting 🔎 Consider clamping instead of truncating- Ok(code) => ExitCode::from(code as u8),
+ Ok(code) => ExitCode::from(code.min(255) as u8),Or document that exit codes > 255 are intentionally truncated per POSIX conventions. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| Err(e) => { | ||||||
| eprintln!("venvlauncher error: {}", e); | ||||||
| ExitCode::from(1) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn run() -> Result<u32, Box<dyn std::error::Error>> { | ||||||
| // 1. Get own executable path | ||||||
| let exe_path = env::current_exe()?; | ||||||
| let exe_name = exe_path | ||||||
| .file_name() | ||||||
| .ok_or("Failed to get executable name")? | ||||||
| .to_string_lossy(); | ||||||
|
|
||||||
| // 2. Determine target executable name based on launcher name | ||||||
| // pythonw.exe / venvwlauncher -> pythonw.exe (GUI, no console) | ||||||
| // python.exe / venvlauncher -> python.exe (console) | ||||||
| let exe_name_lower = exe_name.to_lowercase(); | ||||||
| let target_exe = if exe_name_lower.contains("pythonw") || exe_name_lower.contains("venvw") { | ||||||
| "pythonw.exe" | ||||||
| } else { | ||||||
| "python.exe" | ||||||
| }; | ||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
|
||||||
| // 3. Find pyvenv.cfg | ||||||
| // The launcher is in Scripts/ directory, pyvenv.cfg is in parent (venv root) | ||||||
| let scripts_dir = exe_path.parent().ok_or("Failed to get Scripts directory")?; | ||||||
| let venv_dir = scripts_dir.parent().ok_or("Failed to get venv directory")?; | ||||||
| let cfg_path = venv_dir.join("pyvenv.cfg"); | ||||||
|
|
||||||
| if !cfg_path.exists() { | ||||||
| return Err(format!("pyvenv.cfg not found: {}", cfg_path.display()).into()); | ||||||
| } | ||||||
|
|
||||||
| // 4. Parse home= from pyvenv.cfg | ||||||
| let home = read_home(&cfg_path)?; | ||||||
|
|
||||||
| // 5. Locate python executable in home directory | ||||||
| let python_path = PathBuf::from(&home).join(target_exe); | ||||||
| if !python_path.exists() { | ||||||
| return Err(format!("Python not found: {}", python_path.display()).into()); | ||||||
| } | ||||||
|
|
||||||
| // 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); | ||||||
| } | ||||||
|
Comment on lines
+64
to
+67
Contributor
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. Unsafe environment variable modification has race condition risk. The comment claims "single-threaded context" but this isn't guaranteed. If the Rust runtime or any dependency spawns threads before Consider using the safe alternative: 🔎 Safe alternative using Command::env- // 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)
}
-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()?;This approach is safer and avoids the race condition entirely.
🤖 Prompt for AI Agents |
||||||
|
|
||||||
| // 7. Launch Python with same arguments | ||||||
| let args: Vec<String> = env::args().skip(1).collect(); | ||||||
| launch_process(&python_path, &args) | ||||||
| } | ||||||
|
|
||||||
| /// Parse the `home=` value from pyvenv.cfg | ||||||
| fn read_home(cfg_path: &Path) -> Result<String, Box<dyn std::error::Error>> { | ||||||
| let content = fs::read_to_string(cfg_path)?; | ||||||
|
|
||||||
| for line in content.lines() { | ||||||
| let line = line.trim(); | ||||||
| // Skip comments and empty lines | ||||||
| if line.is_empty() || line.starts_with('#') { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| // Look for "home = <path>" or "home=<path>" | ||||||
| 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()); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| Err("'home' key not found in pyvenv.cfg".into()) | ||||||
| } | ||||||
|
|
||||||
| /// Launch the Python process and wait for it to complete | ||||||
| fn launch_process(exe: &Path, args: &[String]) -> Result<u32, Box<dyn std::error::Error>> { | ||||||
| use std::process::Command; | ||||||
|
|
||||||
| let status = Command::new(exe).args(args).status()?; | ||||||
|
|
||||||
| Ok(status.code().unwrap_or(1) as u32) | ||||||
| } | ||||||
|
|
||||||
| #[cfg(all(test, windows))] | ||||||
| mod tests { | ||||||
| use super::*; | ||||||
| use std::io::Write; | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_read_home() { | ||||||
| let temp_dir = std::env::temp_dir(); | ||||||
| let cfg_path = temp_dir.join("test_pyvenv.cfg"); | ||||||
|
|
||||||
| let mut file = fs::File::create(&cfg_path).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(&cfg_path).unwrap(); | ||||||
| assert_eq!(home, "C:\\Python313"); | ||||||
|
|
||||||
| fs::remove_file(&cfg_path).unwrap(); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_read_home_no_spaces() { | ||||||
| let temp_dir = std::env::temp_dir(); | ||||||
| let cfg_path = temp_dir.join("test_pyvenv2.cfg"); | ||||||
|
|
||||||
| let mut file = fs::File::create(&cfg_path).unwrap(); | ||||||
| writeln!(file, "home=C:\\Python313").unwrap(); | ||||||
|
|
||||||
| let home = read_home(&cfg_path).unwrap(); | ||||||
| assert_eq!(home, "C:\\Python313"); | ||||||
|
|
||||||
| fs::remove_file(&cfg_path).unwrap(); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_read_home_with_comments() { | ||||||
| let temp_dir = std::env::temp_dir(); | ||||||
| let cfg_path = temp_dir.join("test_pyvenv3.cfg"); | ||||||
|
|
||||||
| let mut file = fs::File::create(&cfg_path).unwrap(); | ||||||
| writeln!(file, "# This is a comment").unwrap(); | ||||||
| writeln!(file, "home = D:\\RustPython").unwrap(); | ||||||
|
|
||||||
| let home = read_home(&cfg_path).unwrap(); | ||||||
| assert_eq!(home, "D:\\RustPython"); | ||||||
|
|
||||||
| fs::remove_file(&cfg_path).unwrap(); | ||||||
| } | ||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| } | ||||||
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.
Windows-only crate is excluded on all platforms, including Windows.
The
WORKSPACE_EXCLUDESvariable includesrustpython-venvlauncherwith a "Windows-only" comment, but this exclusion applies to all CI runs, including Windows builds. This means the Windows venv launcher will never be compiled or tested in CI, even onwindows-latestrunners.Consider making the exclusion conditional based on the runner OS. For example, you could:
🔎 Example approach using conditional exclusion
Define platform-specific exclusion variables:
Then update the clippy and test commands to use conditional exclusions: