Skip to content

Commit

Permalink
Improve toolchain and environment missing error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Jun 27, 2024
1 parent 7c3ad62 commit c45acd8
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 193 deletions.
307 changes: 170 additions & 137 deletions crates/uv-toolchain/src/discovery.rs

Large diffs are not rendered by default.

63 changes: 56 additions & 7 deletions crates/uv-toolchain/src/environment.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::borrow::Cow;
use std::env;
use std::fmt;
use std::path::{Path, PathBuf};
use std::sync::Arc;

Expand All @@ -10,8 +11,8 @@ use crate::discovery::find_toolchain;
use crate::toolchain::Toolchain;
use crate::virtualenv::{virtualenv_python_executable, PyVenvConfiguration};
use crate::{
EnvironmentPreference, Error, Interpreter, Prefix, Target, ToolchainPreference,
ToolchainRequest,
EnvironmentPreference, Error, Interpreter, Prefix, Target, ToolchainNotFound,
ToolchainPreference, ToolchainRequest,
};

/// A Python environment, consisting of a Python [`Interpreter`] and its associated paths.
Expand All @@ -24,6 +25,50 @@ struct PythonEnvironmentShared {
interpreter: Interpreter,
}

/// The result of failed environment discovery.
///
/// Generally this is cast from [`ToolchainNotFound`] by [`PythonEnvironment::find`].
#[derive(Clone, Debug, Error)]
pub struct EnvironmentNotFound {
request: ToolchainRequest,
preference: EnvironmentPreference,
}

impl From<ToolchainNotFound> for EnvironmentNotFound {
fn from(value: ToolchainNotFound) -> Self {
Self {
request: value.request,
preference: value.environment_preference,
}
}
}

impl fmt::Display for EnvironmentNotFound {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let environment = match self.preference {
EnvironmentPreference::Any => "virtual or system environment",
EnvironmentPreference::ExplicitSystem => {
if self.request.is_explicit_system() {
"virtual or system environment"
} else {
// TODO(zanieb): We could add a hint to use the `--system` flag here
"virtual environment"
}
}
EnvironmentPreference::OnlySystem => "system environment",
EnvironmentPreference::OnlyVirtual => "virtual environment",
};
match self.request {
ToolchainRequest::Any => {
write!(f, "No {environment} found")
}
_ => {
write!(f, "No {environment} found for {}", self.request)
}
}
}
}

impl PythonEnvironment {
/// Find a [`PythonEnvironment`] matching the given request and preference.
///
Expand All @@ -34,13 +79,16 @@ impl PythonEnvironment {
preference: EnvironmentPreference,
cache: &Cache,
) -> Result<Self, Error> {
let toolchain = find_toolchain(
let toolchain = match find_toolchain(
request,
preference,
// Ignore managed toolchains when looking for environments
ToolchainPreference::OnlySystem,
cache,
)??;
)? {
Ok(toolchain) => toolchain,
Err(err) => return Err(EnvironmentNotFound::from(err).into()),
};
Ok(Self::from_toolchain(toolchain))
}

Expand All @@ -49,9 +97,10 @@ impl PythonEnvironment {
let venv = match fs_err::canonicalize(root.as_ref()) {
Ok(venv) => venv,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return Err(Error::NotFound(
crate::ToolchainNotFound::DirectoryNotFound(root.as_ref().to_path_buf()),
));
return Err(Error::MissingEnvironment(EnvironmentNotFound {
preference: EnvironmentPreference::Any,
request: ToolchainRequest::Directory(root.as_ref().to_owned()),
}));
}
Err(err) => return Err(Error::Discovery(err.into())),
};
Expand Down
58 changes: 18 additions & 40 deletions crates/uv-toolchain/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,10 @@ pub enum Error {
KeyError(#[from] toolchain::ToolchainKeyError),

#[error(transparent)]
NotFound(#[from] ToolchainNotFound),
MissingToolchain(#[from] ToolchainNotFound),

#[error(transparent)]
MissingEnvironment(#[from] environment::EnvironmentNotFound),
}

// The mock interpreters are not valid on Windows so we don't have unit test coverage there
Expand Down Expand Up @@ -99,7 +102,7 @@ mod tests {
use crate::{
implementation::ImplementationName, managed::InstalledToolchains, toolchain::Toolchain,
virtualenv::virtualenv_python_executable, PythonVersion, ToolchainNotFound,
ToolchainRequest, ToolchainSource, VersionRequest,
ToolchainRequest, ToolchainSource,
};

struct TestContext {
Expand Down Expand Up @@ -410,7 +413,7 @@ mod tests {
)
});
assert!(
matches!(result, Ok(Err(ToolchainNotFound::NoPythonInstallation(..)))),
matches!(result, Ok(Err(ToolchainNotFound { .. }))),
"With an empty path, no Python installation should be detected got {result:?}"
);

Expand All @@ -424,7 +427,7 @@ mod tests {
)
});
assert!(
matches!(result, Ok(Err(ToolchainNotFound::NoPythonInstallation(..)))),
matches!(result, Ok(Err(ToolchainNotFound { .. }))),
"With an unset path, no Python installation should be detected got {result:?}"
);

Expand All @@ -450,7 +453,7 @@ mod tests {
assert!(
matches!(
result,
Ok(Err(ToolchainNotFound::NoPythonInstallation(..)))
Ok(Err(ToolchainNotFound { .. }))
),
"With an non-executable Python, no Python installation should be detected; got {result:?}"
);
Expand Down Expand Up @@ -563,7 +566,7 @@ mod tests {
)
})?;
assert!(
matches!(result, Err(ToolchainNotFound::NoPythonInstallation(..))),
matches!(result, Err(ToolchainNotFound { .. })),
// 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 toolchain; got {result:?}"
);
Expand Down Expand Up @@ -789,13 +792,7 @@ mod tests {
)
})?;
assert!(
matches!(
result,
Err(ToolchainNotFound::NoMatchingVersion(
_,
VersionRequest::MajorMinor(3, 9)
))
),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find a toolchain; got {result:?}"
);

