From b37170df9432d76254b449cbabad97b3bbee0ff9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 14 Nov 2024 09:51:11 -0500 Subject: [PATCH] Rename `ResolutionGraph` to `ResolverOutput` (#9103) ## Summary As discussed in Discord... This struct has evolved to include a lot of information apart from the `petgraph::Graph`. And I want to add a graph to the simplified `Resolution` type. So I think this name makes more sense. --- crates/uv-bench/benches/uv.rs | 4 +- crates/uv-dispatch/src/lib.rs | 6 +- crates/uv-resolver/src/lib.rs | 2 +- crates/uv-resolver/src/lock/mod.rs | 60 +++++++++---------- crates/uv-resolver/src/resolution/display.rs | 22 +++---- crates/uv-resolver/src/resolution/mod.rs | 6 +- .../src/resolution/{graph.rs => output.rs} | 60 +++++++++---------- crates/uv-resolver/src/resolver/mod.rs | 8 +-- crates/uv/src/commands/pip/operations.rs | 4 +- crates/uv/src/commands/project/environment.rs | 31 +++++----- crates/uv/src/commands/project/lock.rs | 2 +- crates/uv/src/commands/project/mod.rs | 8 +-- 12 files changed, 104 insertions(+), 109 deletions(-) rename crates/uv-resolver/src/resolution/{graph.rs => output.rs} (95%) diff --git a/crates/uv-bench/benches/uv.rs b/crates/uv-bench/benches/uv.rs index c900837e5..24bf8aa35 100644 --- a/crates/uv-bench/benches/uv.rs +++ b/crates/uv-bench/benches/uv.rs @@ -101,7 +101,7 @@ mod resolver { use uv_python::Interpreter; use uv_resolver::{ FlatIndex, InMemoryIndex, Manifest, OptionsBuilder, PythonRequirement, RequiresPython, - ResolutionGraph, Resolver, ResolverEnvironment, + Resolver, ResolverEnvironment, ResolverOutput, }; use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight}; @@ -139,7 +139,7 @@ mod resolver { client: &RegistryClient, interpreter: &Interpreter, universal: bool, - ) -> Result { + ) -> Result { let build_isolation = BuildIsolation::default(); let build_options = BuildOptions::default(); let concurrency = Concurrency::default(); diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 28c0a9629..a2fbaef0c 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -197,7 +197,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { EmptyInstalledPackages, DistributionDatabase::new(self.client, self, self.concurrency.downloads), )?; - let graph = resolver.resolve().await.with_context(|| { + let resolution = Resolution::from(resolver.resolve().await.with_context(|| { format!( "No solution found when resolving: {}", requirements @@ -205,8 +205,8 @@ impl<'a> BuildContext for BuildDispatch<'a> { .map(|requirement| format!("`{requirement}`")) .join(", ") ) - })?; - Ok(Resolution::from(graph)) + })?); + Ok(resolution) } #[instrument( diff --git a/crates/uv-resolver/src/lib.rs b/crates/uv-resolver/src/lib.rs index d0f3d00c0..eb1ad013b 100644 --- a/crates/uv-resolver/src/lib.rs +++ b/crates/uv-resolver/src/lib.rs @@ -14,7 +14,7 @@ pub use prerelease::PrereleaseMode; pub use python_requirement::PythonRequirement; pub use requires_python::{RequiresPython, RequiresPythonRange}; pub use resolution::{ - AnnotationStyle, ConflictingDistributionError, DisplayResolutionGraph, ResolutionGraph, + AnnotationStyle, ConflictingDistributionError, DisplayResolutionGraph, ResolverOutput, }; pub use resolution_mode::ResolutionMode; pub use resolver::{ diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index 9dceeb6e9..90d2a5b5a 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -20,8 +20,8 @@ pub use crate::lock::tree::TreeDisplay; use crate::requires_python::SimplifiedMarkerTree; use crate::resolution::{AnnotatedDist, ResolutionGraphNode}; use crate::{ - ExcludeNewer, InMemoryIndex, MetadataResponse, PrereleaseMode, RequiresPython, ResolutionGraph, - ResolutionMode, + ExcludeNewer, InMemoryIndex, MetadataResponse, PrereleaseMode, RequiresPython, ResolutionMode, + ResolverOutput, }; use uv_cache_key::RepositoryUrl; use uv_configuration::BuildOptions; @@ -108,16 +108,16 @@ pub struct Lock { } impl Lock { - /// Initialize a [`Lock`] from a [`ResolutionGraph`]. - pub fn from_resolution_graph(graph: &ResolutionGraph, root: &Path) -> Result { + /// Initialize a [`Lock`] from a [`ResolverOutput`]. + pub fn from_resolution(resolution: &ResolverOutput, root: &Path) -> Result { let mut packages = BTreeMap::new(); - let requires_python = graph.requires_python.clone(); + let requires_python = resolution.requires_python.clone(); // Determine the set of packages included at multiple versions. let mut seen = FxHashSet::default(); let mut duplicates = FxHashSet::default(); - for node_index in graph.petgraph.node_indices() { - let ResolutionGraphNode::Dist(dist) = &graph.petgraph[node_index] else { + for node_index in resolution.graph.node_indices() { + let ResolutionGraphNode::Dist(dist) = &resolution.graph[node_index] else { continue; }; if !dist.is_base() { @@ -129,8 +129,8 @@ impl Lock { } // Lock all base packages. - for node_index in graph.petgraph.node_indices() { - let ResolutionGraphNode::Dist(dist) = &graph.petgraph[node_index] else { + for node_index in resolution.graph.node_indices() { + let ResolutionGraphNode::Dist(dist) = &resolution.graph[node_index] else { continue; }; if !dist.is_base() { @@ -140,7 +140,7 @@ impl Lock { // If there are multiple distributions for the same package, include the markers of all // forks that included the current distribution. let fork_markers = if duplicates.contains(dist.name()) { - graph + resolution .fork_markers .iter() .filter(|fork_markers| !fork_markers.is_disjoint(&dist.marker)) @@ -151,11 +151,11 @@ impl Lock { }; let mut package = Package::from_annotated_dist(dist, fork_markers, root)?; - Self::remove_unreachable_wheels(graph, &requires_python, node_index, &mut package); + Self::remove_unreachable_wheels(resolution, &requires_python, node_index, &mut package); // Add all dependencies - for edge in graph.petgraph.edges(node_index) { - let ResolutionGraphNode::Dist(dependency_dist) = &graph.petgraph[edge.target()] + for edge in resolution.graph.edges(node_index) { + let ResolutionGraphNode::Dist(dependency_dist) = &resolution.graph[edge.target()] else { continue; }; @@ -173,8 +173,8 @@ impl Lock { } // Lock all extras and development dependencies. - for node_index in graph.petgraph.node_indices() { - let ResolutionGraphNode::Dist(dist) = &graph.petgraph[node_index] else { + for node_index in resolution.graph.node_indices() { + let ResolutionGraphNode::Dist(dist) = &resolution.graph[node_index] else { continue; }; if let Some(extra) = dist.extra.as_ref() { @@ -186,8 +186,9 @@ impl Lock { } .into()); }; - for edge in graph.petgraph.edges(node_index) { - let ResolutionGraphNode::Dist(dependency_dist) = &graph.petgraph[edge.target()] + for edge in resolution.graph.edges(node_index) { + let ResolutionGraphNode::Dist(dependency_dist) = + &resolution.graph[edge.target()] else { continue; }; @@ -210,8 +211,9 @@ impl Lock { } .into()); }; - for edge in graph.petgraph.edges(node_index) { - let ResolutionGraphNode::Dist(dependency_dist) = &graph.petgraph[edge.target()] + for edge in resolution.graph.edges(node_index) { + let ResolutionGraphNode::Dist(dependency_dist) = + &resolution.graph[edge.target()] else { continue; }; @@ -229,9 +231,9 @@ impl Lock { let packages = packages.into_values().collect(); let options = ResolverOptions { - resolution_mode: graph.options.resolution_mode, - prerelease_mode: graph.options.prerelease_mode, - exclude_newer: graph.options.exclude_newer, + resolution_mode: resolution.options.resolution_mode, + prerelease_mode: resolution.options.prerelease_mode, + exclude_newer: resolution.options.exclude_newer, }; let lock = Self::new( VERSION, @@ -241,7 +243,7 @@ impl Lock { ResolverManifest::default(), Conflicts::empty(), vec![], - graph.fork_markers.clone(), + resolution.fork_markers.clone(), )?; Ok(lock) } @@ -251,7 +253,7 @@ impl Lock { /// For example, a package included under `sys_platform == 'win32'` does not need Linux /// wheels. fn remove_unreachable_wheels( - graph: &ResolutionGraph, + graph: &ResolverOutput, requires_python: &RequiresPython, node_index: NodeIndex, locked_dist: &mut Package, @@ -288,20 +290,16 @@ impl Lock { tag.starts_with(linux_tag) || tag == "linux_armv6l" || tag == "linux_armv7l" }) }) { - !graph.petgraph[node_index] - .marker() - .is_disjoint(&LINUX_MARKERS) + !graph.graph[node_index].marker().is_disjoint(&LINUX_MARKERS) } else if platform_tags .iter() .all(|tag| windows_tags.contains(&&**tag)) { - !graph.petgraph[node_index] + !graph.graph[node_index] .marker() .is_disjoint(&WINDOWS_MARKERS) } else if platform_tags.iter().all(|tag| tag.starts_with("macosx_")) { - !graph.petgraph[node_index] - .marker() - .is_disjoint(&MAC_MARKERS) + !graph.graph[node_index].marker().is_disjoint(&MAC_MARKERS) } else { true } diff --git a/crates/uv-resolver/src/resolution/display.rs b/crates/uv-resolver/src/resolution/display.rs index 5c9d580bb..c811c3e68 100644 --- a/crates/uv-resolver/src/resolution/display.rs +++ b/crates/uv-resolver/src/resolution/display.rs @@ -12,14 +12,14 @@ use uv_normalize::PackageName; use uv_pep508::MarkerTree; use crate::resolution::{RequirementsTxtDist, ResolutionGraphNode}; -use crate::{ResolutionGraph, ResolverEnvironment}; +use crate::{ResolverEnvironment, ResolverOutput}; /// A [`std::fmt::Display`] implementation for the resolution graph. #[derive(Debug)] #[allow(clippy::struct_excessive_bools)] pub struct DisplayResolutionGraph<'a> { /// The underlying graph. - resolution: &'a ResolutionGraph, + resolution: &'a ResolverOutput, /// The resolver marker environment, used to determine the markers that apply to each package. env: &'a ResolverEnvironment, /// The packages to exclude from the output. @@ -50,7 +50,7 @@ impl<'a> DisplayResolutionGraph<'a> { /// Create a new [`DisplayResolutionGraph`] for the given graph. #[allow(clippy::fn_params_excessive_bools)] pub fn new( - underlying: &'a ResolutionGraph, + underlying: &'a ResolverOutput, env: &'a ResolverEnvironment, no_emit_packages: &'a [PackageName], show_hashes: bool, @@ -136,7 +136,7 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> { // We assign each package its propagated markers: In `requirements.txt`, we want a flat list // that for each package tells us if it should be installed on the current platform, without // looking at which packages depend on it. - let petgraph = self.resolution.petgraph.map( + let graph = self.resolution.graph.map( |_index, node| match node { ResolutionGraphNode::Root => DisplayResolutionGraphNode::Root, ResolutionGraphNode::Dist(dist) => { @@ -150,17 +150,17 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> { ); // Reduce the graph, removing or combining extras for a given package. - let petgraph = if self.include_extras { - combine_extras(&petgraph) + let graph = if self.include_extras { + combine_extras(&graph) } else { - strip_extras(&petgraph) + strip_extras(&graph) }; // Collect all packages. - let mut nodes = petgraph + let mut nodes = graph .node_indices() .filter_map(|index| { - let dist = &petgraph[index]; + let dist = &graph[index]; let name = dist.name(); if self.no_emit_packages.contains(name) { return None; @@ -199,9 +199,9 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> { if self.include_annotations { // Display all dependents (i.e., all packages that depend on the current package). let dependents = { - let mut dependents = petgraph + let mut dependents = graph .edges_directed(index, Direction::Incoming) - .map(|edge| &petgraph[edge.source()]) + .map(|edge| &graph[edge.source()]) .map(uv_distribution_types::Name::name) .collect::>(); dependents.sort_unstable(); diff --git a/crates/uv-resolver/src/resolution/mod.rs b/crates/uv-resolver/src/resolution/mod.rs index 5b75baa3f..beb0a85f1 100644 --- a/crates/uv-resolver/src/resolution/mod.rs +++ b/crates/uv-resolver/src/resolution/mod.rs @@ -11,12 +11,12 @@ use uv_pep508::MarkerTree; use uv_pypi_types::HashDigest; pub use crate::resolution::display::{AnnotationStyle, DisplayResolutionGraph}; -pub(crate) use crate::resolution::graph::ResolutionGraphNode; -pub use crate::resolution::graph::{ConflictingDistributionError, ResolutionGraph}; +pub(crate) use crate::resolution::output::ResolutionGraphNode; +pub use crate::resolution::output::{ConflictingDistributionError, ResolverOutput}; pub(crate) use crate::resolution::requirements_txt::RequirementsTxtDist; mod display; -mod graph; +mod output; mod requirements_txt; /// A pinned package with its resolved distribution and metadata. The [`ResolvedDist`] refers to a diff --git a/crates/uv-resolver/src/resolution/graph.rs b/crates/uv-resolver/src/resolution/output.rs similarity index 95% rename from crates/uv-resolver/src/resolution/graph.rs rename to crates/uv-resolver/src/resolution/output.rs index 6696bc998..bb4f2f928 100644 --- a/crates/uv-resolver/src/resolution/graph.rs +++ b/crates/uv-resolver/src/resolution/output.rs @@ -35,12 +35,14 @@ use crate::{ pub(crate) type MarkersForDistribution = Vec; -/// A complete resolution graph in which every node represents a pinned package and every edge -/// represents a dependency between two pinned packages. +/// The output of a successful resolution. +/// +/// Includes a complete resolution graph in which every node represents a pinned package and every +/// edge represents a dependency between two pinned packages. #[derive(Debug)] -pub struct ResolutionGraph { +pub struct ResolverOutput { /// The underlying graph. - pub(crate) petgraph: Graph, + pub(crate) graph: Graph, /// The range of supported Python versions. pub(crate) requires_python: RequiresPython, /// If the resolution had non-identical forks, store the forks in the lockfile so we can @@ -92,8 +94,8 @@ struct PackageRef<'a> { group: Option<&'a GroupName>, } -impl ResolutionGraph { - /// Create a new graph from the resolved PubGrub state. +impl ResolverOutput { + /// Create a new [`ResolverOutput`] from the resolved PubGrub state. pub(crate) fn from_state( resolutions: &[Resolution], requirements: &[Requirement], @@ -108,14 +110,14 @@ impl ResolutionGraph { options: Options, ) -> Result { let size_guess = resolutions[0].nodes.len(); - let mut petgraph: Graph = + let mut graph: Graph = Graph::with_capacity(size_guess, size_guess); let mut inverse: FxHashMap> = FxHashMap::with_capacity_and_hasher(size_guess, FxBuildHasher); let mut diagnostics = Vec::new(); // Add the root node. - let root_index = petgraph.add_node(ResolutionGraphNode::Root); + let root_index = graph.add_node(ResolutionGraphNode::Root); let mut package_markers: FxHashMap = FxHashMap::default(); @@ -140,7 +142,7 @@ impl ResolutionGraph { continue; } Self::add_version( - &mut petgraph, + &mut graph, &mut inverse, &mut diagnostics, preferences, @@ -165,13 +167,7 @@ impl ResolutionGraph { continue; } - Self::add_edge( - &mut petgraph, - &mut inverse, - root_index, - edge, - marker.clone(), - ); + Self::add_edge(&mut graph, &mut inverse, root_index, edge, marker.clone()); } } @@ -213,24 +209,24 @@ impl ResolutionGraph { }; // Compute and apply the marker reachability. - let mut reachability = marker_reachability(&petgraph, &fork_markers); + let mut reachability = marker_reachability(&graph, &fork_markers); // Apply the reachability to the graph. - for index in petgraph.node_indices() { - if let ResolutionGraphNode::Dist(dist) = &mut petgraph[index] { + for index in graph.node_indices() { + if let ResolutionGraphNode::Dist(dist) = &mut graph[index] { dist.marker = reachability.remove(&index).unwrap_or_default(); } } // Discard any unreachable nodes. - petgraph.retain_nodes(|graph, node| !graph[node].marker().is_false()); + graph.retain_nodes(|graph, node| !graph[node].marker().is_false()); if matches!(resolution_strategy, ResolutionStrategy::Lowest) { - report_missing_lower_bounds(&petgraph, &mut diagnostics); + report_missing_lower_bounds(&graph, &mut diagnostics); } - let graph = Self { - petgraph, + let output = Self { + graph, requires_python, diagnostics, requirements: requirements.to_vec(), @@ -253,7 +249,7 @@ impl ResolutionGraph { // versions of the same package into the same virtualenv. ---AG if conflicts.is_empty() { #[allow(unused_mut, reason = "Used in debug_assertions below")] - let mut conflicting = graph.find_conflicting_distributions(); + let mut conflicting = output.find_conflicting_distributions(); if !conflicting.is_empty() { tracing::warn!( "found {} conflicting distributions in resolution, \ @@ -275,7 +271,7 @@ impl ResolutionGraph { return Err(ResolveError::ConflictingDistribution(err)); } } - Ok(graph) + Ok(output) } fn add_edge( @@ -566,9 +562,9 @@ impl ResolutionGraph { /// Returns an iterator over the distinct packages in the graph. fn dists(&self) -> impl Iterator { - self.petgraph + self.graph .node_indices() - .filter_map(move |index| match &self.petgraph[index] { + .filter_map(move |index| match &self.graph[index] { ResolutionGraphNode::Root => None, ResolutionGraphNode::Dist(dist) => Some(dist), }) @@ -677,8 +673,8 @@ impl ResolutionGraph { } let mut seen_marker_values = IndexSet::default(); - for i in self.petgraph.node_indices() { - let ResolutionGraphNode::Dist(dist) = &self.petgraph[i] else { + for i in self.graph.node_indices() { + let ResolutionGraphNode::Dist(dist) = &self.graph[i] else { continue; }; let version_id = match dist.version_or_url() { @@ -750,7 +746,7 @@ impl ResolutionGraph { fn find_conflicting_distributions(&self) -> Vec { let mut name_to_markers: BTreeMap<&PackageName, Vec<(&Version, &MarkerTree)>> = BTreeMap::new(); - for node in self.petgraph.node_weights() { + for node in self.graph.node_weights() { let annotated_dist = match node { ResolutionGraphNode::Root => continue, ResolutionGraphNode::Dist(ref annotated_dist) => annotated_dist, @@ -818,8 +814,8 @@ impl Display for ConflictingDistributionError { } } -impl From for uv_distribution_types::Resolution { - fn from(graph: ResolutionGraph) -> Self { +impl From for uv_distribution_types::Resolution { + fn from(graph: ResolverOutput) -> Self { Self::new( graph .dists() diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 4a20dbcc6..d8fefd1a3 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -56,7 +56,7 @@ use crate::pubgrub::{ PubGrubPython, }; use crate::python_requirement::PythonRequirement; -use crate::resolution::ResolutionGraph; +use crate::resolution::ResolverOutput; use crate::resolution_mode::ResolutionStrategy; pub(crate) use crate::resolver::availability::{ IncompletePackage, ResolverVersion, UnavailablePackage, UnavailableReason, UnavailableVersion, @@ -251,7 +251,7 @@ impl } /// Resolve a set of requirements into a set of pinned versions. - pub async fn resolve(self) -> Result { + pub async fn resolve(self) -> Result { let state = Arc::new(self.state); let provider = Arc::new(self.provider); @@ -291,7 +291,7 @@ impl ResolverState, request_sink: Sender, - ) -> Result { + ) -> Result { debug!( "Solving with installed Python version: {}", self.python_requirement.exact() @@ -598,7 +598,7 @@ impl ResolverState( options: Options, logger: Box, printer: Printer, -) -> Result { +) -> Result { let start = std::time::Instant::now(); // Resolve the requirements from the provided sources. diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index e90c8abbb..5c67b782d 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -58,21 +58,22 @@ impl CachedEnvironment { }; // Resolve the requirements with the interpreter. - let graph = resolve_environment( - spec, - &interpreter, - settings.as_ref().into(), - state, - resolve, - connectivity, - concurrency, - native_tls, - allow_insecure_host, - cache, - printer, - ) - .await?; - let resolution = Resolution::from(graph); + let resolution = Resolution::from( + resolve_environment( + spec, + &interpreter, + settings.as_ref().into(), + state, + resolve, + connectivity, + concurrency, + native_tls, + allow_insecure_host, + cache, + printer, + ) + .await?, + ); // Hash the resolution by hashing the generated lockfile. // TODO(charlie): If the resolution contains any mutable metadata (like a path or URL diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 40ee4817a..93bd97cdc 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -659,7 +659,7 @@ async fn do_lock( .relative_to(workspace)?; let previous = existing_lock.map(ValidatedLock::into_lock); - let lock = Lock::from_resolution_graph(&resolution, workspace.install_path())? + let lock = Lock::from_resolution(&resolution, workspace.install_path())? .with_manifest(manifest) .with_conflicts(workspace.conflicts()) .with_supported_environments( diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index b1921b8ee..516fe62f5 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -31,8 +31,8 @@ use uv_python::{ use uv_requirements::upgrade::{read_lock_requirements, LockedRequirements}; use uv_requirements::{NamedRequirementsResolver, RequirementsSpecification}; use uv_resolver::{ - FlatIndex, Lock, OptionsBuilder, PythonRequirement, RequiresPython, ResolutionGraph, - ResolverEnvironment, + FlatIndex, Lock, OptionsBuilder, PythonRequirement, RequiresPython, ResolverEnvironment, + ResolverOutput, }; use uv_scripts::Pep723Item; use uv_settings::PythonInstallMirrors; @@ -955,7 +955,7 @@ impl<'lock> EnvironmentSpecification<'lock> { } } -/// Run dependency resolution for an interpreter, returning the [`ResolutionGraph`]. +/// Run dependency resolution for an interpreter, returning the [`ResolverOutput`]. pub(crate) async fn resolve_environment<'a>( spec: EnvironmentSpecification<'_>, interpreter: &Interpreter, @@ -968,7 +968,7 @@ pub(crate) async fn resolve_environment<'a>( allow_insecure_host: &[TrustedHost], cache: &Cache, printer: Printer, -) -> Result { +) -> Result { warn_on_requirements_txt_setting(&spec.requirements, settings); let ResolverSettingsRef {