Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .cspell.dict/cpython.txt
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ posonlyarg
posonlyargs
prec
preinitialized
pythonw
PYTHREAD_NAME
SA_ONSTACK
SOABI
Expand All @@ -65,6 +66,11 @@ unparse
unparser
VARKEYWORDS
varkwarg
venvlauncher
venvlaunchert
venvw
venvwlauncher
venvwlaunchert
wbits
weakreflist
webpki
Expand Down
11 changes: 8 additions & 3 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ concurrency:
env:
CARGO_ARGS: --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls
CARGO_ARGS_NO_SSL: --no-default-features --features stdlib,importlib,stdio,encodings,sqlite
# Crates excluded from workspace builds:
# - rustpython_wasm: requires wasm target
# - rustpython-compiler-source: deprecated
# - rustpython-venvlauncher: Windows-only
WORKSPACE_EXCLUDES: --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher
Comment on lines +21 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Windows-only crate is excluded on all platforms, including Windows.

The WORKSPACE_EXCLUDES variable includes rustpython-venvlauncher with 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 on windows-latest runners.

Consider making the exclusion conditional based on the runner OS. For example, you could:

  • Define separate environment variables for Windows vs. non-Windows exclusions
  • Use conditional expressions in the cargo commands to exclude the crate only on non-Windows platforms
🔎 Example approach using conditional exclusion

Define platform-specific exclusion variables:

  # Crates excluded from workspace builds:
  # - rustpython_wasm: requires wasm target
  # - rustpython-compiler-source: deprecated
-  # - rustpython-venvlauncher: Windows-only
-  WORKSPACE_EXCLUDES: --exclude rustpython_wasm --exclude rustpython-compiler-source --exclude rustpython-venvlauncher
+  WORKSPACE_EXCLUDES_COMMON: --exclude rustpython_wasm --exclude rustpython-compiler-source
+  # rustpython-venvlauncher: Windows-only, exclude on non-Windows
+  WORKSPACE_EXCLUDES_NON_WINDOWS: --exclude rustpython-venvlauncher

Then update the clippy and test commands to use conditional exclusions:

      - name: run clippy
-        run: cargo clippy ${{ env.CARGO_ARGS }} --workspace --all-targets ${{ env.WORKSPACE_EXCLUDES }} -- -Dwarnings
+        run: cargo clippy ${{ env.CARGO_ARGS }} --workspace --all-targets ${{ env.WORKSPACE_EXCLUDES_COMMON }} ${{ runner.os != 'Windows' && env.WORKSPACE_EXCLUDES_NON_WINDOWS || '' }} -- -Dwarnings

Committable suggestion skipped: line range outside the PR's diff.

# Skip additional tests on Windows. They are checked on Linux and MacOS.
# test_glob: many failing tests
# test_pathlib: panic by surrogate chars
Expand Down Expand Up @@ -135,13 +140,13 @@ jobs:
if: runner.os == 'macOS'

- name: run clippy
run: cargo clippy ${{ env.CARGO_ARGS }} --workspace --all-targets --exclude rustpython_wasm --exclude rustpython-compiler-source -- -Dwarnings
run: cargo clippy ${{ env.CARGO_ARGS }} --workspace --all-targets ${{ env.WORKSPACE_EXCLUDES }} -- -Dwarnings

- name: run rust tests
run: cargo test --workspace --exclude rustpython_wasm --exclude rustpython-compiler-source --verbose --features threading ${{ env.CARGO_ARGS }}
run: cargo test --workspace ${{ env.WORKSPACE_EXCLUDES }} --verbose --features threading ${{ env.CARGO_ARGS }}
if: runner.os != 'macOS'
- name: run rust tests
run: cargo test --workspace --exclude rustpython_wasm --exclude rustpython-jit --exclude rustpython-compiler-source --verbose --features threading ${{ env.CARGO_ARGS }}
run: cargo test --workspace ${{ env.WORKSPACE_EXCLUDES }} --exclude rustpython-jit --verbose --features threading ${{ env.CARGO_ARGS }}
if: runner.os == 'macOS'

