From 46265b711b3e79b72099e2116cdf1ea8dd643ab2 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 3 Mar 2024 09:44:10 -0800 Subject: [PATCH] Adjust base Python lookup logic for Windows (#2121) ## 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. --- .../src/get_interpreter_info.py | 1 + crates/uv-interpreter/src/interpreter.rs | 10 +++ crates/uv-virtualenv/src/bare.rs | 66 +++++++++++------- crates/uv/tests/venv.rs | 68 +++++++++++++++++++ 4 files changed, 120 insertions(+), 25 deletions(-) diff --git a/crates/uv-interpreter/src/get_interpreter_info.py b/crates/uv-interpreter/src/get_interpreter_info.py index d6d82a645047..cb31dee99a80 100644 --- a/crates/uv-interpreter/src/get_interpreter_info.py +++ b/crates/uv-interpreter/src/get_interpreter_info.py @@ -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(), } diff --git a/crates/uv-interpreter/src/interpreter.rs b/crates/uv-interpreter/src/interpreter.rs index 961978b08a69..de0e0e57b43c 100644 --- a/crates/uv-interpreter/src/interpreter.rs +++ b/crates/uv-interpreter/src/interpreter.rs @@ -32,6 +32,7 @@ pub struct Interpreter { prefix: PathBuf, base_exec_prefix: PathBuf, base_prefix: PathBuf, + base_executable: Option, sys_executable: PathBuf, tags: OnceCell, } @@ -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(), }) @@ -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(), } @@ -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 @@ -455,6 +464,7 @@ struct InterpreterInfo { prefix: PathBuf, base_exec_prefix: PathBuf, base_prefix: PathBuf, + base_executable: Option, sys_executable: PathBuf, } diff --git a/crates/uv-virtualenv/src/bare.rs b/crates/uv-virtualenv/src/bare.rs index c79f1dfd485b..7c3ad74d02a0 100644 --- a/crates/uv-virtualenv/src/bare.rs +++ b/crates/uv-virtualenv/src/bare.rs @@ -48,9 +48,36 @@ pub fn create_bare_venv( system_site_packages: bool, extra_cfg: Vec<(String, String)>, ) -> Result { - // 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() { @@ -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 = [ diff --git a/crates/uv/tests/venv.rs b/crates/uv/tests/venv.rs index ea3c213fa545..3e314e7bdaad 100644 --- a/crates/uv/tests/venv.rs +++ b/crates/uv/tests/venv.rs @@ -3,6 +3,7 @@ use std::process::Command; use anyhow::Result; +use assert_cmd::prelude::*; use assert_fs::prelude::*; use uv_fs::Simplified; @@ -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(()) +}