From f29c991e21c3b28c8f7724e7d454433920a53777 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 22 Apr 2024 13:57:36 +0200 Subject: [PATCH] Dedicated error type for direct url parsing (#3181) Add a dedicated error type for direct url parsing. This change is broken out from the new uv requirement type, which uses direct url parsing internally. --- Cargo.lock | 1 + crates/distribution-types/Cargo.toml | 1 + crates/distribution-types/src/cached.rs | 2 +- crates/distribution-types/src/direct_url.rs | 42 ++++++++++++++------- crates/uv-distribution/src/error.rs | 3 ++ crates/uv-distribution/src/git.rs | 4 +- crates/uv-git/src/lib.rs | 2 +- 7 files changed, 38 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ec39c63e..37da12009 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1049,6 +1049,7 @@ dependencies = [ "cache-key", "distribution-filename", "fs-err", + "git2", "itertools 0.12.1", "once_cell", "pep440_rs", diff --git a/crates/distribution-types/Cargo.toml b/crates/distribution-types/Cargo.toml index 66602434f..67a76750b 100644 --- a/crates/distribution-types/Cargo.toml +++ b/crates/distribution-types/Cargo.toml @@ -25,6 +25,7 @@ uv-normalize = { workspace = true } anyhow = { workspace = true } fs-err = { workspace = true } +git2 = { workspace = true } itertools = { workspace = true } once_cell = { workspace = true } rkyv = { workspace = true } diff --git a/crates/distribution-types/src/cached.rs b/crates/distribution-types/src/cached.rs index b7ddf5155..cf7840d77 100644 --- a/crates/distribution-types/src/cached.rs +++ b/crates/distribution-types/src/cached.rs @@ -116,7 +116,7 @@ impl CachedDist { editable: dist.editable, }))) } else { - DirectUrl::try_from(dist.url.raw()).map(Some) + Ok(Some(DirectUrl::try_from(dist.url.raw())?)) } } } diff --git a/crates/distribution-types/src/direct_url.rs b/crates/distribution-types/src/direct_url.rs index 297f5d090..a3153b1c6 100644 --- a/crates/distribution-types/src/direct_url.rs +++ b/crates/distribution-types/src/direct_url.rs @@ -1,10 +1,24 @@ +use anyhow::{Error, Result}; use std::path::PathBuf; - -use anyhow::{Context, Error, Result}; +use thiserror::Error; use url::Url; use uv_git::{GitSha, GitUrl}; +#[derive(Debug, Error)] +pub enum DirectUrlError { + #[error("Unsupported URL prefix `{prefix}` in URL: `{url}`")] + UnsupportedUrlPrefix { prefix: String, url: Url }, + #[error("Invalid path in file URL: `{0}`")] + InvalidFileUrl(Url), + #[error("Failed to parse Git reference from URL: `{0}`")] + GitShaParse(Url, #[source] git2::Error), + #[error("Not a valid URL: `{0}`")] + UrlParse(String, #[source] url::ParseError), + #[error("Missing `git+` prefix for Git URL: `{0}`")] + MissingUrlPrefix(Url), +} + #[derive(Debug)] pub enum DirectUrl { /// The direct URL is a path to a local directory or file. @@ -49,17 +63,18 @@ pub struct DirectArchiveUrl { } impl TryFrom<&Url> for DirectGitUrl { - type Error = Error; + type Error = DirectUrlError; - fn try_from(url: &Url) -> Result { - let subdirectory = get_subdirectory(url); + fn try_from(url_in: &Url) -> Result { + let subdirectory = get_subdirectory(url_in); - let url = url + let url = url_in .as_str() .strip_prefix("git+") - .context("Missing git+ prefix for Git URL")?; - let url = Url::parse(url)?; - let url = GitUrl::try_from(url)?; + .ok_or_else(|| DirectUrlError::MissingUrlPrefix(url_in.clone()))?; + let url = Url::parse(url).map_err(|err| DirectUrlError::UrlParse(url.to_string(), err))?; + let url = GitUrl::try_from(url) + .map_err(|err| DirectUrlError::GitShaParse(url_in.clone(), err))?; Ok(Self { url, subdirectory }) } } @@ -94,15 +109,16 @@ pub fn git_reference(url: &Url) -> Result, Error> { } impl TryFrom<&Url> for DirectUrl { - type Error = Error; + type Error = DirectUrlError; fn try_from(url: &Url) -> Result { if let Some((prefix, ..)) = url.scheme().split_once('+') { match prefix { "git" => Ok(Self::Git(DirectGitUrl::try_from(url)?)), - _ => Err(Error::msg(format!( - "Unsupported URL prefix `{prefix}` in URL: {url}", - ))), + _ => Err(DirectUrlError::UnsupportedUrlPrefix { + prefix: prefix.to_string(), + url: url.clone(), + }), } } else if url.scheme().eq_ignore_ascii_case("file") { Ok(Self::LocalFile(LocalFileUrl { diff --git a/crates/uv-distribution/src/error.rs b/crates/uv-distribution/src/error.rs index 271ee66b0..5dcb4d470 100644 --- a/crates/uv-distribution/src/error.rs +++ b/crates/uv-distribution/src/error.rs @@ -3,6 +3,7 @@ use tokio::task::JoinError; use zip::result::ZipError; use distribution_filename::WheelFilenameError; +use distribution_types::DirectUrlError; use pep440_rs::Version; use pypi_types::HashDigest; use uv_client::BetterReqwestError; @@ -23,6 +24,8 @@ pub enum Error { #[error("Git operation failed")] Git(#[source] anyhow::Error), #[error(transparent)] + DirectUrl(#[from] Box), + #[error(transparent)] Reqwest(#[from] BetterReqwestError), #[error(transparent)] Client(#[from] uv_client::Error), diff --git a/crates/uv-distribution/src/git.rs b/crates/uv-distribution/src/git.rs index 5cfb63691..8391299ac 100644 --- a/crates/uv-distribution/src/git.rs +++ b/crates/uv-distribution/src/git.rs @@ -67,7 +67,7 @@ pub(crate) async fn fetch_git_archive( ) .map_err(Error::CacheWrite)?; - let DirectGitUrl { url, subdirectory } = DirectGitUrl::try_from(url).map_err(Error::Git)?; + let DirectGitUrl { url, subdirectory } = DirectGitUrl::try_from(url).map_err(Box::new)?; // Fetch the Git repository. let source = if let Some(reporter) = reporter { @@ -95,7 +95,7 @@ pub(crate) async fn resolve_precise( cache: &Cache, reporter: Option<&Arc>, ) -> Result, Error> { - let DirectGitUrl { url, subdirectory } = DirectGitUrl::try_from(url).map_err(Error::Git)?; + let DirectGitUrl { url, subdirectory } = DirectGitUrl::try_from(url).map_err(Box::new)?; // If the Git reference already contains a complete SHA, short-circuit. if url.precise().is_some() { diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs index 7198fd071..c65cc0578 100644 --- a/crates/uv-git/src/lib.rs +++ b/crates/uv-git/src/lib.rs @@ -51,7 +51,7 @@ impl GitUrl { } impl TryFrom for GitUrl { - type Error = anyhow::Error; + type Error = git2::Error; /// Initialize a [`GitUrl`] source from a URL. fn try_from(mut url: Url) -> Result {