Skip to content
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

Adjust base Python lookup logic for Windows #2121

Merged
merged 3 commits into from
Mar 3, 2024
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
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()
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
} 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(())
}