Expand All @@ -816,13 +813,7 @@ mod tests {
)
})?;
assert!(
matches!(
result,
Err(ToolchainNotFound::NoMatchingVersion(
_,
VersionRequest::MajorMinorPatch(3, 11, 9)
))
),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find a toolchain; got {result:?}"
);

Expand Down Expand Up @@ -1298,14 +1289,7 @@ mod tests {
)
})?;
assert!(
matches!(
result,
Err(ToolchainNotFound::NoPythonInstallation(
// TODO(zanieb): We need the environment preference in the error
ToolchainPreference::OnlySystem,
None
))
),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find an toolchain; got {result:?}"
);

Expand All @@ -1322,13 +1306,7 @@ mod tests {
},
)?;
assert!(
matches!(
result,
Err(ToolchainNotFound::NoMatchingVersion(
ToolchainPreference::OnlySystem,
VersionRequest::MajorMinorPatch(3, 12, 3)
))
),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find an toolchain; got {result:?}"
);
Ok(())
Expand Down Expand Up @@ -1362,7 +1340,7 @@ mod tests {
)
})?;
assert!(
matches!(result, Err(ToolchainNotFound::NoPythonInstallation(..))),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find it without a specific request"
);

Expand All @@ -1375,7 +1353,7 @@ mod tests {
)
})?;
assert!(
matches!(result, Err(ToolchainNotFound::NoMatchingVersion(..))),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find it via a matching version request"
);

Expand Down Expand Up @@ -1569,7 +1547,7 @@ mod tests {
assert_eq!(
toolchain.interpreter().python_full_version().to_string(),
"3.10.0",
"We should prefer the requested directory over the system and active virtul toolchains"
"We should prefer the requested directory over the system and active virtual environments"
);

Ok(())
Expand All @@ -1589,7 +1567,7 @@ mod tests {
)
})?;
assert!(
matches!(result, Err(ToolchainNotFound::FileNotFound(_))),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not find the file; got {result:?}"
);

Expand Down Expand Up @@ -1639,7 +1617,7 @@ mod tests {
)
})?;
assert!(
matches!(result, Err(ToolchainNotFound::NoPythonInstallation(..))),
matches!(result, Err(ToolchainNotFound { .. })),
"We should not the pypy interpreter if not named `python` or requested; got {result:?}"
);

Expand Down
2 changes: 1 addition & 1 deletion crates/uv-toolchain/src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl Toolchain {
// Perform a find first
match Self::find(&request, environments, preference, cache) {
Ok(venv) => Ok(venv),
Err(Error::NotFound(_))
Err(Error::MissingToolchain(_))
if preference.allows_managed() && client_builder.connectivity.is_online() =>
{
debug!("Requested Python not found, checking for available download...");
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub(crate) async fn find_interpreter(
return Ok(venv.into_interpreter());
}
}
Err(uv_toolchain::Error::NotFound(_)) => {}
Err(uv_toolchain::Error::MissingEnvironment(_)) => {}
Err(e) => return Err(e.into()),
};

Expand Down Expand Up @@ -245,7 +245,7 @@ pub(crate) async fn init_environment(
fs_err::remove_dir_all(venv.root())
.context("Failed to remove existing virtual environment")?;
}
Err(uv_toolchain::Error::NotFound(_)) => {}
Err(uv_toolchain::Error::MissingEnvironment(_)) => {}
Err(e) => return Err(e.into()),
};

Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2005,7 +2005,7 @@ fn lock_requires_python() -> Result<()> {
----- stderr -----
warning: `uv sync` is experimental and may change without warning.
Removing virtual environment at: .venv
error: No interpreter found for Python >=3.12 in system toolchains
error: No toolchain found for Python >=3.12 in system path
"###);

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/tests/pip_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ fn missing_venv() -> Result<()> {
----- stdout -----
----- stderr -----
error: No Python interpreters found in system toolchains
error: No virtual environment found
"###);

assert!(predicates::path::missing().eval(&context.venv));
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/tests/toolchain_find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ fn toolchain_find() {
----- stdout -----
----- stderr -----
error: No Python interpreters found in system toolchains
error: No toolchain found in system path
"###);

// We find the first interpreter on the path
Expand Down Expand Up @@ -101,7 +101,7 @@ fn toolchain_find() {
----- stdout -----
----- stderr -----
error: No interpreter found for PyPy in system toolchains
error: No toolchain found for PyPy in system path
"###);

// Swap the order of the Python versions
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/tests/venv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ fn create_venv_unknown_python_minor() {
----- stdout -----
----- stderr -----
× No interpreter found for Python 3.100 in system toolchains
× No toolchain found for Python 3.100 in system path
"###
);
}
Expand Down Expand Up @@ -312,7 +312,7 @@ fn create_venv_unknown_python_patch() {
----- stdout -----
----- stderr -----
× No interpreter found for Python 3.12.100 in system toolchains
× No toolchain found for Python 3.12.100 in system path
"###
);
}
Expand Down

0 comments on commit c45acd8

Please sign in to comment.