From 23b9dce30aaf6f9cd54b752b90752b5fa1c4f50c Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Fri, 20 Sep 2024 09:14:03 -0500 Subject: [PATCH] Allow system environments during project environment validity check --- crates/uv-python/src/environment.rs | 36 +++++++++++++++++++++----- crates/uv-python/src/lib.rs | 2 +- crates/uv/src/commands/project/mod.rs | 37 +++++++++++++++++---------- crates/uv/tests/sync.rs | 19 +++++--------- 4 files changed, 60 insertions(+), 34 deletions(-) diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index e254e3095ba1f..fb181f3e0c0af 100644 --- a/crates/uv-python/src/environment.rs +++ b/crates/uv-python/src/environment.rs @@ -37,7 +37,12 @@ pub struct EnvironmentNotFound { #[derive(Clone, Debug, Error)] pub struct InvalidEnvironment { path: PathBuf, - reason: String, + pub kind: InvalidEnvironmentKind, +} +#[derive(Debug, Clone)] +pub enum InvalidEnvironmentKind { + NotDirectory, + MissingExecutable(PathBuf), } impl From for EnvironmentNotFound { @@ -110,11 +115,22 @@ impl std::fmt::Display for InvalidEnvironment { f, "Invalid environment at `{}`: {}", self.path.user_display(), - self.reason + self.kind ) } } +impl std::fmt::Display for InvalidEnvironmentKind { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + Self::NotDirectory => write!(f, "expected directory but found a file"), + Self::MissingExecutable(path) => { + write!(f, "missing Python executable at `{}`", path.user_display()) + } + } + } +} + impl PythonEnvironment { /// Find a [`PythonEnvironment`] matching the given request and preference. /// @@ -139,6 +155,8 @@ impl PythonEnvironment { } /// Create a [`PythonEnvironment`] from the virtual environment at the given root. + /// + /// N.B. This function also works for system Python environments and users depend on this. pub fn from_root(root: impl AsRef, cache: &Cache) -> Result { let venv = match fs_err::canonicalize(root.as_ref()) { Ok(venv) => venv, @@ -154,20 +172,24 @@ impl PythonEnvironment { if venv.is_file() { return Err(InvalidEnvironment { path: venv, - reason: "expected directory but found a file".to_string(), + kind: InvalidEnvironmentKind::NotDirectory, } .into()); } - if !venv.join("pyvenv.cfg").is_file() { + let executable = virtualenv_python_executable(&venv); + + // Check if the executable exists before querying so we can provide a more specific error + // Note we intentionally don't require a resolved link to exist here, we're just trying to + // tell if this _looks_ like a Python environment. + if !(executable.is_symlink() || executable.is_file()) { return Err(InvalidEnvironment { path: venv, - reason: "missing a `pyvenv.cfg` marker".to_string(), + kind: InvalidEnvironmentKind::MissingExecutable(executable.clone()), } .into()); - } + }; - let executable = virtualenv_python_executable(venv); let interpreter = Interpreter::query(executable, cache)?; Ok(Self(Arc::new(PythonEnvironmentShared { diff --git a/crates/uv-python/src/lib.rs b/crates/uv-python/src/lib.rs index 5cdb3a4a41aa5..7b48f19fc151f 100644 --- a/crates/uv-python/src/lib.rs +++ b/crates/uv-python/src/lib.rs @@ -5,7 +5,7 @@ pub use crate::discovery::{ find_python_installations, EnvironmentPreference, Error as DiscoveryError, PythonDownloads, PythonNotFound, PythonPreference, PythonRequest, PythonSource, VersionRequest, }; -pub use crate::environment::PythonEnvironment; +pub use crate::environment::{InvalidEnvironment, InvalidEnvironmentKind, PythonEnvironment}; pub use crate::implementation::ImplementationName; pub use crate::installation::{PythonInstallation, PythonInstallationKey}; pub use crate::interpreter::{Error as InterpreterError, Interpreter}; diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 5324ac9e393ea..b7d40e0f41e07 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -19,8 +19,8 @@ use uv_fs::Simplified; use uv_installer::{SatisfiesResult, SitePackages}; use uv_normalize::PackageName; use uv_python::{ - EnvironmentPreference, Interpreter, PythonDownloads, PythonEnvironment, PythonInstallation, - PythonPreference, PythonRequest, PythonVersionFile, VersionRequest, + EnvironmentPreference, Interpreter, InvalidEnvironmentKind, PythonDownloads, PythonEnvironment, + PythonInstallation, PythonPreference, PythonRequest, PythonVersionFile, VersionRequest, }; use uv_requirements::{ NamedRequirementsError, NamedRequirementsResolver, RequirementsSpecification, @@ -120,8 +120,8 @@ pub(crate) enum ProjectError { #[error("Environment marker is empty")] EmptyEnvironment, - #[error("Project virtual environment directory `{0}` cannot be used because it is not a virtual environment and is non-empty")] - InvalidProjectEnvironmentDir(PathBuf), + #[error("Project virtual environment directory `{0}` cannot be used because {1}")] + InvalidProjectEnvironmentDir(PathBuf, String), #[error("Failed to parse `pyproject.toml`")] TomlParse(#[source] toml::de::Error), @@ -393,12 +393,26 @@ impl FoundInterpreter { } } Err(uv_python::Error::MissingEnvironment(_)) => {} - Err(uv_python::Error::InvalidEnvironment(_)) => { + Err(uv_python::Error::InvalidEnvironment(inner)) => { // If there's an invalid environment with existing content, we error instead of - // deleting it later on. - if fs_err::read_dir(&venv).is_ok_and(|mut dir| dir.next().is_some()) { - return Err(ProjectError::InvalidProjectEnvironmentDir(venv)); - } + // deleting it later on + match inner.kind { + InvalidEnvironmentKind::NotDirectory => { + return Err(ProjectError::InvalidProjectEnvironmentDir( + venv, + inner.kind.to_string(), + )) + } + InvalidEnvironmentKind::MissingExecutable(_) => { + if fs_err::read_dir(&venv).is_ok_and(|mut dir| dir.next().is_some()) { + return Err(ProjectError::InvalidProjectEnvironmentDir( + venv, + "because it is not a valid Python environment (no Python executable was found)" + .to_string(), + )); + } + } + }; } Err(uv_python::Error::Query(uv_python::InterpreterError::NotFound(path))) => { if path.is_symlink() { @@ -408,11 +422,6 @@ impl FoundInterpreter { path.user_display().cyan(), target_path.user_display().cyan(), ); - } else { - warn_user!( - "Ignoring existing virtual environment with missing Python interpreter: {}", - path.user_display().cyan() - ); } } Err(err) => return Err(err.into()), diff --git a/crates/uv/tests/sync.rs b/crates/uv/tests/sync.rs index 95ec51743a6c1..d68ca1ec14462 100644 --- a/crates/uv/tests/sync.rs +++ b/crates/uv/tests/sync.rs @@ -2640,7 +2640,7 @@ fn sync_invalid_environment() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: Project virtual environment directory `[VENV]/` cannot be used because it is not a virtual environment and is non-empty + error: Project virtual environment directory `[VENV]/` cannot be used because because it is not a valid Python environment (no Python executable was found) "###); // But if it's just an incompatible virtual environment... @@ -2677,10 +2677,11 @@ fn sync_invalid_environment() -> Result<()> { let bin = venv_bin_path(context.temp_dir.join(".venv")); - // If it's there's a broken symlink, we should warn + // If there's just a broken symlink, we should warn #[cfg(unix)] { fs_err::remove_file(bin.join("python"))?; + fs_err::os::unix::fs::symlink(context.temp_dir.join("does-not-exist"), bin.join("python"))?; uv_snapshot!(context.filters(), context.sync(), @r###" success: true exit_code: 0 @@ -2697,21 +2698,15 @@ fn sync_invalid_environment() -> Result<()> { "###); } - // And if the Python executable is missing entirely we should warn + // But if the Python executable is missing entirely we should also fail fs_err::remove_dir_all(&bin)?; uv_snapshot!(context.filters(), context.sync(), @r###" - success: true - exit_code: 0 + success: false + exit_code: 2 ----- stdout ----- ----- stderr ----- - warning: Ignoring existing virtual environment with missing Python interpreter: .venv/[BIN]/python - Using Python 3.12.[X] interpreter at: [PYTHON-3.12] - Removed virtual environment at: .venv - Creating virtual environment at: .venv - Resolved 2 packages in [TIME] - Installed 1 package in [TIME] - + iniconfig==2.0.0 + error: Project virtual environment directory `[VENV]/` cannot be used because because it is not a valid Python environment (no Python executable was found) "###); Ok(())