Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
BurntSushi committed Nov 27, 2024
1 parent bcf6f5e commit 94059e8
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 87 deletions.
4 changes: 2 additions & 2 deletions crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,10 @@ impl CandidateSelector {
// first has the matching half and then the mismatching half.
let preferences_match = preferences
.get(package_name)
.filter(|(marker, _index, _version)| env.included_by_marker(marker.pep508()));
.filter(|(marker, _index, _version)| env.included_by_marker(&marker.pep508()));
let preferences_mismatch = preferences
.get(package_name)
.filter(|(marker, _index, _version)| !env.included_by_marker(marker.pep508()));
.filter(|(marker, _index, _version)| !env.included_by_marker(&marker.pep508()));
let preferences = preferences_match.chain(preferences_mismatch).filter_map(
|(marker, source, version)| {
// If the package is mapped to an explicit index, only consider preferences that
Expand Down
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);
}
}
}
}
}
}
6 changes: 4 additions & 2 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3537,7 +3537,7 @@ impl Dependency {
complexified_marker: UniversalMarker,
) -> Dependency {
let simplified_marker =
SimplifiedMarkerTree::new(requires_python, complexified_marker.pep508().clone());
SimplifiedMarkerTree::new(requires_python, complexified_marker.combined().clone());
Dependency {
package_id,
extra,
Expand Down Expand Up @@ -3582,12 +3582,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
3 changes: 2 additions & 1 deletion 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
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 @@ -180,7 +180,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 94059e8

Please sign in to comment.