diff --git a/Cargo.lock b/Cargo.lock index ce0d8bd90..7c98ee638 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2700,7 +2700,7 @@ dependencies = [ [[package]] name = "pubgrub" version = "0.2.1" -source = "git+https://github.com/astral-sh/pubgrub?rev=b4435e2f3af10dab2336a0345b35dcd622699d06#b4435e2f3af10dab2336a0345b35dcd622699d06" +source = "git+https://github.com/astral-sh/pubgrub?rev=3f0ba760951ab0deeac874b98bb18fc90103fcf7#3f0ba760951ab0deeac874b98bb18fc90103fcf7" dependencies = [ "indexmap", "log", diff --git a/Cargo.toml b/Cargo.toml index 59167585f..53abe3725 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -107,7 +107,7 @@ path-slash = { version = "0.2.1" } pathdiff = { version = "0.2.1" } petgraph = { version = "0.6.4" } platform-info = { version = "2.0.2" } -pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "b4435e2f3af10dab2336a0345b35dcd622699d06" } +pubgrub = { git = "https://github.com/astral-sh/pubgrub", rev = "3f0ba760951ab0deeac874b98bb18fc90103fcf7" } pyo3 = { version = "0.21.0" } pyo3-log = { version = "0.10.0" } quote = { version = "1.0.36" } diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index 9be31aa2a..be3fe6530 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -2,10 +2,9 @@ use std::collections::{BTreeMap, BTreeSet}; use std::fmt::Formatter; use std::sync::Arc; -use dashmap::DashMap; use pubgrub::range::Range; use pubgrub::report::{DefaultStringReporter, DerivationTree, External, Reporter}; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashMap; use distribution_types::{BuiltDist, IndexLocations, InstalledDist, SourceDist}; use pep440_rs::Version; @@ -19,9 +18,7 @@ use crate::pubgrub::{ PubGrubPackage, PubGrubPackageInner, PubGrubReportFormatter, PubGrubSpecifierError, }; use crate::python_requirement::PythonRequirement; -use crate::resolver::{ - FxOnceMap, IncompletePackage, UnavailablePackage, UnavailableReason, VersionsResponse, -}; +use crate::resolver::{IncompletePackage, UnavailablePackage, UnavailableReason}; #[derive(Debug, thiserror::Error)] pub enum ResolveError { @@ -117,109 +114,90 @@ impl From> for ResolveError { } } -/// Given a [`DerivationTree`], collapse any [`External::FromDependencyOf`] incompatibilities -/// wrap an [`PubGrubPackageInner::Extra`] package. -fn collapse_proxies( - derivation_tree: &mut DerivationTree, UnavailableReason>, -) { - match derivation_tree { - DerivationTree::External(_) => {} - DerivationTree::Derived(derived) => { - match ( - Arc::make_mut(&mut derived.cause1), - Arc::make_mut(&mut derived.cause2), - ) { - ( - DerivationTree::External(External::FromDependencyOf(package, ..)), - ref mut cause, - ) if matches!( - &**package, - PubGrubPackageInner::Extra { .. } - | PubGrubPackageInner::Marker { .. } - | PubGrubPackageInner::Dev { .. } - ) => - { - collapse_proxies(cause); - *derivation_tree = cause.clone(); - } - ( - ref mut cause, - DerivationTree::External(External::FromDependencyOf(package, ..)), - ) if matches!( - &**package, - PubGrubPackageInner::Extra { .. } - | PubGrubPackageInner::Marker { .. } - | PubGrubPackageInner::Dev { .. } - ) => - { - collapse_proxies(cause); - *derivation_tree = cause.clone(); - } - _ => { - collapse_proxies(Arc::make_mut(&mut derived.cause1)); - collapse_proxies(Arc::make_mut(&mut derived.cause2)); - } - } - } - } -} - -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, - fork_urls: ForkUrls, - ) -> Self { - match value { - // These are all never type variant that can never match, but never is experimental - pubgrub::error::PubGrubError::ErrorChoosingPackageVersion(_) - | pubgrub::error::PubGrubError::ErrorInShouldCancel(_) - | pubgrub::error::PubGrubError::ErrorRetrievingDependencies { .. } => { - unreachable!() - } - pubgrub::error::PubGrubError::Failure(inner) => Self::Failure(inner), - pubgrub::error::PubGrubError::NoSolution(mut derivation_tree) => { - collapse_proxies(&mut derivation_tree); - - Self::NoSolution(NoSolutionError { - derivation_tree, - // The following should be populated before display for the best error messages - available_versions: FxHashMap::default(), - selector: None, - python_requirement: None, - index_locations: None, - unavailable_packages: FxHashMap::default(), - incomplete_packages: FxHashMap::default(), - fork_urls, - }) - } - pubgrub::error::PubGrubError::SelfDependency { package, version } => { - Self::SelfDependency { - package: Box::new(package), - version: Box::new(version), - } - } - } - } -} - -/// A wrapper around [`pubgrub::error::PubGrubError::NoSolution`] that displays a resolution failure report. +/// A wrapper around [`pubgrub::error::NoSolutionError`] that displays a resolution failure report. #[derive(Debug)] pub struct NoSolutionError { - derivation_tree: DerivationTree, UnavailableReason>, + error: pubgrub::error::NoSolutionError, available_versions: FxHashMap>, - selector: Option, - python_requirement: Option, - index_locations: Option, + selector: CandidateSelector, + python_requirement: PythonRequirement, + index_locations: IndexLocations, unavailable_packages: FxHashMap, incomplete_packages: FxHashMap>, fork_urls: ForkUrls, } +impl NoSolutionError { + pub(crate) fn new( + error: pubgrub::error::NoSolutionError, + available_versions: FxHashMap>, + selector: CandidateSelector, + python_requirement: PythonRequirement, + index_locations: IndexLocations, + unavailable_packages: FxHashMap, + incomplete_packages: FxHashMap>, + fork_urls: ForkUrls, + ) -> Self { + Self { + error, + available_versions, + selector, + python_requirement, + index_locations, + unavailable_packages, + incomplete_packages, + fork_urls, + } + } + + /// Given a [`DerivationTree`], collapse any [`External::FromDependencyOf`] incompatibilities + /// wrap an [`PubGrubPackageInner::Extra`] package. + pub(crate) fn collapse_proxies( + derivation_tree: &mut DerivationTree, UnavailableReason>, + ) { + match derivation_tree { + DerivationTree::External(_) => {} + DerivationTree::Derived(derived) => { + match ( + Arc::make_mut(&mut derived.cause1), + Arc::make_mut(&mut derived.cause2), + ) { + ( + DerivationTree::External(External::FromDependencyOf(package, ..)), + ref mut cause, + ) if matches!( + &**package, + PubGrubPackageInner::Extra { .. } + | PubGrubPackageInner::Marker { .. } + | PubGrubPackageInner::Dev { .. } + ) => + { + Self::collapse_proxies(cause); + *derivation_tree = cause.clone(); + } + ( + ref mut cause, + DerivationTree::External(External::FromDependencyOf(package, ..)), + ) if matches!( + &**package, + PubGrubPackageInner::Extra { .. } + | PubGrubPackageInner::Marker { .. } + | PubGrubPackageInner::Dev { .. } + ) => + { + Self::collapse_proxies(cause); + *derivation_tree = cause.clone(); + } + _ => { + Self::collapse_proxies(Arc::make_mut(&mut derived.cause1)); + Self::collapse_proxies(Arc::make_mut(&mut derived.cause2)); + } + } + } + } + } +} + impl std::error::Error for NoSolutionError {} impl std::fmt::Display for NoSolutionError { @@ -227,15 +205,14 @@ impl std::fmt::Display for NoSolutionError { // Write the derivation report. let formatter = PubGrubReportFormatter { available_versions: &self.available_versions, - python_requirement: self.python_requirement.as_ref(), + python_requirement: &self.python_requirement, }; - let report = - DefaultStringReporter::report_with_formatter(&self.derivation_tree, &formatter); + let report = DefaultStringReporter::report_with_formatter(&self.error, &formatter); write!(f, "{report}")?; // Include any additional hints. for hint in formatter.hints( - &self.derivation_tree, + &self.error, &self.selector, &self.index_locations, &self.unavailable_packages, @@ -248,113 +225,3 @@ impl std::fmt::Display for NoSolutionError { Ok(()) } } - -impl NoSolutionError { - /// Update the available versions attached to the error using the given package version index. - /// - /// Only packages used in the error's derivation tree will be retrieved. - #[must_use] - pub(crate) fn with_available_versions( - mut self, - visited: &FxHashSet, - package_versions: &FxOnceMap>, - ) -> Self { - let mut available_versions = FxHashMap::default(); - for package in self.derivation_tree.packages() { - match &**package { - PubGrubPackageInner::Root { .. } => {} - PubGrubPackageInner::Python { .. } => {} - PubGrubPackageInner::Marker { .. } => {} - PubGrubPackageInner::Extra { .. } => {} - PubGrubPackageInner::Dev { .. } => {} - PubGrubPackageInner::Package { name, .. } => { - // Avoid including available versions for packages that exist in the derivation - // tree, but were never visited during resolution. We _may_ have metadata for - // these packages, but it's non-deterministic, and omitting them ensures that - // we represent the state of the resolver at the time of failure. - if visited.contains(name) { - if let Some(response) = package_versions.get(name) { - if let VersionsResponse::Found(ref version_maps) = *response { - for version_map in version_maps { - available_versions - .entry(package.clone()) - .or_insert_with(BTreeSet::new) - .extend( - version_map.iter().map(|(version, _)| version.clone()), - ); - } - } - } - } - } - } - } - self.available_versions = available_versions; - self - } - - /// Update the candidate selector attached to the error. - #[must_use] - pub(crate) fn with_selector(mut self, selector: CandidateSelector) -> Self { - self.selector = Some(selector); - self - } - - /// Update the index locations attached to the error. - #[must_use] - pub(crate) fn with_index_locations(mut self, index_locations: &IndexLocations) -> Self { - self.index_locations = Some(index_locations.clone()); - self - } - - /// Update the unavailable packages attached to the error. - #[must_use] - pub(crate) fn with_unavailable_packages( - mut self, - unavailable_packages: &DashMap, - ) -> Self { - let mut new = FxHashMap::default(); - for package in self.derivation_tree.packages() { - if let PubGrubPackageInner::Package { name, .. } = &**package { - if let Some(reason) = unavailable_packages.get(name) { - new.insert(name.clone(), reason.clone()); - } - } - } - self.unavailable_packages = new; - self - } - - /// Update the incomplete packages attached to the error. - #[must_use] - pub(crate) fn with_incomplete_packages( - mut self, - incomplete_packages: &DashMap>, - ) -> Self { - let mut new = FxHashMap::default(); - for package in self.derivation_tree.packages() { - if let PubGrubPackageInner::Package { name, .. } = &**package { - if let Some(versions) = incomplete_packages.get(name) { - for entry in versions.iter() { - let (version, reason) = entry.pair(); - new.entry(name.clone()) - .or_insert_with(BTreeMap::default) - .insert(version.clone(), reason.clone()); - } - } - } - } - self.incomplete_packages = new; - self - } - - /// Update the Python requirements attached to the error. - #[must_use] - pub(crate) fn with_python_requirement( - mut self, - python_requirement: &PythonRequirement, - ) -> Self { - self.python_requirement = Some(python_requirement.clone()); - self - } -} diff --git a/crates/uv-resolver/src/pubgrub/report.rs b/crates/uv-resolver/src/pubgrub/report.rs index 24e133fb6..1747dffcb 100644 --- a/crates/uv-resolver/src/pubgrub/report.rs +++ b/crates/uv-resolver/src/pubgrub/report.rs @@ -30,7 +30,7 @@ pub(crate) struct PubGrubReportFormatter<'a> { pub(crate) available_versions: &'a FxHashMap>, /// The versions that were available for each package - pub(crate) python_requirement: Option<&'a PythonRequirement>, + pub(crate) python_requirement: &'a PythonRequirement, } impl ReportFormatter, UnavailableReason> @@ -47,33 +47,31 @@ impl ReportFormatter, UnavailableReason> format!("we are solving dependencies of {package} {version}") } External::NoVersions(package, set) => { - if let Some(python) = self.python_requirement { - if matches!( - &**package, - PubGrubPackageInner::Python(PubGrubPython::Target) - ) { - return if let Some(target) = python.target() { - format!( - "the requested {package} version ({target}) does not satisfy {}", - PackageRange::compatibility(package, set) - ) - } else { - format!( - "the requested {package} version does not satisfy {}", - PackageRange::compatibility(package, set) - ) - }; - } - if matches!( - &**package, - PubGrubPackageInner::Python(PubGrubPython::Installed) - ) { - return format!( - "the current {package} version ({}) does not satisfy {}", - python.installed(), + if matches!( + &**package, + PubGrubPackageInner::Python(PubGrubPython::Target) + ) { + return if let Some(target) = self.python_requirement.target() { + format!( + "the requested {package} version ({target}) does not satisfy {}", PackageRange::compatibility(package, set) - ); - } + ) + } else { + format!( + "the requested {package} version does not satisfy {}", + PackageRange::compatibility(package, set) + ) + }; + } + if matches!( + &**package, + PubGrubPackageInner::Python(PubGrubPython::Installed) + ) { + return format!( + "the current {package} version ({}) does not satisfy {}", + self.python_requirement.installed(), + PackageRange::compatibility(package, set) + ); } let set = self.simplify_set(set, package); @@ -404,8 +402,8 @@ impl PubGrubReportFormatter<'_> { pub(crate) fn hints( &self, derivation_tree: &DerivationTree, UnavailableReason>, - selector: &Option, - index_locations: &Option, + selector: &CandidateSelector, + index_locations: &IndexLocations, unavailable_packages: &FxHashMap, incomplete_packages: &FxHashMap>, fork_urls: &ForkUrls, @@ -417,28 +415,22 @@ impl PubGrubReportFormatter<'_> { ) => { if let PubGrubPackageInner::Package { name, .. } = &**package { // Check for no versions due to pre-release options. - if let Some(selector) = selector { - if !fork_urls.contains_key(name) { - self.prerelease_available_hint( - package, name, set, selector, &mut hints, - ); - } + if !fork_urls.contains_key(name) { + self.prerelease_available_hint(package, name, set, selector, &mut hints); } } if let PubGrubPackageInner::Package { name, .. } = &**package { // Check for no versions due to no `--find-links` flat index - if let Some(index_locations) = index_locations { - Self::index_hints( - package, - name, - set, - index_locations, - unavailable_packages, - incomplete_packages, - &mut hints, - ); - } + Self::index_hints( + package, + name, + set, + index_locations, + unavailable_packages, + incomplete_packages, + &mut hints, + ); } } DerivationTree::External(External::FromDependencyOf( @@ -452,16 +444,15 @@ impl PubGrubReportFormatter<'_> { &**dependency, PubGrubPackageInner::Python(PubGrubPython::Target) ) { - if let Some(python) = self.python_requirement { - if let Some(PythonTarget::RequiresPython(requires_python)) = python.target() - { - hints.insert(PubGrubHint::RequiresPython { - requires_python: requires_python.clone(), - package: package.clone(), - package_set: self.simplify_set(package_set, package).into_owned(), - package_requires_python: dependency_set.clone(), - }); - } + if let Some(PythonTarget::RequiresPython(requires_python)) = + self.python_requirement.target() + { + hints.insert(PubGrubHint::RequiresPython { + requires_python: requires_python.clone(), + package: package.clone(), + package_set: self.simplify_set(package_set, package).into_owned(), + package_requires_python: dependency_set.clone(), + }); } } } diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 4d448bd1f..43fe644d3 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use std::collections::hash_map::Entry; -use std::collections::{BTreeMap, VecDeque}; +use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::fmt::{Display, Formatter}; use std::ops::Bound; use std::sync::Arc; @@ -11,9 +11,8 @@ use std::{iter, thread}; use dashmap::DashMap; use either::Either; -use futures::{FutureExt, StreamExt, TryFutureExt}; +use futures::{FutureExt, StreamExt}; use itertools::Itertools; -use pubgrub::error::PubGrubError; use pubgrub::range::Range; use pubgrub::solver::{Incompatibility, State}; use rustc_hash::{FxHashMap, FxHashSet}; @@ -24,8 +23,8 @@ use tracing::{debug, enabled, instrument, trace, warn, Level}; use distribution_types::{ BuiltDist, CompatibleDist, Dist, DistributionMetadata, IncompatibleDist, IncompatibleSource, - IncompatibleWheel, InstalledDist, PythonRequirementKind, RemoteSource, ResolvedDist, - ResolvedDistRef, SourceDist, VersionOrUrlRef, + IncompatibleWheel, IndexLocations, InstalledDist, PythonRequirementKind, RemoteSource, + ResolvedDist, ResolvedDistRef, SourceDist, VersionOrUrlRef, }; pub(crate) use locals::Locals; use pep440_rs::{Version, MIN_VERSION}; @@ -41,7 +40,7 @@ use uv_types::{BuildContext, HashStrategy, InstalledPackagesProvider}; use crate::candidate_selector::{CandidateDist, CandidateSelector}; use crate::dependency_provider::UvDependencyProvider; -use crate::error::ResolveError; +use crate::error::{NoSolutionError, ResolveError}; use crate::fork_urls::ForkUrls; use crate::manifest::Manifest; use crate::marker::{normalize, requires_python_marker}; @@ -245,51 +244,26 @@ impl let (request_sink, request_stream) = mpsc::channel(300); // Run the fetcher. - let requests_fut = state - .clone() - .fetch(provider.clone(), request_stream) - .map_err(|err| (err, FxHashSet::default())) - .fuse(); + let requests_fut = state.clone().fetch(provider.clone(), request_stream).fuse(); // Spawn the PubGrub solver on a dedicated thread. let solver = state.clone(); + let index_locations = provider.index_locations().clone(); let (tx, rx) = oneshot::channel(); thread::Builder::new() .name("uv-resolver".into()) .spawn(move || { - let result = solver.solve(request_sink); + let result = solver.solve(index_locations, request_sink); tx.send(result).unwrap(); }) .unwrap(); - let resolve_fut = async move { - rx.await - .map_err(|_| (ResolveError::ChannelClosed, FxHashSet::default())) - .and_then(|result| result) - }; + let resolve_fut = async move { rx.await.map_err(|_| ResolveError::ChannelClosed) }; // Wait for both to complete. - match tokio::try_join!(requests_fut, resolve_fut) { - Ok(((), resolution)) => { - state.on_complete(); - Ok(resolution) - } - Err((err, visited)) => { - // Add version information to improve unsat error messages. - Err(if let ResolveError::NoSolution(err) = err { - ResolveError::NoSolution( - err.with_available_versions(&visited, state.index.packages()) - .with_selector(state.selector.clone()) - .with_python_requirement(&state.python_requirement) - .with_index_locations(provider.index_locations()) - .with_unavailable_packages(&state.unavailable_packages) - .with_incomplete_packages(&state.incomplete_packages), - ) - } else { - err - }) - } - } + let ((), resolution) = tokio::try_join!(requests_fut, resolve_fut)?; + state.on_complete(); + resolution } } @@ -297,19 +271,8 @@ impl ResolverState, - request_sink: Sender, - ) -> Result)> { - let mut visited = FxHashSet::default(); - self.solve_tracked(&mut visited, request_sink) - .map_err(|err| (err, visited)) - } - - /// Run the PubGrub solver, updating the `visited` set for each package visited during - /// resolution. - #[instrument(skip_all)] - fn solve_tracked( - self: Arc, - visited: &mut FxHashSet, + // No solution error context. + index_locations: IndexLocations, request_sink: Sender, ) -> Result { debug!( @@ -320,6 +283,8 @@ impl ResolverState ResolverState ResolverState ResolverState ResolverState ResolverState ResolverState, + fork_urls: ForkUrls, + visited: &FxHashSet, + index_locations: &IndexLocations, + ) -> ResolveError { + NoSolutionError::collapse_proxies(&mut err); + + let mut unavailable_packages = FxHashMap::default(); + for package in err.packages() { + if let PubGrubPackageInner::Package { name, .. } = &**package { + if let Some(reason) = self.unavailable_packages.get(name) { + unavailable_packages.insert(name.clone(), reason.clone()); + } + } + } + + let mut incomplete_packages = FxHashMap::default(); + for package in err.packages() { + if let PubGrubPackageInner::Package { name, .. } = &**package { + if let Some(versions) = self.incomplete_packages.get(name) { + for entry in versions.iter() { + let (version, reason) = entry.pair(); + incomplete_packages + .entry(name.clone()) + .or_insert_with(BTreeMap::default) + .insert(version.clone(), reason.clone()); + } + } + } + } + + let mut available_versions = FxHashMap::default(); + for package in err.packages() { + let PubGrubPackageInner::Package { name, .. } = &**package else { + continue; + }; + if !visited.contains(name) { + // Avoid including available versions for packages that exist in the derivation + // tree, but were never visited during resolution. We _may_ have metadata for + // these packages, but it's non-deterministic, and omitting them ensures that + // we represent the self of the resolver at the time of failure. + continue; + } + if let Some(response) = self.index.packages().get(name) { + if let VersionsResponse::Found(ref version_maps) = *response { + for version_map in version_maps { + available_versions + .entry(package.clone()) + .or_insert_with(BTreeSet::new) + .extend(version_map.iter().map(|(version, _)| version.clone())); + } + } + } + } + + ResolveError::NoSolution(NoSolutionError::new( + err, + available_versions, + self.selector.clone(), + self.python_requirement.clone(), + index_locations.clone(), + unavailable_packages, + incomplete_packages, + fork_urls, + )) + } + fn on_progress(&self, package: &PubGrubPackage, version: &Version) { if let Some(reporter) = self.reporter.as_ref() { match &**package { @@ -1908,25 +1935,7 @@ impl ForkState { urls: &Urls, dependencies: Vec, git: &GitResolver, - prefetcher: &BatchPrefetcher, ) -> Result<(), ResolveError> { - // Check for self-dependencies. - if dependencies - .iter() - .any(|dependency| dependency.package == self.next) - { - if enabled!(Level::DEBUG) { - prefetcher.log_tried_versions(); - } - return Err(ResolveError::from_pubgrub_error( - PubGrubError::SelfDependency { - package: self.next.clone(), - version: version.clone(), - }, - self.fork_urls.clone(), - )); - } - for dependency in &dependencies { let PubGrubDependency { package,