wip

BurntSushi · BurntSushi · commit e3ef041d27f2 · 2024-11-26T09:19:13.000-05:00
diff --git a/crates/uv-resolver/src/graph_ops.rs b/crates/uv-resolver/src/graph_ops.rs
@@ -91,8 +91,8 @@ pub(crate) fn marker_reachability<T>(
 pub(crate) fn simplify_conflict_markers(graph: &mut Graph<ResolutionGraphNode, UniversalMarker>) {
     // 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 +108,217 @@ 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);
+            // activated
+            // .entry(child_edge.target())
+            // .or_default()
+            // .extend(sets.clone());
             if seen.insert(child_edge.target()) {
                 queue.push(child_edge.target());
             }
         }
     }
+    /*
+    eprintln!("EXTRA ASSUMPTIONS");
+    for (&node_id, extras) in &activated {
+        let node = &graph[node_id];
+        eprintln!(
+            "    {} -> assuming ({})",
+            node.debug(),
+            extras
+                .iter()
+                .map(|set| {
+                    format!(
+                        "{{{}}}",
+                        set.iter()
+                            .map(|&(ref p, ref e)| format!("{p}[{e}]"))
+                            .collect::<Vec<String>>()
+                            .join(", "),
+                    )
+                })
+                .collect::<Vec<String>>()
+                .join(", ")
+        );
+    }
+    */
+    for edge_index in (0..graph.edge_count()).map(EdgeIndex::new) {
+        let (from_index, _) = graph.edge_endpoints(edge_index).unwrap();
+        let Some(activated) = activated.get(&from_index) else {
+            continue;
+        };
+        let all_paths_satisfied = activated.iter().all(|set| {
+            let extras = set
+                .iter()
+                .cloned()
+                .collect::<Vec<(PackageName, ExtraName)>>();
+            graph[edge_index].conflict().evaluate(&extras)
+        });
+        if all_paths_satisfied {
+            for set in activated {
+                for (package, extra) in set {
+                    graph[edge_index].assume_extra(package, extra);
+                }
+            }
+        }
+        /*
+        let activated = activated
+            .iter()
+            .cloned()
+            .collect::<Vec<(PackageName, ExtraName)>>();
+        if activated.iter().all(|&(ref package, ref extra)| {
+            graph[edge_index]
+                .conflict()
+                .evaluate(&[(package.clone(), extra.clone())])
+        }) {
+            for &(ref package, ref extra) in &activated {
+                graph[edge_index].assume_extra(package, extra);
+            }
+        }
+        */
+    }
+}
+
+/*
+/// Traverse the given dependency graph and propagate activated markers.
+///
+/// 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>) {
+    // Collect the root nodes.
+    //
+    // Besides the actual virtual root node, virtual dev dependencies packages are also root
+    // nodes since the edges don't cover dev dependencies.
+    let mut queue: Vec<EdgeIndex> = graph
+        .node_indices()
+        .filter(|node_index| {
+            graph
+                .edges_directed(*node_index, Direction::Incoming)
+                .next()
+                .is_none()
+        })
+        .flat_map(|node_index| graph.edges_directed(node_index, Direction::Outgoing))
+        .map(|edge| edge.id())
+        .collect();
+    // let mut seen: FxHashSet<EdgeIndex> = FxHashSet::default();
+    let mut assume_by_edge: FxHashMap<EdgeIndex, FxHashSet<(PackageName, ExtraName)>> =
+        FxHashMap::default();
+    while let Some(edge_id) = queue.pop() {
+        // OK because we know `edge_id` is valid.
+        let (_from_id, to_id) = graph.edge_endpoints(edge_id).unwrap();
+        let parent_assumptions = assume_by_edge.get(&edge_id).cloned().unwrap_or_default();
+        for child_edge in graph.edges_directed(to_id, Direction::Outgoing) {
+            let mut assumptions = assume_by_edge
+                .get(&child_edge.id())
+                .cloned()
+                .unwrap_or_default();
+            assumptions.extend(parent_assumptions.iter().cloned());
+            if let Some((package, extra)) = graph[to_id].package_extra_names() {
+                assume_by_edge
+                    .entry(child_edge.id())
+                    .or_default()
+                    .insert((package.clone(), extra.clone()));
+                assumptions.insert((package.clone(), extra.clone()));
+            }
+            if !child_edge
+                .weight()
+                .conflict()
+                .evaluate(&assumptions.iter().cloned().collect::<Vec<_>>())
+            {
+                assume_by_edge.insert(child_edge.id(), assumptions);
+            }
+            queue.push(child_edge.id());
+            // if seen.insert(child_item) {
+            // queue.push(child_item);
+            // }
+        }
+    }
+    eprintln!("EXTRA ASSUMPTIONS");
     for (edge_id, extras) in assume_by_edge {
+        let (ni1, ni2) = graph.edge_endpoints(edge_id).unwrap();
+        let (n1, n2) = (&graph[ni1], &graph[ni2]);
+        eprintln!(
+            "    ({}, {}) -> assuming {{{}}}",
+            n1.debug(),
+            n2.debug(),
+            extras
+                .iter()
+                .map(|&(ref p, ref e)| format!("{p}[{e}]"))
+                .collect::<Vec<String>>()
+                .join(", ")
+        );
         for &(ref package, ref extra) in &extras {
             graph[edge_id].assume_extra(package, extra);
         }
     }
 }
+*/
+
+/*
+pub(crate) fn simplify_conflict_markers3(graph: &mut Graph<ResolutionGraphNode, UniversalMarker>) {
+    #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
+    struct Item<'a> {
+        node_index: NodeIndex,
+        extra: Option<(&'a PackageName, &'a ExtraName)>,
+    }
+
+    // Collect the root nodes.
+    //
+    // Besides the actual virtual root node, virtual dev dependencies packages are also root
+    // nodes since the edges don't cover dev dependencies.
+    let mut queue: Vec<Item<'_>> = graph
+        .node_indices()
+        .filter(|node_index| {
+            graph
+                .edges_directed(*node_index, Direction::Incoming)
+                .next()
+                .is_none()
+        })
+        .map(|node_index| Item {
+            node_index,
+            extra: None,
+        })
+        .collect();
+    let mut seen: FxHashSet<Item<'_>> = FxHashSet::default();
+    let mut assume_by_edge: FxHashMap<EdgeIndex, FxHashSet<(PackageName, ExtraName)>> =
+        FxHashMap::default();
+    while let Some(item) = queue.pop() {
+        let parent = &graph[item.node_index];
+        for edge in graph.edges_directed(item.node_index, Direction::Outgoing) {
+            // BREADCRUMBS: OK, so maybe we do need to traverse by edge?
+            // Or just add it to `Item`? I think the issue here is that
+            // we need to union the `assume_by_edge` sets as we traverse
+            // the graph.
+            if let Some((package, extra)) = parent.package_extra_names() {
+                assume_by_edge
+                    .entry(edge.id())
+                    .or_default()
+                    .insert((package.clone(), extra.clone()));
+            }
+            let child_item = Item {
+                node_index: edge.target(),
+                extra: parent.package_extra_names(),
+            };
+            if seen.insert(child_item) {
+                queue.push(child_item);
+            }
+        }
+    }
+}
+*/
diff --git a/crates/uv-resolver/src/resolution/output.rs b/crates/uv-resolver/src/resolution/output.rs
@@ -34,6 +34,8 @@ use crate::{
     VersionsResponse,
 };
 
