diff --git a/crates/uv-python/src/environment.rs b/crates/uv-python/src/environment.rs index e254e3095ba1..fb181f3e0c0a 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 5cdb3a4a41aa..7b48f19fc151 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 5324ac9e393e..7447adb06c2c 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()), @@ -500,6 +509,14 @@ pub(crate) async fn get_or_init_environment( FoundInterpreter::Interpreter(interpreter) => { let venv = workspace.venv(); + // Avoid removing things that are not virtual environments + if venv.exists() && !venv.join("pyvenv.cfg").exists() { + return Err(ProjectError::InvalidProjectEnvironmentDir( + venv, + "it is not a compatible environment but cannot be recreated because it is not a virtual environment".to_string(), + )); + } + // Remove the existing virtual environment if it doesn't meet the requirements. match fs_err::remove_dir_all(&venv) { Ok(()) => { diff --git a/crates/uv/tests/sync.rs b/crates/uv/tests/sync.rs index 95ec51743a6c..13af2e6ffef5 100644 --- a/crates/uv/tests/sync.rs +++ b/crates/uv/tests/sync.rs @@ -1747,7 +1747,7 @@ fn sync_custom_environment_path() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: Project virtual environment directory `[TEMP_DIR]/foo` cannot be used because it is not a virtual environment and is non-empty + error: Project virtual environment directory `[TEMP_DIR]/foo` 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... @@ -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,22 +2698,57 @@ 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: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + 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 not a virtual environment... + fs_err::remove_dir_all(context.temp_dir.join(".venv"))?; + uv_snapshot!(context.filters(), context.venv().arg("--python").arg("3.11"), @r###" success: true exit_code: 0 ----- 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 + Using Python 3.11.[X] interpreter at: [PYTHON-3.11] Creating virtual environment at: .venv - Resolved 2 packages in [TIME] - Installed 1 package in [TIME] - + iniconfig==2.0.0 + Activate with: source .venv/[BIN]/activate "###); + // Which we detect by the presence of a `pyvenv.cfg` file + fs_err::remove_file(context.temp_dir.join(".venv").join("pyvenv.cfg"))?; + + // Let's make sure some extraneous content isn't removed + fs_err::write(context.temp_dir.join(".venv").join("file"), b"")?; + + // We should never delete it + uv_snapshot!(context.filters(), context.sync(), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON-3.12] + error: Project virtual environment directory `[VENV]/` cannot be used because it is not a compatible environment but cannot be recreated because it is not a virtual environment + "###); + + context + .temp_dir + .child(".venv") + .assert(predicate::path::is_dir()); + + context + .temp_dir + .child(".venv") + .child("file") + .assert(predicate::path::is_file()); + Ok(()) } diff --git a/crates/uv/tests/tool_list.rs b/crates/uv/tests/tool_list.rs index b8ff45c5a18f..3f0c26d90e6e 100644 --- a/crates/uv/tests/tool_list.rs +++ b/crates/uv/tests/tool_list.rs @@ -159,7 +159,7 @@ fn tool_list_bad_environment() -> Result<()> { - ruff ----- stderr ----- - Python interpreter not found at `[TEMP_DIR]/tools/black/[BIN]/python` + Invalid environment at `tools/black`: missing Python executable at `tools/black/[BIN]/python` "### );