From b543881b1b742e1bf98e6c503c2387f7955fad62 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Tue, 21 Jan 2025 15:11:55 -0600 Subject: [PATCH] Show non-critical Python discovery errors if no other interpreter is found (#10716) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, these errors would only be visible in the debug logs as "Skipping bad interpreter ..." which can lead us to making some ridiculous claims like "There is no virtual environment" or "Python is not installed" when really we just failed to query the interpreter for some reason. We show the first error, sort of arbitrarily — but I think it matches user expectation, i.e., this would be the first Python on your PATH. Related to #10713 --- crates/uv-python/src/discovery.rs | 13 +++++++++++++ crates/uv-python/src/lib.rs | 9 ++++----- crates/uv/tests/it/common/mod.rs | 2 +- crates/uv/tests/it/pip_sync.rs | 18 +++++++++++++++++- crates/uv/tests/it/python_find.rs | 8 ++++---- 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/crates/uv-python/src/discovery.rs b/crates/uv-python/src/discovery.rs index 4d5088f7e..f3de55aa4 100644 --- a/crates/uv-python/src/discovery.rs +++ b/crates/uv-python/src/discovery.rs @@ -977,9 +977,16 @@ pub(crate) fn find_python_installation( ) -> Result { let installations = find_python_installations(request, environments, preference, cache); let mut first_prerelease = None; + let mut first_error = None; for result in installations { // Iterate until the first critical error or happy result if !result.as_ref().err().map_or(true, Error::is_critical) { + // Track the first non-critical error + if first_error.is_none() { + if let Err(err) = result { + first_error = Some(err); + } + } continue; } @@ -1032,6 +1039,12 @@ pub(crate) fn find_python_installation( return Ok(Ok(installation)); } + // If we found a Python, but it was unusable for some reason, report that instead of saying we + // couldn't find any Python interpreters. + if let Some(err) = first_error { + return Err(err); + } + Ok(Err(PythonNotFound { request: request.clone(), environment_preference: environments, diff --git a/crates/uv-python/src/lib.rs b/crates/uv-python/src/lib.rs index 0eb3cb36f..114415043 100644 --- a/crates/uv-python/src/lib.rs +++ b/crates/uv-python/src/lib.rs @@ -111,7 +111,7 @@ mod tests { use crate::{ discovery::{ - find_best_python_installation, find_python_installation, EnvironmentPreference, + self, find_best_python_installation, find_python_installation, EnvironmentPreference, }, PythonPreference, }; @@ -589,11 +589,10 @@ mod tests { PythonPreference::default(), &context.cache, ) - })?; + }); assert!( - matches!(result, Err(PythonNotFound { .. })), - // TODO(zanieb): We could improve the error handling to hint this to the user - "If only Python 2 is available, we should not find a python; got {result:?}" + matches!(result, Err(discovery::Error::Query(..))), + "If only Python 2 is available, we should report the interpreter query error; got {result:?}" ); Ok(()) diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index 8247cae92..0a7ca3def 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -636,7 +636,7 @@ impl TestContext { .env(EnvVars::UV_PREVIEW, "1") .env(EnvVars::UV_PYTHON_INSTALL_DIR, "") .current_dir(&self.temp_dir); - self.add_shared_args(&mut command, true); + self.add_shared_args(&mut command, false); command } diff --git a/crates/uv/tests/it/pip_sync.rs b/crates/uv/tests/it/pip_sync.rs index a54d87721..cd58221dc 100644 --- a/crates/uv/tests/it/pip_sync.rs +++ b/crates/uv/tests/it/pip_sync.rs @@ -51,7 +51,10 @@ fn missing_requirements_txt() { #[test] fn missing_venv() -> Result<()> { - let context = TestContext::new("3.12"); + let context = TestContext::new("3.12") + .with_filtered_virtualenv_bin() + .with_filtered_python_names(); + let requirements = context.temp_dir.child("requirements.txt"); requirements.write_str("anyio")?; fs::remove_dir_all(&context.venv)?; @@ -61,6 +64,19 @@ fn missing_venv() -> Result<()> { exit_code: 2 ----- stdout ----- + ----- stderr ----- + error: Failed to inspect Python interpreter from active virtual environment at `.venv/[BIN]/python` + Caused by: Python interpreter not found at `[VENV]/[BIN]/python` + "###); + + assert!(predicates::path::missing().eval(&context.venv)); + + // If not "active", we hint to create one + uv_snapshot!(context.filters(), context.pip_sync().arg("requirements.txt").env_remove("VIRTUAL_ENV"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + ----- stderr ----- error: No virtual environment found; run `uv venv` to create an environment, or pass `--system` to install into a non-virtual environment "###); diff --git a/crates/uv/tests/it/python_find.rs b/crates/uv/tests/it/python_find.rs index 4eeb73be7..87fbb42d1 100644 --- a/crates/uv/tests/it/python_find.rs +++ b/crates/uv/tests/it/python_find.rs @@ -569,7 +569,7 @@ fn python_find_venv_invalid() { .with_filtered_virtualenv_bin(); // We find the virtual environment - uv_snapshot!(context.filters(), context.python_find(), @r###" + uv_snapshot!(context.filters(), context.python_find().env(EnvVars::VIRTUAL_ENV, context.venv.as_os_str()), @r###" success: true exit_code: 0 ----- stdout ----- @@ -581,7 +581,7 @@ fn python_find_venv_invalid() { // If the binaries are missing from a virtual environment, we fail fs_err::remove_dir_all(venv_bin_path(&context.venv)).unwrap(); - uv_snapshot!(context.filters(), context.python_find(), @r###" + uv_snapshot!(context.filters(), context.python_find().env(EnvVars::VIRTUAL_ENV, context.venv.as_os_str()), @r###" success: false exit_code: 2 ----- stdout ----- @@ -592,7 +592,7 @@ fn python_find_venv_invalid() { "###); // Unless the virtual environment is not active - uv_snapshot!(context.filters(), context.python_find().env_remove(EnvVars::VIRTUAL_ENV), @r###" + uv_snapshot!(context.filters(), context.python_find(), @r###" success: true exit_code: 0 ----- stdout ----- @@ -604,7 +604,7 @@ fn python_find_venv_invalid() { // If there's not a `pyvenv.cfg` file, it's also non-fatal, we ignore the environment fs_err::remove_file(context.venv.join("pyvenv.cfg")).unwrap(); - uv_snapshot!(context.filters(), context.python_find(), @r###" + uv_snapshot!(context.filters(), context.python_find().env(EnvVars::VIRTUAL_ENV, context.venv.as_os_str()), @r###" success: true exit_code: 0 ----- stdout -----