From 6481aa3e64f1fad2f418502cb6b0103353790ee6 Mon Sep 17 00:00:00 2001 From: John Mumm Date: Mon, 23 Jun 2025 09:12:43 -0400 Subject: [PATCH] Consolidate logic for checking for a virtual environment (#14214) We were checking whether a path was an executable in a virtual environment or the base directory of a virtual environment in multiple places in the codebase. This PR consolidates this logic into one place. Closes #13947. --- crates/uv-fs/src/lib.rs | 24 ++++++++++++++++++++++++ crates/uv-python/src/discovery.rs | 9 ++------- crates/uv-python/src/interpreter.rs | 7 +------ crates/uv-python/src/virtualenv.rs | 4 ++-- crates/uv-virtualenv/src/virtualenv.rs | 2 +- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/crates/uv-fs/src/lib.rs b/crates/uv-fs/src/lib.rs index 0b5055b40..ad4a883ad 100644 --- a/crates/uv-fs/src/lib.rs +++ b/crates/uv-fs/src/lib.rs @@ -575,6 +575,30 @@ pub fn is_temporary(path: impl AsRef) -> bool { .is_some_and(|name| name.starts_with(".tmp")) } +/// Checks if the grandparent directory of the given executable is the base +/// of a virtual environment. +/// +/// The procedure described in PEP 405 includes checking both the parent and +/// grandparent directory of an executable, but in practice we've found this to +/// be unnecessary. +pub fn is_virtualenv_executable(executable: impl AsRef) -> bool { + executable + .as_ref() + .parent() + .and_then(Path::parent) + .is_some_and(is_virtualenv_base) +} + +/// Returns `true` if a path is the base path of a virtual environment, +/// indicated by the presence of a `pyvenv.cfg` file. +/// +/// The procedure described in PEP 405 includes scanning `pyvenv.cfg` +/// for a `home` key, but in practice we've found this to be +/// unnecessary. +pub fn is_virtualenv_base(path: impl AsRef) -> bool { + path.as_ref().join("pyvenv.cfg").is_file() +} + /// A file lock that is automatically released when dropped. #[derive(Debug)] pub struct LockedFile(fs_err::File); diff --git a/crates/uv-python/src/discovery.rs b/crates/uv-python/src/discovery.rs index eaf4b4830..d1f3a690a 100644 --- a/crates/uv-python/src/discovery.rs +++ b/crates/uv-python/src/discovery.rs @@ -888,13 +888,8 @@ impl Error { | InterpreterError::BrokenSymlink(BrokenSymlink { path, .. }) => { // If the interpreter is from an active, valid virtual environment, we should // fail because it's broken - if let Some(Ok(true)) = matches!(source, PythonSource::ActiveEnvironment) - .then(|| { - path.parent() - .and_then(Path::parent) - .map(|path| path.join("pyvenv.cfg").try_exists()) - }) - .flatten() + if matches!(source, PythonSource::ActiveEnvironment) + && uv_fs::is_virtualenv_executable(path) { true } else { diff --git a/crates/uv-python/src/interpreter.rs b/crates/uv-python/src/interpreter.rs index 19e790b7e..b62633283 100644 --- a/crates/uv-python/src/interpreter.rs +++ b/crates/uv-python/src/interpreter.rs @@ -993,14 +993,9 @@ impl InterpreterInfo { .symlink_metadata() .is_ok_and(|metadata| metadata.is_symlink()) { - let venv = executable - .parent() - .and_then(Path::parent) - .map(|path| path.join("pyvenv.cfg").is_file()) - .unwrap_or(false); Error::BrokenSymlink(BrokenSymlink { path: executable.to_path_buf(), - venv, + venv: uv_fs::is_virtualenv_executable(executable), }) } else { Error::NotFound(executable.to_path_buf()) diff --git a/crates/uv-python/src/virtualenv.rs b/crates/uv-python/src/virtualenv.rs index ea578fff3..8b51a5e1b 100644 --- a/crates/uv-python/src/virtualenv.rs +++ b/crates/uv-python/src/virtualenv.rs @@ -130,14 +130,14 @@ pub(crate) fn virtualenv_from_working_dir() -> Result, Error> { for dir in current_dir.ancestors() { // If we're _within_ a virtualenv, return it. - if dir.join("pyvenv.cfg").is_file() { + if uv_fs::is_virtualenv_base(dir) { return Ok(Some(dir.to_path_buf())); } // Otherwise, search for a `.venv` directory. let dot_venv = dir.join(".venv"); if dot_venv.is_dir() { - if !dot_venv.join("pyvenv.cfg").is_file() { + if !uv_fs::is_virtualenv_base(&dot_venv) { return Err(Error::MissingPyVenvCfg(dot_venv)); } return Ok(Some(dot_venv)); diff --git a/crates/uv-virtualenv/src/virtualenv.rs b/crates/uv-virtualenv/src/virtualenv.rs index bb6db01a3..f466233c0 100644 --- a/crates/uv-virtualenv/src/virtualenv.rs +++ b/crates/uv-virtualenv/src/virtualenv.rs @@ -85,7 +85,7 @@ pub(crate) fn create( } else if metadata.is_dir() { if allow_existing { debug!("Allowing existing directory"); - } else if location.join("pyvenv.cfg").is_file() { + } else if uv_fs::is_virtualenv_base(location) { debug!("Removing existing directory"); // On Windows, if the current executable is in the directory, guard against