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.
This commit is contained in:
Charlie Marsh 2024-11-14 09:51:11 -05:00 committed by GitHub
parent 02a7bb43d9
commit b37170df94
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
12 changed files with 104 additions and 109 deletions

View file

@ -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::{

View file

@ -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<Self, LockError> {
/// Initialize a [`Lock`] from a [`ResolverOutput`].
pub fn from_resolution(resolution: &ResolverOutput, root: &Path) -> Result<Self, LockError> {
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
}

View file

@ -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::<Vec<_>>();
dependents.sort_unstable();

View file

@ -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

View file

@ -35,12 +35,14 @@ use crate::{
pub(crate) type MarkersForDistribution = Vec<MarkerTree>;
/// 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<ResolutionGraphNode, MarkerTree, Directed>,
pub(crate) graph: Graph<ResolutionGraphNode, MarkerTree, Directed>,
/// 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<Self, ResolveError> {
let size_guess = resolutions[0].nodes.len();
let mut petgraph: Graph<ResolutionGraphNode, MarkerTree, Directed> =
let mut graph: Graph<ResolutionGraphNode, MarkerTree, Directed> =
Graph::with_capacity(size_guess, size_guess);
let mut inverse: FxHashMap<PackageRef, NodeIndex<u32>> =
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<PackageName, MarkersForDistribution> =
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<Item = &AnnotatedDist> {
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<ConflictingDistributionError> {
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<ResolutionGraph> for uv_distribution_types::Resolution {
fn from(graph: ResolutionGraph) -> Self {
impl From<ResolverOutput> for uv_distribution_types::Resolution {
fn from(graph: ResolverOutput) -> Self {
Self::new(
graph
.dists()

View file

@ -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<Provider: ResolverProvider, InstalledPackages: InstalledPackagesProvider>
}
/// Resolve a set of requirements into a set of pinned versions.
pub async fn resolve(self) -> Result<ResolutionGraph, ResolveError> {
pub async fn resolve(self) -> Result<ResolverOutput, ResolveError> {
let state = Arc::new(self.state);
let provider = Arc::new(self.provider);
@ -291,7 +291,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
fn solve(
self: Arc<Self>,
request_sink: Sender<Request>,
) -> Result<ResolutionGraph, ResolveError> {
) -> Result<ResolverOutput, ResolveError> {
debug!(
"Solving with installed Python version: {}",
self.python_requirement.exact()
@ -598,7 +598,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
for resolution in &resolutions {
Self::trace_resolution(resolution);
}
ResolutionGraph::from_state(
ResolverOutput::from_state(
&resolutions,
&self.requirements,
&self.constraints,