Skip to content

Commit

Permalink
Adjust base Python lookup logic for Windows (#2121)
Browse files Browse the repository at this point in the history
## Summary

When I install via the Windows Store, `interpreter.base_prefix` contains
a bunch of resolved information that leads to a broken environment.

Instead, we now use `sys._base_executable` on Windows by default,
falling back to `sys.base_prefix` if it doesn't exist. (There are some
issues with `sys.base_executable` that lead to complexity in
`virtualenv`, but they only affect POSIX.) Admittedly, I don't know when
`sys._base_executable` wouldn't exist. It exists in all the environments
I've tested.

Additionally, we use the system interpreter directly if we're outside of
a virtualenv.
  • Loading branch information
charliermarsh committed Mar 3, 2024
1 parent d4f1973 commit 46265b7
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 25 deletions.
1 change: 1 addition & 0 deletions crates/uv-interpreter/src/get_interpreter_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ def format_full_version(info):
"base_prefix": sys.base_prefix,
"base_exec_prefix": sys.base_exec_prefix,
"prefix": sys.prefix,
"base_executable": getattr(sys, "_base_executable", None),
"sys_executable": sys.executable,
"sysconfig_paths": sysconfig.get_paths(),
}
Expand Down
10 changes: 10 additions & 0 deletions crates/uv-interpreter/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub struct Interpreter {
prefix: PathBuf,
base_exec_prefix: PathBuf,
base_prefix: PathBuf,
base_executable: Option<PathBuf>,
sys_executable: PathBuf,
tags: OnceCell<Tags>,
}
Expand All @@ -54,6 +55,7 @@ impl Interpreter {
prefix: info.prefix,
base_exec_prefix: info.base_exec_prefix,
base_prefix: info.base_prefix,
base_executable: info.base_executable,
sys_executable: info.sys_executable,
tags: OnceCell::new(),
})
Expand All @@ -77,6 +79,7 @@ impl Interpreter {
prefix: PathBuf::from("/dev/null"),
base_exec_prefix: PathBuf::from("/dev/null"),
base_prefix: PathBuf::from("/dev/null"),
base_executable: None,
sys_executable: PathBuf::from("/dev/null"),
tags: OnceCell::new(),
}
Expand Down Expand Up @@ -355,6 +358,12 @@ impl Interpreter {
&self.prefix
}

/// Return the `sys._base_executable` path for this Python interpreter. Some platforms do not
/// have this attribute, so it may be `None`.
pub fn base_executable(&self) -> Option<&Path> {
self.base_executable.as_deref()
}

