From 098944fc7df16e5de34f2bc44a15c7dc71208fe5 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 6 May 2024 13:28:05 +0200 Subject: [PATCH] Improve non-git error message (#3403) The boxing changes are due to clippy --- crates/distribution-types/src/parsed_url.rs | 24 +++++++++++++++++-- crates/distribution-types/src/requirement.rs | 2 +- .../src/specified_requirement.rs | 2 +- crates/requirements-txt/src/lib.rs | 2 +- crates/uv-build/src/lib.rs | 2 +- crates/uv-requirements/src/lookahead.rs | 2 +- crates/uv-requirements/src/pyproject.rs | 2 +- crates/uv-resolver/src/preferences.rs | 2 +- crates/uv-resolver/src/resolver/mod.rs | 6 ++--- crates/uv/src/commands/pip_compile.rs | 4 ++-- crates/uv/src/commands/pip_install.rs | 4 ++-- crates/uv/tests/pip_compile.rs | 2 +- 12 files changed, 36 insertions(+), 18 deletions(-) diff --git a/crates/distribution-types/src/parsed_url.rs b/crates/distribution-types/src/parsed_url.rs index 5ac3d4851..5da9ee3ae 100644 --- a/crates/distribution-types/src/parsed_url.rs +++ b/crates/distribution-types/src/parsed_url.rs @@ -8,8 +8,12 @@ use uv_git::{GitSha, GitUrl}; #[derive(Debug, Error)] pub enum ParsedUrlError { - #[error("Unsupported URL prefix `{prefix}` in URL: `{url}`")] - UnsupportedUrlPrefix { prefix: String, url: Url }, + #[error("Unsupported URL prefix `{prefix}` in URL: `{url}` ({message})")] + UnsupportedUrlPrefix { + prefix: String, + url: Url, + message: &'static str, + }, #[error("Invalid path in file URL: `{0}`")] InvalidFileUrl(Url), #[error("Failed to parse Git reference from URL: `{0}`")] @@ -123,9 +127,25 @@ impl TryFrom for ParsedUrl { if let Some((prefix, ..)) = url.scheme().split_once('+') { match prefix { "git" => Ok(Self::Git(ParsedGitUrl::try_from(url)?)), + "bzr" => Err(ParsedUrlError::UnsupportedUrlPrefix { + prefix: prefix.to_string(), + url: url.clone(), + message: "Bazaar is not supported", + }), + "hg" => Err(ParsedUrlError::UnsupportedUrlPrefix { + prefix: prefix.to_string(), + url: url.clone(), + message: "Mercurial is not supported", + }), + "svn" => Err(ParsedUrlError::UnsupportedUrlPrefix { + prefix: prefix.to_string(), + url: url.clone(), + message: "Subversion is not supported", + }), _ => Err(ParsedUrlError::UnsupportedUrlPrefix { prefix: prefix.to_string(), url: url.clone(), + message: "Unknown scheme", }), } } else if url.scheme().eq_ignore_ascii_case("file") { diff --git a/crates/distribution-types/src/requirement.rs b/crates/distribution-types/src/requirement.rs index bc883837f..efbb94618 100644 --- a/crates/distribution-types/src/requirement.rs +++ b/crates/distribution-types/src/requirement.rs @@ -41,7 +41,7 @@ impl Requirement { } /// Convert a [`pep508_rs::Requirement`] to a [`Requirement`]. - pub fn from_pep508(requirement: pep508_rs::Requirement) -> Result { + pub fn from_pep508(requirement: pep508_rs::Requirement) -> Result> { let source = match requirement.version_or_url { None => RequirementSource::Registry { specifier: VersionSpecifiers::empty(), diff --git a/crates/distribution-types/src/specified_requirement.rs b/crates/distribution-types/src/specified_requirement.rs index b7baba012..6d0ea8b27 100644 --- a/crates/distribution-types/src/specified_requirement.rs +++ b/crates/distribution-types/src/specified_requirement.rs @@ -59,7 +59,7 @@ impl UnresolvedRequirement { } /// Return the version specifier or URL for the requirement. - pub fn source(&self) -> Result, ParsedUrlError> { + pub fn source(&self) -> Result, Box> { // TODO(konsti): This is a bad place to raise errors, we should have parsed the url earlier. match self { Self::Named(requirement) => Ok(Cow::Borrowed(&requirement.source)), diff --git a/crates/requirements-txt/src/lib.rs b/crates/requirements-txt/src/lib.rs index f2c13c546..da68cd5fb 100644 --- a/crates/requirements-txt/src/lib.rs +++ b/crates/requirements-txt/src/lib.rs @@ -311,7 +311,7 @@ pub struct RequirementEntry { // `UnresolvedRequirementSpecification` is defined in `distribution-types` and `requirements-txt` // depends on `distribution-types`. impl TryFrom for UnresolvedRequirementSpecification { - type Error = ParsedUrlError; + type Error = Box; fn try_from(value: RequirementEntry) -> Result { Ok(Self { diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index ee5810980..488bc23fe 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -105,7 +105,7 @@ pub enum Error { #[error("Failed to build PATH for build script")] BuildScriptPath(#[source] env::JoinPathsError), #[error("Failed to parse requirements from build backend")] - DirectUrl(#[source] ParsedUrlError), + DirectUrl(#[source] Box), } #[derive(Debug)] diff --git a/crates/uv-requirements/src/lookahead.rs b/crates/uv-requirements/src/lookahead.rs index 10af862d9..49cb86ae3 100644 --- a/crates/uv-requirements/src/lookahead.rs +++ b/crates/uv-requirements/src/lookahead.rs @@ -26,7 +26,7 @@ pub enum LookaheadError { #[error(transparent)] UnsupportedUrl(#[from] distribution_types::Error), #[error(transparent)] - InvalidRequirement(#[from] distribution_types::ParsedUrlError), + InvalidRequirement(#[from] Box), } /// A resolver for resolving lookahead requirements from direct URLs. diff --git a/crates/uv-requirements/src/pyproject.rs b/crates/uv-requirements/src/pyproject.rs index 7f314a660..da247eb5c 100644 --- a/crates/uv-requirements/src/pyproject.rs +++ b/crates/uv-requirements/src/pyproject.rs @@ -407,7 +407,7 @@ pub(crate) fn lower_requirement( return if requirement.version_or_url.is_none() && &requirement.name != project_name { Err(LoweringError::UnconstrainedVersion) } else { - Ok(Requirement::from_pep508(requirement).map_err(Box::new)?) + Ok(Requirement::from_pep508(requirement)?) }; }; diff --git a/crates/uv-resolver/src/preferences.rs b/crates/uv-resolver/src/preferences.rs index ffa8cdae2..97664a5db 100644 --- a/crates/uv-resolver/src/preferences.rs +++ b/crates/uv-resolver/src/preferences.rs @@ -33,7 +33,7 @@ impl Preference { Ok(Self { requirement: match entry.requirement { RequirementsTxtRequirement::Named(requirement) => { - Requirement::from_pep508(requirement).map_err(Box::new)? + Requirement::from_pep508(requirement)? } RequirementsTxtRequirement::Unnamed(requirement) => { return Err(PreferenceError::Bare(requirement)); diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 38e9f3338..f779cee74 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -892,8 +892,7 @@ impl< .iter() .cloned() .map(Requirement::from_pep508) - .collect::>() - .map_err(Box::new)?; + .collect::>()?; let constraints = PubGrubDependencies::from_requirements( &requirements, &self.constraints, @@ -1008,8 +1007,7 @@ impl< .iter() .cloned() .map(Requirement::from_pep508) - .collect::>() - .map_err(Box::new)?; + .collect::>()?; let constraints = PubGrubDependencies::from_requirements( &requirements, &self.constraints, diff --git a/crates/uv/src/commands/pip_compile.rs b/crates/uv/src/commands/pip_compile.rs index 594940c66..b2043a885 100644 --- a/crates/uv/src/commands/pip_compile.rs +++ b/crates/uv/src/commands/pip_compile.rs @@ -385,10 +385,10 @@ pub(crate) async fn pip_compile( .iter() .cloned() .map(Requirement::from_pep508) - .collect::>()?, + .collect::>()?, optional_dependencies: IndexMap::default(), }; - Ok::<_, ParsedUrlError>(( + Ok::<_, Box>(( built_editable.editable, built_editable.metadata, requirements, diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index 7b1843dd4..612dfa86a 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -694,7 +694,7 @@ async fn resolve( .cloned() .map(Requirement::from_pep508) .collect::>()?; - Ok::<_, ParsedUrlError>(( + Ok::<_, Box>(( built_editable.editable.clone(), built_editable.metadata.clone(), Requirements { @@ -704,7 +704,7 @@ async fn resolve( )) }) .collect::>() - .map_err(|err| Error::ParsedUrl(Box::new(err)))?; + .map_err(Error::ParsedUrl)?; // Determine any lookahead requirements. let lookaheads = match options.dependency_mode { diff --git a/crates/uv/tests/pip_compile.rs b/crates/uv/tests/pip_compile.rs index 8bfa8f9cf..b12841a9b 100644 --- a/crates/uv/tests/pip_compile.rs +++ b/crates/uv/tests/pip_compile.rs @@ -5224,7 +5224,7 @@ fn unsupported_scheme() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: Unsupported URL prefix `bzr` in URL: `bzr+https://example.com/anyio` + error: Unsupported URL prefix `bzr` in URL: `bzr+https://example.com/anyio` (Bazaar is not supported) "### );