Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
BurntSushi committed Dec 3, 2024
1 parent 3381d36 commit dbe806f
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 91 deletions.
123 changes: 100 additions & 23 deletions crates/uv-resolver/src/graph_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet};
use std::collections::hash_map::Entry;

use uv_normalize::{ExtraName, PackageName};
use uv_pypi_types::Conflicts;

use crate::resolution::ResolutionGraphNode;
use crate::universal_marker::UniversalMarker;
Expand Down Expand Up @@ -88,11 +89,21 @@ pub(crate) fn marker_reachability<T>(
/// For example, given an edge like `foo[x1] -> bar`, then it is known that
/// `x1` is activated. This in turn can be used to simplify any downstream
/// conflict markers with `extra == "x1"` in them.
pub(crate) fn simplify_conflict_markers(graph: &mut Graph<ResolutionGraphNode, UniversalMarker>) {
pub(crate) fn simplify_conflict_markers(
conflicts: &Conflicts,
graph: &mut Graph<ResolutionGraphNode, UniversalMarker>,
) {
#[derive(Clone, Debug, Eq, Hash, PartialEq)]
struct Inference {
package: PackageName,
extra: ExtraName,
included: bool,
}

// The set of activated extras (and TODO, in the future, groups)
// for each node. The ROOT nodes don't have any extras activated.
let mut activated: FxHashMap<NodeIndex, FxHashSet<(PackageName, ExtraName)>> =
FxHashMap::with_capacity_and_hasher(graph.node_count(), FxBuildHasher);
let mut activated: FxHashMap<NodeIndex, Vec<FxHashSet<(PackageName, ExtraName)>>> =
FxHashMap::default();

// Collect the root nodes.
//
Expand All @@ -108,35 +119,101 @@ pub(crate) fn simplify_conflict_markers(graph: &mut Graph<ResolutionGraphNode, U
})
.collect();

let mut assume_by_edge: FxHashMap<EdgeIndex, FxHashSet<(PackageName, ExtraName)>> =
FxHashMap::default();
// let mut assume_by_edge: FxHashMap<EdgeIndex, FxHashSet<(PackageName, ExtraName)>> =
// FxHashMap::default();
let mut seen: FxHashSet<NodeIndex> = FxHashSet::default();
while let Some(parent_index) = queue.pop() {
for child_edge in graph.edges_directed(parent_index, Direction::Outgoing) {
// TODO: The below seems excessively clone-y.
// Consider tightening this up a bit.
let target = child_edge.target();
let mut extras: FxHashSet<(PackageName, ExtraName)> =
activated.get(&parent_index).cloned().unwrap_or_default();
if let Some((package, extra)) = graph[parent_index].package_extra_names() {
extras.insert((package.clone(), extra.clone()));
if let Some((package, extra)) = graph[parent_index].package_extra_names() {
for set in activated
.entry(parent_index)
.or_insert_with(|| vec![FxHashSet::default()])
{
set.insert((package.clone(), extra.clone()));
}
if let Some((package, extra)) = graph[target].package_extra_names() {
extras.insert((package.clone(), extra.clone()));
}
let sets = activated.get(&parent_index).cloned().unwrap_or_default();
for child_edge in graph.edges_directed(parent_index, Direction::Outgoing) {
for set in sets.clone() {
activated.entry(child_edge.target()).or_default().push(set);
}
activated.entry(target).or_default().extend(extras.clone());
assume_by_edge
.entry(child_edge.id())
.or_default()
.extend(extras);
if seen.insert(child_edge.target()) {
queue.push(child_edge.target());
}
}
}
for (edge_id, extras) in assume_by_edge {
for &(ref package, ref extra) in &extras {
graph[edge_id].assume_extra(package, extra);

let mut inferences: FxHashMap<NodeIndex, Vec<FxHashSet<Inference>>> = FxHashMap::default();
for (node_id, sets) in activated {
let mut new_sets = vec![];
for set in sets {
let definitive_conflict = conflicts.iter().any(|conflict_set| {
set.iter()
.filter(|(package, extra)| conflict_set.contains(package, extra))
.count()
>= 2
});
if definitive_conflict {
continue;
}

let mut new_set = FxHashSet::default();
for (package, extra) in set {
for conflict_set in conflicts.iter() {
if !conflict_set.contains(&package, &extra) {
continue;
}
for conflict_item in conflict_set.iter() {
let Some(conflict_item_extra) = conflict_item.extra() else {
continue;
};
if conflict_item.package() == &package && conflict_item_extra == &extra {
continue;
}
new_set.insert(Inference {
package: conflict_item.package().clone(),
extra: conflict_item_extra.clone(),
included: false,
});
}
}
new_set.insert(Inference {
package,
extra,
included: true,
});
}
new_sets.push(new_set);
}
inferences.insert(node_id, new_sets);
}

for edge_index in (0..graph.edge_count()).map(EdgeIndex::new) {
let (from_index, _) = graph.edge_endpoints(edge_index).unwrap();
let Some(inference) = inferences.get(&from_index) else {
continue;
};
let all_paths_satisfied = inference.iter().all(|set| {
let extras = set
.iter()
.filter_map(|inf| {
if !inf.included {
return None;
}
Some((inf.package.clone(), inf.extra.clone()))
})
.collect::<Vec<(PackageName, ExtraName)>>();
graph[edge_index].conflict().evaluate(&extras)
});
if all_paths_satisfied {
for set in inference {
for inf in set {
if inf.included {
graph[edge_index].assume_extra(&inf.package, &inf.extra);
} else {
graph[edge_index].assume_not_extra(&inf.package, &inf.extra);
}
}
}
}
}
}
14 changes: 8 additions & 6 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl Lock {
resolution
.fork_markers
.iter()
.filter(|fork_markers| !fork_markers.is_disjoint(&dist.marker))
.filter(|fork_markers| !fork_markers.is_disjoint(dist.marker))
.copied()
.collect()
} else {
Expand Down Expand Up @@ -294,16 +294,16 @@ impl Lock {
tag.starts_with(linux_tag) || tag == "linux_armv6l" || tag == "linux_armv7l"
})
}) {
!graph.graph[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.graph[node_index]
.marker()
.is_disjoint(&WINDOWS_MARKERS)
.is_disjoint(*WINDOWS_MARKERS)
} else if platform_tags.iter().all(|tag| tag.starts_with("macosx_")) {
!graph.graph[node_index].marker().is_disjoint(&MAC_MARKERS)
!graph.graph[node_index].marker().is_disjoint(*MAC_MARKERS)
} else {
true
}
Expand Down Expand Up @@ -3536,7 +3536,7 @@ impl Dependency {
complexified_marker: UniversalMarker,
) -> Dependency {
let simplified_marker =
SimplifiedMarkerTree::new(requires_python, complexified_marker.pep508());
SimplifiedMarkerTree::new(requires_python, complexified_marker.combined());
Dependency {
package_id,
extra,
Expand Down Expand Up @@ -3581,12 +3581,14 @@ impl Dependency {
if let Some(marker) = self.simplified_marker.try_to_string() {
table.insert("marker", value(marker));
}
/*
if !self.complexified_marker.conflict().is_true() {
table.insert(
"conflict-marker",
value(self.complexified_marker.conflict().to_string()),
value(self.complexified_marker.combined().to_string()),
);
}
*/

table
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ Ok(
true,
),
complexified_marker: UniversalMarker {
pep508_marker: python_full_version >= '3.12',
conflict_marker: ConflictMarker(true),
marker: python_full_version >= '3.12',
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ Ok(
true,
),
complexified_marker: UniversalMarker {
pep508_marker: python_full_version >= '3.12',
conflict_marker: ConflictMarker(true),
marker: python_full_version >= '3.12',
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,7 @@ Ok(
true,
),
complexified_marker: UniversalMarker {
pep508_marker: python_full_version >= '3.12',
conflict_marker: ConflictMarker(true),
marker: python_full_version >= '3.12',
},
},
],
Expand Down
11 changes: 6 additions & 5 deletions crates/uv-resolver/src/resolution/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,8 @@ impl ResolverOutput {
for weight in graph.edge_weights_mut() {
weight.imbibe(conflicts);
}
simplify_conflict_markers(&mut graph);

simplify_conflict_markers(conflicts, &mut graph);

// Discard any unreachable nodes.
graph.retain_nodes(|graph, node| !graph[node].marker().is_false());
Expand Down Expand Up @@ -746,8 +747,8 @@ impl ResolverOutput {
}
let mut dupes = vec![];
for (name, marker_trees) in name_to_markers {
for (i, (version1, marker1)) in marker_trees.iter().enumerate() {
for (version2, marker2) in &marker_trees[i + 1..] {
for (i, (version1, &marker1)) in marker_trees.iter().enumerate() {
for (version2, &marker2) in &marker_trees[i + 1..] {
if version1 == version2 {
continue;
}
Expand All @@ -756,8 +757,8 @@ impl ResolverOutput {
name: name.clone(),
version1: (*version1).clone(),
version2: (*version2).clone(),
marker1: *(*marker1),
marker2: *(*marker2),
marker1,
marker2,
});
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/resolution/requirements_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl<'dist> RequirementsTxtDist<'dist> {
// OK because we've asserted above that this dist
// does not have a non-trivial conflicting marker
// that we would otherwise need to care about.
markers: annotated.marker.pep508(),
markers: annotated.marker.combined(),
extras: if let Some(extra) = annotated.extra.clone() {
vec![extra]
} else {
Expand Down
Loading

0 comments on commit dbe806f

Please sign in to comment.