Defer reporting of build failures in resolver

astral-sh · Nov 13, 2024 · 585abc6 · 585abc6
1 parent d2b9036
commit 585abc6
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 53 deletions.
diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs
@@ -32,6 +32,9 @@ pub enum ResolveError {
     #[error(transparent)]
     Client(#[from] uv_client::Error),
 
+    #[error(transparent)]
+    Distribution(#[from] uv_distribution::Error),
+
     #[error("The channel closed unexpectedly")]
     ChannelClosed,
 
@@ -94,20 +97,20 @@ pub enum ResolveError {
     ParsedUrl(#[from] uv_pypi_types::ParsedUrlError),
 
     #[error("Failed to download `{0}`")]
-    Download(Box<BuiltDist>, #[source] uv_distribution::Error),
+    Download(Box<BuiltDist>, #[source] Arc<uv_distribution::Error>),
 
     #[error("Failed to download and build `{0}`")]
-    DownloadAndBuild(Box<SourceDist>, #[source] uv_distribution::Error),
+    DownloadAndBuild(Box<SourceDist>, #[source] Arc<uv_distribution::Error>),
 
     #[error("Failed to read `{0}`")]
-    Read(Box<BuiltDist>, #[source] uv_distribution::Error),
+    Read(Box<BuiltDist>, #[source] Arc<uv_distribution::Error>),
 
     // TODO(zanieb): Use `thiserror` in `InstalledDist` so we can avoid chaining `anyhow`
     #[error("Failed to read metadata from installed package `{0}`")]
     ReadInstalled(Box<InstalledDist>, #[source] anyhow::Error),
 
     #[error("Failed to build `{0}`")]
-    Build(Box<SourceDist>, #[source] uv_distribution::Error),
+    Build(Box<SourceDist>, #[source] Arc<uv_distribution::Error>),
 
     #[error(transparent)]
     NoSolution(#[from] NoSolutionError),

diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs
@@ -1241,7 +1241,7 @@ enum TagPolicy<'tags> {
     /// Exclusively consider wheels that match the specified platform tags.
     Required(&'tags Tags),
     /// Prefer wheels that match the specified platform tags, but fall back to incompatible wheels
-    /// if  necessary.
+    /// if necessary.
     Preferred(&'tags Tags),
 }
 

diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs
@@ -945,6 +945,32 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
             MetadataResponse::RequiresPython(..) => {
                 unreachable!("`requires-python` is only known upfront for registry distributions")
             }
+            MetadataResponse::Error(dist, err) => {
+                return Err(match &**dist {
+                    Dist::Built(built_dist @ BuiltDist::Path(_)) => {
+                        ResolveError::Read(Box::new(built_dist.clone()), (*err).clone())
+                    }
+                    Dist::Source(source_dist @ SourceDist::Path(_)) => {
+                        ResolveError::Build(Box::new(source_dist.clone()), (*err).clone())
+                    }
+                    Dist::Source(source_dist @ SourceDist::Directory(_)) => {
+                        ResolveError::Build(Box::new(source_dist.clone()), (*err).clone())
+                    }
+                    Dist::Built(built_dist) => {
+                        ResolveError::Download(Box::new(built_dist.clone()), (*err).clone())
+                    }
+                    Dist::Source(source_dist) => {
+                        if source_dist.is_local() {
+                            ResolveError::Build(Box::new(source_dist.clone()), (*err).clone())
+                        } else {
+                            ResolveError::DownloadAndBuild(
+                                Box::new(source_dist.clone()),
+                                (*err).clone(),
+                            )
+                        }
+                    }
+                });
+            }
         };
 
         let version = &metadata.version;
@@ -1331,6 +1357,35 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                             UnavailableVersion::RequiresPython(requires_python.clone()),
                         ));
                     }
+                    MetadataResponse::Error(dist, err) => {
+                        return Err(match &**dist {
+                            Dist::Built(built_dist @ BuiltDist::Path(_)) => {
+                                ResolveError::Read(Box::new(built_dist.clone()), (*err).clone())
+                            }
+                            Dist::Source(source_dist @ SourceDist::Path(_)) => {
+                                ResolveError::Build(Box::new(source_dist.clone()), (*err).clone())
+                            }
+                            Dist::Source(source_dist @ SourceDist::Directory(_)) => {
+                                ResolveError::Build(Box::new(source_dist.clone()), (*err).clone())
+                            }
+                            Dist::Built(built_dist) => {
+                                ResolveError::Download(Box::new(built_dist.clone()), (*err).clone())
+                            }
+                            Dist::Source(source_dist) => {
+                                if source_dist.is_local() {
+                                    ResolveError::Build(
+                                        Box::new(source_dist.clone()),
+                                        (*err).clone(),
+                                    )
+                                } else {
+                                    ResolveError::DownloadAndBuild(
+                                        Box::new(source_dist.clone()),
+                                        (*err).clone(),
+                                    )
+                                }
+                            }
+                        });
+                    }
                 };
 
                 if let Some(err) =
@@ -1791,28 +1846,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                 let metadata = provider
                     .get_or_build_wheel_metadata(&dist)
                     .boxed_local()
-                    .await
-                    .map_err(|err| match dist.clone() {
-                        Dist::Built(built_dist @ BuiltDist::Path(_)) => {
-                            ResolveError::Read(Box::new(built_dist), err)
-                        }
-                        Dist::Source(source_dist @ SourceDist::Path(_)) => {
-                            ResolveError::Build(Box::new(source_dist), err)
-                        }
-                        Dist::Source(source_dist @ SourceDist::Directory(_)) => {
-                            ResolveError::Build(Box::new(source_dist), err)
-                        }
-                        Dist::Built(built_dist) => {
-                            ResolveError::Download(Box::new(built_dist), err)
-                        }
-                        Dist::Source(source_dist) => {
-                            if source_dist.is_local() {
-                                ResolveError::Build(Box::new(source_dist), err)
-                            } else {
-                                ResolveError::DownloadAndBuild(Box::new(source_dist), err)
-                            }
-                        }
-                    })?;
+                    .await?;
 
                 Ok(Some(Response::Dist { dist, metadata }))
             }
@@ -1928,31 +1962,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                             let metadata = provider
                                 .get_or_build_wheel_metadata(&dist)
                                 .boxed_local()
-                                .await
-                                .map_err(|err| match dist.clone() {
-                                    Dist::Built(built_dist @ BuiltDist::Path(_)) => {
-                                        ResolveError::Read(Box::new(built_dist), err)
-                                    }
-                                    Dist::Source(source_dist @ SourceDist::Path(_)) => {
-                                        ResolveError::Build(Box::new(source_dist), err)
-                                    }
-                                    Dist::Source(source_dist @ SourceDist::Directory(_)) => {
-                                        ResolveError::Build(Box::new(source_dist), err)
-                                    }
-                                    Dist::Built(built_dist) => {
-                                        ResolveError::Download(Box::new(built_dist), err)
-                                    }
-                                    Dist::Source(source_dist) => {
-                                        if source_dist.is_local() {
-                                            ResolveError::Build(Box::new(source_dist), err)
-                                        } else {
-                                            ResolveError::DownloadAndBuild(
-                                                Box::new(source_dist),
-                                                err,
-                                            )
-                                        }
-                                    }
-                                })?;
+                                .await?;
 
                             Response::Dist { dist, metadata }
                         }

diff --git a/crates/uv-resolver/src/resolver/provider.rs b/crates/uv-resolver/src/resolver/provider.rs
@@ -1,4 +1,5 @@
 use std::future::Future;
+use std::sync::Arc;
 
 use uv_configuration::BuildOptions;
 use uv_distribution::{ArchiveMetadata, DistributionDatabase};
@@ -46,6 +47,8 @@ pub enum MetadataResponse {
     /// The source distribution has a `requires-python` requirement that is not met by the installed
     /// Python version (and static metadata is not available).
     RequiresPython(VersionSpecifiers, Version),
+    /// The distribution could not be built or downloaded.
+    Error(Box<Dist>, Arc<uv_distribution::Error>),
 }
 
 pub trait ResolverProvider {
@@ -210,7 +213,10 @@ impl<'a, Context: BuildContext> ResolverProvider for DefaultResolverProvider<'a,
                 uv_distribution::Error::RequiresPython(requires_python, version) => {
                     Ok(MetadataResponse::RequiresPython(requires_python, version))
                 }
-                err => Err(err),
+                err => Ok(MetadataResponse::Error(
+                    Box::new(dist.clone()),
+                    Arc::new(err),
+                )),
             },
         }
     }