wip

BurntSushi · BurntSushi · commit 79ade5674c5e · 2024-12-04T07:51:01.000-05:00
diff --git a/crates/uv-resolver/src/graph_ops.rs b/crates/uv-resolver/src/graph_ops.rs
@@ -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;
@@ -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.
     //
@@ -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);
+                    }
+                }
+            }
         }
     }
 }
diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs
@@ -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 {
@@ -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
             }
@@ -3537,7 +3537,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,
@@ -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
     }
diff --git a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_unambiguous.snap
@@ -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',
                         },
                     },
                 ],
diff --git a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_source_version_unambiguous.snap
@@ -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',
                         },
                     },
                 ],
diff --git a/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap b/crates/uv-resolver/src/lock/snapshots/uv_resolver__lock__tests__missing_dependency_version_unambiguous.snap
@@ -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',
                         },
                     },
                 ],
diff --git a/crates/uv-resolver/src/resolution/output.rs b/crates/uv-resolver/src/resolution/output.rs
@@ -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());
@@ -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;
                     }
@@ -756,8 +757,8 @@ impl ResolverOutput {
                             name: name.clone(),
                             version1: (*version1).clone(),
                             version2: (*version2).clone(),
-                            marker1: *(*marker1),
-                            marker2: *(*marker2),
+                            marker1,
+                            marker2,
                         });
                     }
                 }
diff --git a/crates/uv-resolver/src/resolution/requirements_txt.rs b/crates/uv-resolver/src/resolution/requirements_txt.rs
@@ -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 {
diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs

Original file line number	Diff line number	Diff line change
`@@ -200,7 +200,8 @@ impl ResolverOutput {`
`200`	`200`	`for weight in graph.edge_weights_mut() {`
`201`	`201`	`weight.imbibe(conflicts);`
`202`	`202`	`}`
`203`		`- simplify_conflict_markers(&mut graph);`
	`203`	`+`
	`204`	`+ simplify_conflict_markers(conflicts, &mut graph);`
`204`	`205`
`205`	`206`	`// Discard any unreachable nodes.`
`206`	`207`	`graph.retain_nodes(\|graph, node\| !graph[node].marker().is_false());`
`@@ -746,8 +747,8 @@ impl ResolverOutput {`
`746`	`747`	`}`
`747`	`748`	`let mut dupes = vec![];`
`748`	`749`	`for (name, marker_trees) in name_to_markers {`
`749`		`- for (i, (version1, marker1)) in marker_trees.iter().enumerate() {`
`750`		`- for (version2, marker2) in &marker_trees[i + 1..] {`
	`750`	`+ for (i, (version1, &marker1)) in marker_trees.iter().enumerate() {`
	`751`	`+ for (version2, &marker2) in &marker_trees[i + 1..] {`
`751`	`752`	`if version1 == version2 {`
`752`	`753`	`continue;`
`753`	`754`	`}`
`@@ -756,8 +757,8 @@ impl ResolverOutput {`
`756`	`757`	`name: name.clone(),`
`757`	`758`	`version1: (*version1).clone(),`
`758`	`759`	`version2: (*version2).clone(),`
`759`		`- marker1: (marker1),`
`760`		`- marker2: (marker2),`
	`760`	`+ marker1,`
	`761`	`+ marker2,`
`761`	`762`	`});`
`762`	`763`	`}`
`763`	`764`	`}`