mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-04 10:58:28 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Remove uses of Option<MarkerTree> in ResolutionGraph (#6035)

Browse source 
## Summary

Missed this one in https://github.com/astral-sh/uv/pull/5978.

Resolves https://github.com/astral-sh/uv/issues/5902.
This commit is contained in:
Ibraheem Ahmed 2024-08-12 09:31:07 -05:00 • committed by GitHub
parent b911a108b9
commit fb5c3bb918
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 42 additions and 55 deletions
Download patch file Download diff file Expand all files Collapse all files

10
crates/pep508-rs/src/marker/tree.rs
View file

@ -1308,8 +1308,14 @@ impl ExtraMarkerTree<'_> {
pub struct MarkerTreeContents(MarkerTree);
impl From<MarkerTreeContents> for MarkerTree {
fn from(value: MarkerTreeContents) -> Self {
value.0
fn from(contents: MarkerTreeContents) -> Self {
contents.0
}
}
impl From<Option<MarkerTreeContents>> for MarkerTree {
fn from(marker: Option<MarkerTreeContents>) -> Self {
marker.map(|contents| contents.0).unwrap_or_default()
}
}

14
crates/uv-resolver/src/lock.rs
View file

@ -141,7 +141,7 @@ impl Lock {
continue;
};
let marker = edge.weight().clone();
locked_dist.add_dependency(dependency_dist, marker.unwrap_or_default());
locked_dist.add_dependency(dependency_dist, marker);
}
let id = locked_dist.id.clone();
if let Some(locked_dist) = locked_dists.insert(id, locked_dist) {
@ -173,11 +173,7 @@ impl Lock {
continue;
};
let marker = edge.weight().clone();
locked_dist.add_optional_dependency(
extra.clone(),
dependency_dist,
marker.unwrap_or_default(),
);
locked_dist.add_optional_dependency(extra.clone(), dependency_dist, marker);
}
}
if let Some(group) = dist.dev.as_ref() {
@ -195,11 +191,7 @@ impl Lock {
continue;
};
let marker = edge.weight().clone();
locked_dist.add_dev_dependency(
group.clone(),
dependency_dist,
marker.unwrap_or_default(),
);
locked_dist.add_dev_dependency(group.clone(), dependency_dist, marker);
}
}
}

28
crates/uv-resolver/src/resolution/display.rs
View file

