From a286e95f4433d60c57228fce6071dc937f5d1c48 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 6 Dec 2024 13:43:14 +0100 Subject: [PATCH] Use `thiserror` in `InstalledDist` (#9676) Wanted to try something that didn't work, ended up removing that todo. --- crates/uv-distribution-types/src/installed.rs | 81 ++++++++++++++----- crates/uv-resolver/src/error.rs | 9 ++- 2 files changed, 67 insertions(+), 23 deletions(-) diff --git a/crates/uv-distribution-types/src/installed.rs b/crates/uv-distribution-types/src/installed.rs index dee8570d6..1d4cf9662 100644 --- a/crates/uv-distribution-types/src/installed.rs +++ b/crates/uv-distribution-types/src/installed.rs @@ -2,8 +2,8 @@ use std::borrow::Cow; use std::path::{Path, PathBuf}; use std::str::FromStr; -use anyhow::{anyhow, Context, Result}; use fs_err as fs; +use thiserror::Error; use tracing::warn; use url::Url; @@ -12,10 +12,51 @@ use uv_distribution_filename::EggInfoFilename; use uv_fs::Simplified; use uv_normalize::PackageName; use uv_pep440::Version; -use uv_pypi_types::DirectUrl; +use uv_pypi_types::{DirectUrl, MetadataError}; use crate::{DistributionMetadata, InstalledMetadata, InstalledVersion, Name, VersionOrUrlRef}; +#[derive(Error, Debug)] +pub enum InstalledDistError { + #[error(transparent)] + Io(#[from] std::io::Error), + + #[error(transparent)] + UrlParse(#[from] url::ParseError), + + #[error(transparent)] + Json(#[from] serde_json::Error), + + #[error(transparent)] + EggInfoParse(#[from] uv_distribution_filename::EggInfoFilenameError), + + #[error(transparent)] + VersionParse(#[from] uv_pep440::VersionParseError), + + #[error(transparent)] + PackageNameParse(#[from] uv_normalize::InvalidNameError), + + #[error("Invalid .egg-link path: `{}`", _0.user_display())] + InvalidEggLinkPath(PathBuf), + + #[error("Invalid .egg-link target: `{}`", _0.user_display())] + InvalidEggLinkTarget(PathBuf), + + #[error("Failed to parse METADATA file: `{}`", path.user_display())] + MetadataParse { + path: PathBuf, + #[source] + err: Box, + }, + + #[error("Failed to parse `PKG-INFO` file: `{}`", path.user_display())] + PkgInfoParse { + path: PathBuf, + #[source] + err: Box, + }, +} + /// A built distribution (wheel) that is installed in a virtual environment. #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub enum InstalledDist { @@ -78,7 +119,7 @@ impl InstalledDist { /// Try to parse a distribution from a `.dist-info` directory name (like `django-5.0a1.dist-info`). /// /// See: - pub fn try_from_path(path: &Path) -> Result> { + pub fn try_from_path(path: &Path) -> Result, InstalledDistError> { // Ex) `cffi-1.16.0.dist-info` if path.extension().is_some_and(|ext| ext == "dist-info") { let Some(file_stem) = path.file_stem() else { @@ -92,7 +133,7 @@ impl InstalledDist { }; let name = PackageName::from_str(name)?; - let version = Version::from_str(version).map_err(|err| anyhow!(err))?; + let version = Version::from_str(version)?; let cache_info = Self::cache_info(path)?; return if let Some(direct_url) = Self::direct_url(path)? { @@ -212,13 +253,13 @@ impl InstalledDist { // Match pip, but note setuptools only puts absolute paths in `.egg-link` files. let target = path .parent() - .ok_or_else(|| anyhow!("Invalid `.egg-link` path: {}", path.user_display()))? + .ok_or_else(|| InstalledDistError::InvalidEggLinkPath(path.to_path_buf()))? .join(target); // Normalisation comes from `pkg_resources.to_filename`. let egg_info = target.join(file_stem.replace('-', "_") + ".egg-info"); let url = Url::from_file_path(&target) - .map_err(|()| anyhow!("Invalid `.egg-link` target: {}", target.user_display()))?; + .map_err(|()| InstalledDistError::InvalidEggLinkTarget(path.to_path_buf()))?; // Mildly unfortunate that we must read metadata to get the version. let Some(egg_metadata) = read_metadata(&egg_info.join("PKG-INFO")) else { @@ -261,7 +302,7 @@ impl InstalledDist { } /// Read the `direct_url.json` file from a `.dist-info` directory. - pub fn direct_url(path: &Path) -> Result> { + pub fn direct_url(path: &Path) -> Result, InstalledDistError> { let path = path.join("direct_url.json"); let file = match fs_err::File::open(&path) { Ok(file) => file, @@ -273,7 +314,7 @@ impl InstalledDist { } /// Read the `uv_cache.json` file from a `.dist-info` directory. - pub fn cache_info(path: &Path) -> Result> { + pub fn cache_info(path: &Path) -> Result, InstalledDistError> { let path = path.join("uv_cache.json"); let file = match fs_err::File::open(&path) { Ok(file) => file, @@ -285,17 +326,17 @@ impl InstalledDist { } /// Read the `METADATA` file from a `.dist-info` directory. - pub fn metadata(&self) -> Result { + pub fn metadata(&self) -> Result { match self { Self::Registry(_) | Self::Url(_) => { let path = self.path().join("METADATA"); let contents = fs::read(&path)?; // TODO(zanieb): Update this to use thiserror so we can unpack parse errors downstream - uv_pypi_types::ResolutionMetadata::parse_metadata(&contents).with_context(|| { - format!( - "Failed to parse `METADATA` file at: {}", - path.user_display() - ) + uv_pypi_types::ResolutionMetadata::parse_metadata(&contents).map_err(|err| { + InstalledDistError::MetadataParse { + path: path.clone(), + err: Box::new(err), + } }) } Self::EggInfoFile(_) | Self::EggInfoDirectory(_) | Self::LegacyEditable(_) => { @@ -306,18 +347,18 @@ impl InstalledDist { _ => unreachable!(), }; let contents = fs::read(path.as_ref())?; - uv_pypi_types::ResolutionMetadata::parse_metadata(&contents).with_context(|| { - format!( - "Failed to parse `PKG-INFO` file at: {}", - path.user_display() - ) + uv_pypi_types::ResolutionMetadata::parse_metadata(&contents).map_err(|err| { + InstalledDistError::PkgInfoParse { + path: path.to_path_buf(), + err: Box::new(err), + } }) } } } /// Return the `INSTALLER` of the distribution. - pub fn installer(&self) -> Result> { + pub fn installer(&self) -> Result, InstalledDistError> { let path = self.path().join("INSTALLER"); match fs::read_to_string(path) { Ok(installer) => Ok(Some(installer)), diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index 5e6b8c2c9..5c84c123c 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -11,7 +11,7 @@ use tracing::trace; use uv_distribution_types::{ DerivationChain, Dist, DistErrorKind, IndexCapabilities, IndexLocations, IndexUrl, - InstalledDist, + InstalledDist, InstalledDistError, }; use uv_normalize::{ExtraName, PackageName}; use uv_pep440::{LocalVersionSlice, Version}; @@ -105,9 +105,12 @@ pub enum ResolveError { #[source] Arc, ), - // TODO(zanieb): Use `thiserror` in `InstalledDist` so we can avoid chaining `anyhow` #[error("Failed to read metadata from installed package `{0}`")] - ReadInstalled(Box, DerivationChain, #[source] anyhow::Error), + ReadInstalled( + Box, + DerivationChain, + #[source] InstalledDistError, + ), #[error(transparent)] NoSolution(#[from] NoSolutionError),