Use specialized error message for invalid Python install / uninstall requests (#5171)

## Summary

Closes https://github.com/astral-sh/uv/issues/4819.
This commit is contained in:
Charlie Marsh 2024-07-17 20:47:55 -04:00 committed by GitHub
parent 622e9e8799
commit 91bf213641
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 27 additions and 24 deletions

View file

@ -68,8 +68,6 @@ pub enum Error {
NameError(String), NameError(String),
#[error("Failed to parse request part")] #[error("Failed to parse request part")]
InvalidRequestPlatform(#[from] platform::Error), InvalidRequestPlatform(#[from] platform::Error),
#[error("Cannot download managed Python for request: {0}")]
InvalidRequestKind(PythonRequest),
// TODO(zanieb): Implement display for `PythonDownloadRequest` // TODO(zanieb): Implement display for `PythonDownloadRequest`
#[error("No download found for request: {0:?}")] #[error("No download found for request: {0:?}")]
NoDownloadFound(PythonDownloadRequest), NoDownloadFound(PythonDownloadRequest),
@ -142,26 +140,23 @@ impl PythonDownloadRequest {
/// ///
/// Returns [`None`] if the request kind is not compatible with a download, e.g., it is /// Returns [`None`] if the request kind is not compatible with a download, e.g., it is
/// a request for a specific directory or executable name. /// a request for a specific directory or executable name.
pub fn try_from_request(request: &PythonRequest) -> Option<Self> { pub fn from_request(request: &PythonRequest) -> Option<Self> {
Self::from_request(request).ok()
}
/// Construct a new [`PythonDownloadRequest`] from a [`PythonRequest`].
pub fn from_request(request: &PythonRequest) -> Result<Self, Error> {
match request { match request {
PythonRequest::Version(version) => Ok(Self::default().with_version(version.clone())), PythonRequest::Version(version) => Some(Self::default().with_version(version.clone())),
PythonRequest::Implementation(implementation) => { PythonRequest::Implementation(implementation) => {
Ok(Self::default().with_implementation(*implementation)) Some(Self::default().with_implementation(*implementation))
} }
PythonRequest::ImplementationVersion(implementation, version) => Ok(Self::default() PythonRequest::ImplementationVersion(implementation, version) => Some(
.with_implementation(*implementation) Self::default()
.with_version(version.clone())), .with_implementation(*implementation)
PythonRequest::Key(request) => Ok(request.clone()), .with_version(version.clone()),
PythonRequest::Any => Ok(Self::default()), ),
PythonRequest::Key(request) => Some(request.clone()),
PythonRequest::Any => Some(Self::default()),
// We can't download a managed installation for these request kinds // We can't download a managed installation for these request kinds
PythonRequest::Directory(_) PythonRequest::Directory(_)
| PythonRequest::ExecutableName(_) | PythonRequest::ExecutableName(_)
| PythonRequest::File(_) => Err(Error::InvalidRequestKind(request.clone())), | PythonRequest::File(_) => None,
} }
} }

View file

@ -90,7 +90,7 @@ impl PythonInstallation {
// Perform a fetch aggressively if managed Python is preferred // Perform a fetch aggressively if managed Python is preferred
if matches!(preference, PythonPreference::Managed) && python_fetch.is_automatic() { if matches!(preference, PythonPreference::Managed) && python_fetch.is_automatic() {
if let Some(request) = PythonDownloadRequest::try_from_request(&request) { if let Some(request) = PythonDownloadRequest::from_request(&request) {
return Self::fetch(request.fill(), client_builder, cache, reporter).await; return Self::fetch(request.fill(), client_builder, cache, reporter).await;
} }
} }
@ -104,7 +104,7 @@ impl PythonInstallation {
&& python_fetch.is_automatic() && python_fetch.is_automatic()
&& client_builder.connectivity.is_online() => && client_builder.connectivity.is_online() =>
{ {
if let Some(request) = PythonDownloadRequest::try_from_request(&request) { if let Some(request) = PythonDownloadRequest::from_request(&request) {
debug!("Requested Python not found, checking for available download..."); debug!("Requested Python not found, checking for available download...");
match Self::fetch(request.fill(), client_builder, cache, reporter).await { match Self::fetch(request.fill(), client_builder, cache, reporter).await {
Ok(installation) => Ok(installation), Ok(installation) => Ok(installation),

View file

@ -12,7 +12,7 @@ use uv_cache::Cache;
use uv_client::Connectivity; use uv_client::Connectivity;
use uv_configuration::PreviewMode; use uv_configuration::PreviewMode;
use uv_fs::Simplified; use uv_fs::Simplified;
use uv_python::downloads::{self, DownloadResult, ManagedPythonDownload, PythonDownloadRequest}; use uv_python::downloads::{DownloadResult, ManagedPythonDownload, PythonDownloadRequest};
use uv_python::managed::{ManagedPythonInstallation, ManagedPythonInstallations}; use uv_python::managed::{ManagedPythonInstallation, ManagedPythonInstallations};
use uv_python::{ use uv_python::{
requests_from_version_file, PythonRequest, PYTHON_VERSIONS_FILENAME, PYTHON_VERSION_FILENAME, requests_from_version_file, PythonRequest, PYTHON_VERSIONS_FILENAME, PYTHON_VERSION_FILENAME,
@ -67,8 +67,12 @@ pub(crate) async fn install(
let download_requests = requests let download_requests = requests
.iter() .iter()
.map(PythonDownloadRequest::from_request) .map(|request| {
.collect::<Result<Vec<_>, downloads::Error>>()?; PythonDownloadRequest::from_request(request).ok_or_else(|| {
anyhow::anyhow!("Cannot download managed Python for request: {request}")
})
})
.collect::<Result<Vec<_>>>()?;
let installed_installations: Vec<_> = installations.find_all()?.collect(); let installed_installations: Vec<_> = installations.find_all()?.collect();
let mut unfilled_requests = Vec::new(); let mut unfilled_requests = Vec::new();

View file

@ -7,7 +7,7 @@ use itertools::Itertools;
use owo_colors::OwoColorize; use owo_colors::OwoColorize;
use uv_configuration::PreviewMode; use uv_configuration::PreviewMode;
use uv_python::downloads::{self, PythonDownloadRequest}; use uv_python::downloads::PythonDownloadRequest;
use uv_python::managed::ManagedPythonInstallations; use uv_python::managed::ManagedPythonInstallations;
use uv_python::PythonRequest; use uv_python::PythonRequest;
use uv_warnings::warn_user_once; use uv_warnings::warn_user_once;
@ -43,8 +43,12 @@ pub(crate) async fn uninstall(
let download_requests = requests let download_requests = requests
.iter() .iter()
.map(PythonDownloadRequest::from_request) .map(|request| {
.collect::<Result<Vec<_>, downloads::Error>>()?; PythonDownloadRequest::from_request(request).ok_or_else(|| {
anyhow::anyhow!("Cannot uninstall managed Python for request: {request}")
})
})
.collect::<Result<Vec<_>>>()?;
let installed_installations: Vec<_> = installations.find_all()?.collect(); let installed_installations: Vec<_> = installations.find_all()?.collect();
let mut matching_installations = BTreeSet::default(); let mut matching_installations = BTreeSet::default();