mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-03 02:22:19 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Propagate fork markers to extras (#6065)

Browse source 
## Summary

When constructing the `Resolution`, we only propagated the fork markers
to the package node, but not the extras node. This led to cases in which
an extra could be included unconditionally or otherwise diverge from the
base package version.

Closes https://github.com/astral-sh/uv/issues/6062.
This commit is contained in:
Charlie Marsh 2024-08-14 09:55:39 -04:00 • committed by GitHub
parent 8c8f723005
commit 4a902a7ca1
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 88 additions and 83 deletions
Download patch file Download diff file Expand all files Collapse all files

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

@ -134,16 +134,30 @@ impl ResolutionGraph {
)?;
}
}
let mut seen = FxHashSet::default();
for resolution in resolutions {
// Add every edge to the graph.
let marker = resolution
.markers
.fork_markers()
.cloned()
.unwrap_or_default();
// Add every edge to the graph, propagating the marker for the current fork, if
// necessary.
for edge in &resolution.edges {
if !seen.insert(edge) {
if !seen.insert((edge, marker.clone())) {
// Insert each node only once.
continue;
}
Self::add_edge(&mut petgraph, &mut inverse, root_index, edge);
Self::add_edge(
&mut petgraph,
&mut inverse,
root_index,
edge,
marker.clone(),
);
}
}
@ -216,6 +230,7 @@ impl ResolutionGraph {
inverse: &mut FxHashMap<PackageRef<'_>, NodeIndex>,
root_index: NodeIndex,
edge: &ResolutionDependencyEdge,
marker: MarkerTree,
) {
let from_index = edge.from.as_ref().map_or(root_index, |from| {
inverse[&PackageRef {
@ -234,15 +249,21 @@ impl ResolutionGraph {
group: edge.to_dev.as_ref(),
}];
let edge_marker = {
let mut edge_marker = edge.marker.clone();
edge_marker.and(marker);
edge_marker
};
if let Some(marker) = petgraph
.find_edge(from_index, to_index)
.and_then(|edge| petgraph.edge_weight_mut(edge))
{
// 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());
marker.or(edge_marker);
} else {
petgraph.update_edge(from_index, to_index, edge.marker.clone());
petgraph.update_edge(from_index, to_index, edge_marker);
}
}

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

@ -2353,36 +2353,7 @@ impl ForkState {
to_url: to_url.cloned(),
to_extra: dependency_extra.clone(),
to_dev: dependency_dev.clone(),
// This propagates markers from the fork to
// packages without any markers. These might wind
// up be duplicative (and are even further merged
// via disjunction when a ResolutionGraph is
// constructed), but normalization should simplify
// most such cases.
//
// In a previous implementation of marker
// propagation, markers were propagated at the
// time a fork was created. But this was crucially
// missing a key detail: the specific version of
// a package outside of a fork can be determined
// by the forks of its dependencies, even when
// that package is not part of a fork at the time
// the forks were created. In that case, it was
// possible for two versions of the same package
// to be unconditionally included in a resolution,
// which must never be.
//
// See https://github.com/astral-sh/uv/pull/5583
// for an example of where this occurs with
// `Sphinx`.
//
// Here, instead, we do the marker propagation
// after resolution has completed. This relies
// on the fact that the markers aren't otherwise
// 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().unwrap_or_default(),
marker: MarkerTree::TRUE,
};
edges.insert(edge);
}