@ -307,13 +307,13 @@ pub enum AnnotationStyle {
}
type ResolutionPetGraph =
petgraph::graph::Graph<ResolutionGraphNode, Option<MarkerTree>, petgraph::Directed>;
petgraph::graph::Graph<ResolutionGraphNode, MarkerTree, petgraph::Directed>;
type IntermediatePetGraph =
petgraph::graph::Graph<DisplayResolutionGraphNode, Option<MarkerTree>, petgraph::Directed>;
petgraph::graph::Graph<DisplayResolutionGraphNode, MarkerTree, petgraph::Directed>;
type RequirementsTxtGraph =
petgraph::graph::Graph<RequirementsTxtDist, Option<MarkerTree>, petgraph::Directed>;
petgraph::graph::Graph<RequirementsTxtDist, MarkerTree, petgraph::Directed>;
/// Convert a [`petgraph::graph::Graph`] based on [`ResolutionGraphNode`] to a graph based on
/// [`DisplayResolutionGraphNode`].
@ -384,10 +384,10 @@ fn propagate_markers(mut graph: IntermediatePetGraph) -> IntermediatePetGraph {
let mut edges = graph.edges_directed(index, Direction::Incoming);
edges
.next()
.and_then(|edge| graph.edge_weight(edge.id()).cloned().flatten())
.and_then(|edge| graph.edge_weight(edge.id()).cloned())
.and_then(|initial| {
edges.try_fold(initial, |mut acc, edge| {
acc.or(graph.edge_weight(edge.id())?.clone()?);
acc.or(graph.edge_weight(edge.id())?.clone());
Some(acc)
})
})
@ -400,11 +400,7 @@ fn propagate_markers(mut graph: IntermediatePetGraph) -> IntermediatePetGraph {
.detach();
while let Some((outgoing, _)) = walker.next(&graph) {
if let Some(weight) = graph.edge_weight_mut(outgoing) {
if let Some(weight) = weight {
weight.and(marker_tree.clone());
} else {
*weight = Some(marker_tree.clone());
}
weight.and(marker_tree.clone());
}
}
}
@ -417,7 +413,7 @@ fn propagate_markers(mut graph: IntermediatePetGraph) -> IntermediatePetGraph {
// Re-add the removed edges. We no longer care about the edge _weights_, but we do want the
// edges to be present, to power the `# via` annotations.
for (source, target) in edges {
graph.add_edge(source, target, None);
graph.add_edge(source, target, MarkerTree::TRUE);
}
graph
@ -464,16 +460,16 @@ fn combine_extras(graph: &IntermediatePetGraph) -> RequirementsTxtGraph {
let source = inverse[&source_node.version_id()];
let target = inverse[&target_node.version_id()];
// If either the existing marker or new marker is `None`, then the dependency is
// included unconditionally, and so the combined marker should be `None`.
// If either the existing marker or new marker is `true`, then the dependency is
// included unconditionally, and so the combined marker should be `true`.
if let Some(edge) = next
.find_edge(source, target)
.and_then(|edge| next.edge_weight_mut(edge))
{
if let (Some(marker), Some(ref version_marker)) = (edge.as_mut(), weight) {
marker.and(version_marker.clone());
if edge.is_true() || weight.is_true() {
*edge = MarkerTree::TRUE;
} else {
*edge = None;
edge.and(weight.clone());
}
} else {
next.update_edge(source, target, weight);

33
crates/uv-resolver/src/resolution/graph.rs
View file

@ -40,7 +40,7 @@ pub(crate) type MarkersForDistribution =
#[derive(Debug)]
pub struct ResolutionGraph {
/// The underlying graph.
pub(crate) petgraph: Graph<ResolutionGraphNode, Option<MarkerTree>, Directed>,
pub(crate) petgraph: Graph<ResolutionGraphNode, MarkerTree, Directed>,
/// The range of supported Python versions.
pub(crate) requires_python: Option<RequiresPython>,
/// If the resolution had non-identical forks, store the forks in the lockfile so we can
@ -91,7 +91,7 @@ impl ResolutionGraph {
options: Options,
) -> Result<Self, ResolveError> {
let size_guess = resolutions[0].nodes.len();
let mut petgraph: Graph<ResolutionGraphNode, Option<MarkerTree>, Directed> =
let mut petgraph: 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);
@ -163,12 +163,10 @@ impl ResolutionGraph {
// Normalize any markers.
if let Some(ref requires_python) = requires_python {
for edge in petgraph.edge_indices() {
if let Some(marker) = petgraph[edge].take() {
petgraph[edge] = Some(marker.simplify_python_versions(
Range::from(requires_python.bound_major_minor().clone()),
Range::from(requires_python.bound().clone()),
));
}
petgraph[edge] = petgraph[edge].clone().simplify_python_versions(
Range::from(requires_python.bound_major_minor().clone()),
Range::from(requires_python.bound().clone()),
);
}
}
@ -214,7 +212,7 @@ impl ResolutionGraph {
}
fn add_edge(
petgraph: &mut Graph<ResolutionGraphNode, Option<MarkerTree>>,
petgraph: &mut Graph<ResolutionGraphNode, MarkerTree>,
inverse: &mut FxHashMap<PackageRef<'_>, NodeIndex>,
root_index: NodeIndex,
edge: &ResolutionDependencyEdge,
@ -240,21 +238,16 @@ impl ResolutionGraph {
.find_edge(from_index, to_index)
.and_then(|edge| petgraph.edge_weight_mut(edge))
{
// If either the existing marker or new marker is `None`, then the dependency is
// included unconditionally, and so the combined marker should be `None`.
if let (Some(marker), Some(ref version_marker)) = (marker.as_mut(), edge.marker.clone())
{
marker.or(version_marker.clone());
} else {
*marker = None;
}
// If either the existing marker or new marker is `true`, then the dependency is
// included unconditionally, and so the combined marker is `true`.
marker.or(edge.marker.clone());
} else {
petgraph.update_edge(from_index, to_index, edge.marker.clone());
}
}
fn add_version<'a>(
petgraph: &mut Graph<ResolutionGraphNode, Option<MarkerTree>>,
petgraph: &mut Graph<ResolutionGraphNode, MarkerTree>,
inverse: &mut FxHashMap<PackageRef<'a>, NodeIndex>,
diagnostics: &mut Vec<ResolutionDiagnostic>,
preferences: &Preferences,
@ -660,7 +653,7 @@ impl From<ResolutionGraph> for distribution_types::Resolution {
/// Find any packages that don't have any lower bound on them when in resolution-lowest mode.
fn report_missing_lower_bounds(
petgraph: &Graph<ResolutionGraphNode, Option<MarkerTree>>,
petgraph: &Graph<ResolutionGraphNode, MarkerTree>,
diagnostics: &mut Vec<ResolutionDiagnostic>,
) {
for node_index in petgraph.node_indices() {
@ -687,7 +680,7 @@ fn report_missing_lower_bounds(
fn has_lower_bound(
node_index: NodeIndex,
package_name: &PackageName,
petgraph: &Graph<ResolutionGraphNode, Option<MarkerTree>>,
petgraph: &Graph<ResolutionGraphNode, MarkerTree>,
) -> bool {
for neighbor_index in petgraph.neighbors_directed(node_index, Direction::Incoming) {
let neighbor_dist = match petgraph.node_weight(neighbor_index).unwrap() {

12
crates/uv-resolver/src/resolver/mod.rs
View file

@ -670,7 +670,7 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
if let Some(ref dev) = edge.to_dev {
write!(msg, " (group: {dev})").unwrap();
}
if let Some(marker) = edge.marker.as_ref().and_then(MarkerTree::contents) {
if let Some(marker) = edge.marker.contents() {
write!(msg, " ; {marker}").unwrap();
}
trace!("Resolution: {msg}");
@ -2341,7 +2341,7 @@ impl ForkState {
// needed during resolution (which I believe is
// true), but is a more robust approach that should
// capture all cases.
marker: self.markers.fork_markers().cloned(),
marker: self.markers.fork_markers().cloned().unwrap_or_default(),
};
edges.insert(edge);
}
@ -2366,7 +2366,7 @@ impl ForkState {
to_url: to_url.cloned(),
to_extra: None,
to_dev: None,
marker: Some(dependency_marker.as_ref().clone()),
marker: MarkerTree::from(dependency_marker.clone()),
};
edges.insert(edge);
}
@ -2392,7 +2392,7 @@ impl ForkState {
to_url: to_url.cloned(),
to_extra: Some(dependency_extra.clone()),
to_dev: None,
marker: dependency_marker.clone().map(MarkerTree::from),
marker: MarkerTree::from(dependency_marker.clone()),
};
edges.insert(edge);
}
@ -2418,7 +2418,7 @@ impl ForkState {
to_url: to_url.cloned(),
to_extra: None,
to_dev: Some(dependency_dev.clone()),
marker: dependency_marker.clone().map(MarkerTree::from),
marker: MarkerTree::from(dependency_marker.clone()),
};
edges.insert(edge);
}
@ -2501,7 +2501,7 @@ pub(crate) struct ResolutionDependencyEdge {
pub(crate) to_url: Option<VerbatimParsedUrl>,
pub(crate) to_extra: Option<ExtraName>,
pub(crate) to_dev: Option<GroupName>,
pub(crate) marker: Option<MarkerTree>,
pub(crate) marker: MarkerTree,
}
impl Resolution {