- name: check compilation without threading
Expand Down
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file added Lib/venv/scripts/nt/venvlauncher.exe
Binary file not shown.
Binary file added Lib/venv/scripts/nt/venvlaunchert.exe
Binary file not shown.
Binary file added Lib/venv/scripts/nt/venvwlauncher.exe
Binary file not shown.
Binary file added Lib/venv/scripts/nt/venvwlaunchert.exe
Binary file not shown.
39 changes: 39 additions & 0 deletions crates/venvlauncher/Cargo.toml
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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/null

Repository: RustPython/RustPython

Length of output: 280


🏁 Script executed:

cat -n crates/venvlauncher/src/main.rs

Repository: 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 venvlaunchert and venvwlaunchert variants that indicate free-threaded mode.

The current implementation will treat all four binary variants identically:

  • venvlauncher → python.exe ✓
  • venvwlauncher → pythonw.exe ✓
  • venvlaunchert → python.exe (should be free-threaded)
  • venvwlaunchert → pythonw.exe (should be free-threaded)

Add logic to detect the "t" suffix and select the appropriate free-threaded Python executable (e.g., pythont.exe / pythonwt.exe) or set an environment variable like PYTHONFREETHREADING=1 when the binary name contains the "t" suffix.


[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
21 changes: 21 additions & 0 deletions crates/venvlauncher/build.rs
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");
}
}
}
155 changes: 155 additions & 0 deletions crates/venvlauncher/src/main.rs
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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Exit code truncation may lose information.

Casting u32 to u8 truncates exit codes greater than 255. While most programs use codes 0-127, this could lose information for programs that use higher exit codes.

🔎 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Ok(code) => ExitCode::from(code as u8),
Ok(code) => ExitCode::from(code.min(255) as u8),
🤖 Prompt for AI Agents
In crates/venvlauncher/src/main.rs around line 13, the Ok(code) =>
ExitCode::from(code as u8) cast truncates u32 exit codes above 255; replace the
blind cast with a clamp-to-255 approach (e.g., take min(code, 255) and convert
that to u8) so values >255 map to 255 instead of wrapping, and optionally log or
comment that higher exit codes are being clamped per POSIX convention.

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"
};

// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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 main() runs, or if future code changes introduce threading, this unsafe block becomes unsound. Rust's env::set_var is unsafe precisely because it's not thread-safe.

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.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In crates/venvlauncher/src/main.rs around lines 60-63, the code unsafely sets a
process-wide env var with an unsafe block which risks a race condition; remove
the unsafe env::set_var call and instead set the variable on the child process
invocation (use Command::env to pass "__PYVENV_LAUNCHER__" => exe_path when
spawning the Python process) so no global env mutation is needed; if a global is
truly required, ensure it is done deterministically before any threads start
(e.g., perform std::env::set_var during single-threaded early init guarded by a
Once) but prefer Command::env to avoid thread-safety issues entirely.


// 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();
}
}
23 changes: 15 additions & 8 deletions crates/vm/src/getpath.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,19 +110,26 @@ pub fn init_path_config(settings: &Settings) -> Paths {

// Step 0: Get executable path
let executable = get_executable_path();
paths.executable = executable
let real_executable = executable
.as_ref()
.map(|p| p.to_string_lossy().into_owned())
.unwrap_or_default();

let exe_dir = executable
.as_ref()
.and_then(|p| p.parent().map(PathBuf::from));

// Step 1: Check for __PYVENV_LAUNCHER__ environment variable
if let Ok(launcher) = env::var("__PYVENV_LAUNCHER__") {
paths.base_executable = launcher;
}
// When launched from a venv launcher, __PYVENV_LAUNCHER__ contains the venv's python.exe path
// In this case:
// - sys.executable should be the launcher path (where user invoked Python)
// - sys._base_executable should be the real Python executable
let exe_dir = if let Ok(launcher) = env::var("__PYVENV_LAUNCHER__") {
paths.executable = launcher.clone();
paths.base_executable = real_executable;
PathBuf::from(&launcher).parent().map(PathBuf::from)
} else {
paths.executable = real_executable;
executable
.as_ref()
.and_then(|p| p.parent().map(PathBuf::from))
};

// Step 2: Check for venv (pyvenv.cfg) and get 'home'
let (venv_prefix, home_dir) = detect_venv(&exe_dir);
Expand Down
Loading