Actually attach fork urls

astral-sh · Jun 25, 2024 · d21ab02 · d21ab02
1 parent 6815c52
commit d21ab02
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 11 deletions.
diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs
@@ -164,8 +164,16 @@ fn collapse_proxies(
     }
 }
 
-impl From<pubgrub::error::PubGrubError<UvDependencyProvider>> for ResolveError {
-    fn from(value: pubgrub::error::PubGrubError<UvDependencyProvider>) -> Self {
+impl ResolveError {
+    /// Convert an error from PubGrub to a resolver error.
+    ///
+    /// [`ForkUrls`] breaks the usual pattern used here since it's part of one the [`SolveState`],
+    /// not of the [`ResolverState`], so we have to take it from the fork that errored instead of
+    /// being able to add it later.
+    pub(crate) fn from_pubgrub_error(
+        value: pubgrub::error::PubGrubError<UvDependencyProvider>,
+        fork_urls: ForkUrls,
+    ) -> Self {
         match value {
             // These are all never type variant that can never match, but never is experimental
             pubgrub::error::PubGrubError::ErrorChoosingPackageVersion(_)
@@ -186,7 +194,7 @@ impl From<pubgrub::error::PubGrubError<UvDependencyProvider>> for ResolveError {
                     index_locations: None,
                     unavailable_packages: FxHashMap::default(),
                     incomplete_packages: FxHashMap::default(),
-                    fork_urls: ForkUrls::default(),
+                    fork_urls,
                 })
             }
             pubgrub::error::PubGrubError::SelfDependency { package, version } => {

diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs
@@ -344,7 +344,12 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
         'FORK: while let Some(mut state) = forked_states.pop() {
             loop {
                 // Run unit propagation.
-                state.pubgrub.unit_propagation(state.next.clone())?;
+                state
+                    .pubgrub
+                    .unit_propagation(state.next.clone())
+                    .map_err(|err| {
+                        ResolveError::from_pubgrub_error(err, state.fork_urls.clone())
+                    })?;
 
                 // Pre-visit all candidate packages, to allow metadata to be fetched in parallel. If
                 // the dependency mode is direct, we only need to visit the root package.
@@ -391,8 +396,11 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
                     .partial_solution
                     .term_intersection_for_package(&state.next)
                     .ok_or_else(|| {
-                        PubGrubError::Failure(
-                            "a package was chosen but we don't have a term.".into(),
+                        ResolveError::from_pubgrub_error(
+                            PubGrubError::Failure(
+                                "a package was chosen but we don't have a term.".into(),
+                            ),
+                            state.fork_urls.clone(),
                         )
                     })?;
                 let decision = self.choose_version(
@@ -1683,11 +1691,13 @@ impl SolveState {
             if enabled!(Level::DEBUG) {
                 prefetcher.log_tried_versions();
             }
-            return Err(PubGrubError::SelfDependency {
-                package: self.next.clone(),
-                version: version.clone(),
-            }
-            .into());
+            return Err(ResolveError::from_pubgrub_error(
+                PubGrubError::SelfDependency {
+                    package: self.next.clone(),
+                    version: version.clone(),
+                },
+                self.fork_urls.clone(),
+            ));
         }
 
         for dependency in &dependencies {