/// Return the `sys.executable` path for this Python interpreter.
pub fn sys_executable(&self) -> &Path {
&self.sys_executable
Expand Down Expand Up @@ -455,6 +464,7 @@ struct InterpreterInfo {
prefix: PathBuf,
base_exec_prefix: PathBuf,
base_prefix: PathBuf,
base_executable: Option<PathBuf>,
sys_executable: PathBuf,
}

Expand Down
66 changes: 41 additions & 25 deletions crates/uv-virtualenv/src/bare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,36 @@ pub fn create_bare_venv(
system_site_packages: bool,
extra_cfg: Vec<(String, String)>,
) -> Result<Virtualenv, Error> {
// We have to canonicalize the interpreter path, otherwise the home is set to the venv dir instead of the real root.
// This would make python-build-standalone fail with the encodings module not being found because its home is wrong.
let base_python = fs_err::canonicalize(interpreter.sys_executable())?;
// Determine the base Python executable; that is, the Python executable that should be
// considered the "base" for the virtual environment. This is typically the Python executable
// from the [`Interpreter`]; however, if the interpreter is a virtual environment itself, then
// the base Python executable is the Python executable of the interpreter's base interpreter.
let base_python = if cfg!(unix) {
// On Unix, follow symlinks to resolve the base interpreter, since the Python executable in
// a virtual environment is a symlink to the base interpreter.
fs_err::canonicalize(interpreter.sys_executable())?
} else if cfg!(windows) {
// On Windows, follow `virtualenv`. If we're in a virtual environment, use
// `sys._base_executable` if it exists; if not, use `sys.base_prefix`. For example, with
// Python installed from the Windows Store, `sys.base_prefix` is slightly "incorrect".
//
// If we're _not_ in a virtual environment, use the interpreter's executable, since it's
// already a "system Python". We canonicalize the path to ensure that it's real and
// consistent, though we don't expect any symlinks on Windows.
if interpreter.is_virtualenv() {
if let Some(base_executable) = interpreter.base_executable() {
base_executable.to_path_buf()
} else {
// Assume `python.exe`, though the exact executable name is never used (below) on
// Windows, only its parent directory.
interpreter.base_prefix().join("python.exe")
}
} else {
fs_err::canonicalize(interpreter.sys_executable())?
}
} else {
unimplemented!("Only Windows and Unix are supported")
};

// Validate the existing location.
match location.metadata() {
Expand Down Expand Up @@ -188,28 +215,17 @@ pub fn create_bare_venv(
fs::write(scripts.join(name), activator)?;
}

// pyvenv.cfg
let python_home = if cfg!(unix) {
// On Linux and Mac, Python is symlinked so the base home is the parent of the resolved-by-canonicalize path.
base_python
.parent()
.ok_or_else(|| {
io::Error::new(
io::ErrorKind::NotFound,
"The python interpreter needs to have a parent directory",
)
})?
.simplified_display()
.to_string()
} else if cfg!(windows) {
// `virtualenv` seems to rely on the undocumented, private `sys._base_executable`. When I tried,
// `sys.base_prefix` was the same as the parent of `sys._base_executable`, but a much simpler logic and
// documented.
// https://github.com/pypa/virtualenv/blob/d9fdf48d69f0d0ca56140cf0381edbb5d6fe09f5/src/virtualenv/discovery/py_info.py#L136-L156
interpreter.base_prefix().simplified_display().to_string()
} else {
unimplemented!("Only Windows and Unix are supported")
};
// Per PEP 405, the Python `home` is the parent directory of the interpreter.
let python_home = base_python
.parent()
.ok_or_else(|| {
io::Error::new(
io::ErrorKind::NotFound,
"The Python interpreter needs to have a parent directory",
)
})?
.simplified_display()
.to_string();

// Validate extra_cfg
let reserved_keys = [
Expand Down
68 changes: 68 additions & 0 deletions crates/uv/tests/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::process::Command;

use anyhow::Result;
use assert_cmd::prelude::*;
use assert_fs::prelude::*;

use uv_fs::Simplified;
Expand Down Expand Up @@ -663,3 +664,70 @@ fn verify_pyvenv_cfg() {
let search_string = format!("uv = {version}");
pyvenv_cfg.assert(predicates::str::contains(search_string));
}

/// Ensure that a nested virtual environment uses the same `home` directory as the parent.
#[test]
fn verify_nested_pyvenv_cfg() -> Result<()> {
let temp_dir = assert_fs::TempDir::new()?;
let cache_dir = assert_fs::TempDir::new()?;
let bin = create_bin_with_executables(&temp_dir, &["3.12"]).expect("Failed to create bin dir");
let venv = temp_dir.child(".venv");

// Create a virtual environment at `.venv`.
Command::new(get_bin())
.arg("venv")
.arg(venv.as_os_str())
.arg("--python")
.arg("3.12")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("UV_TEST_PYTHON_PATH", bin.clone())
.current_dir(&temp_dir)
.assert()
.success();

let pyvenv_cfg = venv.child("pyvenv.cfg");

// Check pyvenv.cfg exists
pyvenv_cfg.assert(predicates::path::is_file());

// Extract the "home" line from the pyvenv.cfg file.
let contents = fs_err::read_to_string(pyvenv_cfg.path())?;
let venv_home = contents
.lines()
.find(|line| line.starts_with("home"))
.expect("home line not found");

// Now, create a virtual environment from within the virtual environment.
let subvenv = temp_dir.child(".subvenv");
Command::new(get_bin())
.arg("venv")
.arg(subvenv.as_os_str())
.arg("--python")
.arg("3.12")
.arg("--cache-dir")
.arg(cache_dir.path())
.arg("--exclude-newer")
.arg(EXCLUDE_NEWER)
.env("VIRTUAL_ENV", venv.as_os_str())
.env("UV_TEST_PYTHON_PATH", bin.clone())
.current_dir(&temp_dir)
.assert()
.success();

let sub_pyvenv_cfg = subvenv.child("pyvenv.cfg");

// Extract the "home" line from the pyvenv.cfg file.
let contents = fs_err::read_to_string(sub_pyvenv_cfg.path())?;
let sub_venv_home = contents
.lines()
.find(|line| line.starts_with("home"))
.expect("home line not found");

// Check that both directories point to the same home.
assert_eq!(sub_venv_home, venv_home);

Ok(())
}

0 comments on commit 46265b7

Please sign in to comment.