+static DEBUG: bool = false;
+
 /// The output of a successful resolution.
 ///
 /// Includes a complete resolution graph in which every node represents a pinned package and every
@@ -82,6 +84,29 @@ impl ResolutionGraphNode {
             }
         }
     }
+
+    #[allow(dead_code)]
+    pub(crate) fn debug(&self) -> String {
+        match *self {
+            ResolutionGraphNode::Root => "ROOT".to_string(),
+            ResolutionGraphNode::Dist(ref dist) => {
+                use std::fmt::Write;
+                let mut buf = String::new();
+                write!(buf, "{}", dist.name).unwrap();
+                if let Some(ref extra) = dist.extra {
+                    write!(buf, "[{extra}]").unwrap();
+                }
+                if let Some(ref group) = dist.dev {
+                    write!(buf, ":{group}").unwrap();
+                }
+                write!(buf, "=={}", dist.version).unwrap();
+                if !dist.marker.is_true() {
+                    write!(buf, "{{{}}}", dist.marker).unwrap();
+                }
+                buf
+            }
+        }
+    }
 }
 
 impl Display for ResolutionGraphNode {
@@ -118,6 +143,43 @@ impl ResolverOutput {
         resolution_strategy: &ResolutionStrategy,
         options: Options,
     ) -> Result<Self, ResolveError> {
+        if DEBUG {
+            for resolution in resolutions {
+                dbg!(&resolution.env);
+                eprintln!("  NODES");
+                for (node, version) in &resolution.nodes {
+                    eprintln!(
+                        "    name: {}, version: {}, extra: {:?}, dev: {:?}",
+                        node.name, version, node.extra, node.dev,
+                    );
+                }
+                eprintln!("  EDGES");
+                for edge in &resolution.edges {
+                    let from_name = edge
+                        .from
+                        .as_ref()
+                        .map(|n| n.to_string())
+                        .unwrap_or("ROOT".to_string());
+                    eprintln!(
+                        "    {}=={}{} -> {}=={}{}",
+                        from_name,
+                        edge.from_version,
+                        edge.from_extra
+                            .as_ref()
+                            .map(|x| format!("[{}]", x))
+                            .unwrap_or(String::new()),
+                        edge.to,
+                        edge.to_version,
+                        edge.to_extra
+                            .as_ref()
+                            .map(|x| format!("[{}]", x))
+                            .unwrap_or(String::new()),
+                    );
+                }
+            }
+            eprintln!("------------------------------------");
+        }
+
         let size_guess = resolutions[0].nodes.len();
         let mut graph: Graph<ResolutionGraphNode, UniversalMarker, Directed> =
             Graph::with_capacity(size_guess, size_guess);
@@ -186,6 +248,19 @@ impl ResolverOutput {
                 .collect()
         };
 
+        if DEBUG {
+            dbg!(&fork_markers);
+            for ni in graph.node_indices() {
+                let n1 = &graph[ni];
+                for edge in graph.edges(ni) {
+                    use petgraph::visit::EdgeRef;
+                    let n2 = &graph[edge.target()];
+                    let weight = edge.weight();
+                    eprintln!("    ({}, {}) -> {weight}", n1.debug(), n2.debug());
+                }
+            }
+        }
+
         // Compute and apply the marker reachability.
         let mut reachability = marker_reachability(&graph, &fork_markers);
 
@@ -200,11 +275,38 @@ impl ResolverOutput {
         for weight in graph.edge_weights_mut() {
             weight.imbibe(conflicts);
         }
+
+        if DEBUG {
+            eprintln!("AFTER REACHABILITY AND IMBIBING");
+            for ni in graph.node_indices() {
+                let n1 = &graph[ni];
+                for edge in graph.edges(ni) {
+                    use petgraph::visit::EdgeRef;
+                    let n2 = &graph[edge.target()];
+                    let weight = edge.weight();
+                    eprintln!("    ({}, {}) -> {weight}", n1.debug(), n2.debug());
+                }
+            }
+        }
+
         simplify_conflict_markers(&mut graph);
 
         // Discard any unreachable nodes.
         graph.retain_nodes(|graph, node| !graph[node].marker().is_false());
 
+        if DEBUG {
+            eprintln!("AFTER SIMPLIFICATION");
+            for ni in graph.node_indices() {
+                let n1 = &graph[ni];
+                for edge in graph.edges(ni) {
+                    use petgraph::visit::EdgeRef;
+                    let n2 = &graph[edge.target()];
+                    let weight = edge.weight();
+                    eprintln!("    ({}, {}) -> {weight}", n1.debug(), n2.debug());
+                }
+            }
+        }
+
         if matches!(resolution_strategy, ResolutionStrategy::Lowest) {
             report_missing_lower_bounds(&graph, &mut diagnostics);
         }
diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs
