From e0e123873ba5060fd3b38c8f1c11b9cc551c4874 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Fri, 3 May 2024 19:57:47 +0800 Subject: [PATCH] revset_graph: rename to `graph` and make generic over graph node type --- cli/src/commands/log.rs | 14 +- lib/src/default_index/revset_engine.rs | 6 +- .../default_index/revset_graph_iterator.rs | 26 +- lib/src/{revset_graph.rs => graph.rs} | 518 +++++++++--------- lib/src/lib.rs | 2 +- lib/src/revset.rs | 4 +- .../test_default_revset_graph_iterator.rs | 15 +- lib/tests/test_revset.rs | 20 +- 8 files changed, 300 insertions(+), 305 deletions(-) rename lib/src/{revset_graph.rs => graph.rs} (73%) diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index f67d6853dc..d1afd286db 100644 --- a/cli/src/commands/log.rs +++ b/cli/src/commands/log.rs @@ -13,11 +13,9 @@ // limitations under the License. use jj_lib::backend::CommitId; +use jj_lib::graph::{GraphEdgeType, ReverseGraphIterator, TopoGroupedGraphIterator}; use jj_lib::repo::Repo; use jj_lib::revset::{RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt}; -use jj_lib::revset_graph::{ - ReverseRevsetGraphIterator, RevsetGraphEdgeType, TopoGroupedRevsetGraphIterator, -}; use tracing::instrument; use crate::cli_util::{format_template, CommandHelper, LogContentFormat, RevisionArg}; @@ -141,9 +139,9 @@ pub(crate) fn cmd_log( if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); - let forward_iter = TopoGroupedRevsetGraphIterator::new(revset.iter_graph()); + let forward_iter = TopoGroupedGraphIterator::new(revset.iter_graph()); let iter: Box> = if args.reversed { - Box::new(ReverseRevsetGraphIterator::new(forward_iter)) + Box::new(ReverseGraphIterator::new(forward_iter)) } else { Box::new(forward_iter) }; @@ -157,13 +155,13 @@ pub(crate) fn cmd_log( let mut elided_targets = vec![]; for edge in edges { match edge.edge_type { - RevsetGraphEdgeType::Missing => { + GraphEdgeType::Missing => { has_missing = true; } - RevsetGraphEdgeType::Direct => { + GraphEdgeType::Direct => { graphlog_edges.push(Edge::Direct((edge.target, false))); } - RevsetGraphEdgeType::Indirect => { + GraphEdgeType::Indirect => { if use_elided_nodes { elided_targets.push(edge.target.clone()); graphlog_edges.push(Edge::Direct((edge.target, true))); diff --git a/lib/src/default_index/revset_engine.rs b/lib/src/default_index/revset_engine.rs index b9e18e6532..9afbbdd9bc 100644 --- a/lib/src/default_index/revset_engine.rs +++ b/lib/src/default_index/revset_engine.rs @@ -28,13 +28,13 @@ use super::rev_walk::{EagerRevWalk, PeekableRevWalk, RevWalk, RevWalkBuilder}; use super::revset_graph_iterator::RevsetGraphWalk; use crate::backend::{ChangeId, CommitId, MillisSinceEpoch}; use crate::default_index::{AsCompositeIndex, CompositeIndex, IndexEntry, IndexPosition}; +use crate::graph::GraphEdge; use crate::matchers::{Matcher, Visit}; use crate::repo_path::RepoPath; use crate::revset::{ ResolvedExpression, ResolvedPredicateExpression, Revset, RevsetEvaluationError, RevsetFilterExtensionWrapper, RevsetFilterPredicate, GENERATION_RANGE_FULL, }; -use crate::revset_graph::RevsetGraphEdge; use crate::store::Store; use crate::{rewrite, union_find}; @@ -103,7 +103,7 @@ impl RevsetImpl { pub fn iter_graph_impl( &self, skip_transitive_edges: bool, - ) -> impl Iterator)> { + ) -> impl Iterator>)> { let index = self.index.clone(); let walk = self.inner.positions(); let mut graph_walk = RevsetGraphWalk::new(walk, skip_transitive_edges); @@ -147,7 +147,7 @@ impl Revset for RevsetImpl { })) } - fn iter_graph<'a>(&self) -> Box)> + 'a> + fn iter_graph<'a>(&self) -> Box>)> + 'a> where Self: 'a, { diff --git a/lib/src/default_index/revset_graph_iterator.rs b/lib/src/default_index/revset_graph_iterator.rs index e88bb62007..304f0cfc0e 100644 --- a/lib/src/default_index/revset_graph_iterator.rs +++ b/lib/src/default_index/revset_graph_iterator.rs @@ -22,7 +22,7 @@ use super::entry::{IndexEntry, IndexPosition}; use super::rev_walk::RevWalk; use super::revset_engine::BoxedRevWalk; use crate::backend::CommitId; -use crate::revset_graph::{RevsetGraphEdge, RevsetGraphEdgeType}; +use crate::graph::{GraphEdge, GraphEdgeType}; /// Like `RevsetGraphEdge`, but stores `IndexPosition` instead. /// @@ -30,27 +30,27 @@ use crate::revset_graph::{RevsetGraphEdge, RevsetGraphEdgeType}; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] struct IndexGraphEdge { target: IndexPosition, - edge_type: RevsetGraphEdgeType, + edge_type: GraphEdgeType, } impl IndexGraphEdge { fn missing(target: IndexPosition) -> Self { - let edge_type = RevsetGraphEdgeType::Missing; + let edge_type = GraphEdgeType::Missing; IndexGraphEdge { target, edge_type } } fn direct(target: IndexPosition) -> Self { - let edge_type = RevsetGraphEdgeType::Direct; + let edge_type = GraphEdgeType::Direct; IndexGraphEdge { target, edge_type } } fn indirect(target: IndexPosition) -> Self { - let edge_type = RevsetGraphEdgeType::Indirect; + let edge_type = GraphEdgeType::Indirect; IndexGraphEdge { target, edge_type } } - fn to_revset_edge(self, index: &CompositeIndex) -> RevsetGraphEdge { - RevsetGraphEdge { + fn to_revset_edge(self, index: &CompositeIndex) -> GraphEdge { + GraphEdge { target: index.entry_by_pos(self.target).commit_id(), edge_type: self.edge_type, } @@ -200,7 +200,7 @@ impl<'a> RevsetGraphWalk<'a> { let parent_edges = self.edges_from_external_commit(index, parent); if parent_edges .iter() - .all(|edge| edge.edge_type == RevsetGraphEdgeType::Missing) + .all(|edge| edge.edge_type == GraphEdgeType::Missing) { edges.push(IndexGraphEdge::missing(parent_position)); } else { @@ -240,7 +240,7 @@ impl<'a> RevsetGraphWalk<'a> { } else if let Some(parent_edges) = self.edges.get(&parent_position) { if parent_edges .iter() - .all(|edge| edge.edge_type == RevsetGraphEdgeType::Missing) + .all(|edge| edge.edge_type == GraphEdgeType::Missing) { edges.push(IndexGraphEdge::missing(parent_position)); } else { @@ -275,7 +275,7 @@ impl<'a> RevsetGraphWalk<'a> { ) -> Vec { if !edges .iter() - .any(|edge| edge.edge_type == RevsetGraphEdgeType::Indirect) + .any(|edge| edge.edge_type == GraphEdgeType::Indirect) { return edges; } @@ -285,7 +285,7 @@ impl<'a> RevsetGraphWalk<'a> { // To start with, add the edges one step after the input edges. for edge in &edges { initial_targets.insert(edge.target); - if edge.edge_type != RevsetGraphEdgeType::Missing { + if edge.edge_type != GraphEdgeType::Missing { assert!(self.look_ahead.contains(&edge.target)); let entry = index.entry_by_pos(edge.target); min_generation = min(min_generation, entry.generation_number()); @@ -295,7 +295,7 @@ impl<'a> RevsetGraphWalk<'a> { // Find commits reachable transitively and add them to the `unwanted` set. let mut unwanted = HashSet::new(); while let Some(edge) = work.pop() { - if edge.edge_type == RevsetGraphEdgeType::Missing || edge.target < self.min_position { + if edge.edge_type == GraphEdgeType::Missing || edge.target < self.min_position { continue; } if !unwanted.insert(edge.target) { @@ -333,7 +333,7 @@ impl<'a> RevsetGraphWalk<'a> { } impl RevWalk for RevsetGraphWalk<'_> { - type Item = (CommitId, Vec); + type Item = (CommitId, Vec>); fn next(&mut self, index: &CompositeIndex) -> Option { let position = self.next_index_position(index)?; diff --git a/lib/src/revset_graph.rs b/lib/src/graph.rs similarity index 73% rename from lib/src/revset_graph.rs rename to lib/src/graph.rs index ff3c4b72ff..4f9ef678a6 100644 --- a/lib/src/revset_graph.rs +++ b/lib/src/graph.rs @@ -15,84 +15,83 @@ #![allow(missing_docs)] use std::collections::{HashMap, HashSet, VecDeque}; - -use crate::backend::CommitId; +use std::hash::Hash; #[derive(Debug, PartialEq, Eq, Clone, Hash)] -pub struct RevsetGraphEdge { - pub target: CommitId, - pub edge_type: RevsetGraphEdgeType, +pub struct GraphEdge { + pub target: N, + pub edge_type: GraphEdgeType, } -impl RevsetGraphEdge { - pub fn missing(target: CommitId) -> Self { +impl GraphEdge { + pub fn missing(target: N) -> Self { Self { target, - edge_type: RevsetGraphEdgeType::Missing, + edge_type: GraphEdgeType::Missing, } } - pub fn direct(target: CommitId) -> Self { + pub fn direct(target: N) -> Self { Self { target, - edge_type: RevsetGraphEdgeType::Direct, + edge_type: GraphEdgeType::Direct, } } - pub fn indirect(target: CommitId) -> Self { + pub fn indirect(target: N) -> Self { Self { target, - edge_type: RevsetGraphEdgeType::Indirect, + edge_type: GraphEdgeType::Indirect, } } } #[derive(Debug, PartialEq, Eq, Clone, Copy, Hash)] -pub enum RevsetGraphEdgeType { +pub enum GraphEdgeType { Missing, Direct, Indirect, } -fn reachable_targets(edges: &[RevsetGraphEdge]) -> impl DoubleEndedIterator { +fn reachable_targets(edges: &[GraphEdge]) -> impl DoubleEndedIterator { edges .iter() - .filter(|edge| edge.edge_type != RevsetGraphEdgeType::Missing) + .filter(|edge| edge.edge_type != GraphEdgeType::Missing) .map(|edge| &edge.target) } -pub struct ReverseRevsetGraphIterator { - items: Vec<(CommitId, Vec)>, +pub struct ReverseGraphIterator { + items: Vec<(N, Vec>)>, } -impl ReverseRevsetGraphIterator { - pub fn new(input: impl IntoIterator)>) -> Self { +impl ReverseGraphIterator +where + N: Hash + Eq + Clone, +{ + pub fn new(input: impl IntoIterator>)>) -> Self { let mut entries = vec![]; - let mut reverse_edges: HashMap> = HashMap::new(); - for (commit_id, edges) in input { - for RevsetGraphEdge { target, edge_type } in edges { - reverse_edges - .entry(target) - .or_default() - .push(RevsetGraphEdge { - target: commit_id.clone(), - edge_type, - }) + let mut reverse_edges: HashMap>> = HashMap::new(); + for (node, edges) in input { + for GraphEdge { target, edge_type } in edges { + reverse_edges.entry(target).or_default().push(GraphEdge { + target: node.clone(), + edge_type, + }) } - entries.push(commit_id); + entries.push(node); } let mut items = vec![]; - for commit_id in entries.into_iter() { - let edges = reverse_edges.get(&commit_id).cloned().unwrap_or_default(); - items.push((commit_id, edges)); + for node in entries.into_iter() { + let edges = reverse_edges.get(&node).cloned().unwrap_or_default(); + items.push((node, edges)); } Self { items } } } -impl Iterator for ReverseRevsetGraphIterator { - type Item = (CommitId, Vec); +impl Iterator for ReverseGraphIterator { + type Item = (N, Vec>); fn next(&mut self) -> Option { self.items.pop() @@ -111,34 +110,44 @@ impl Iterator for ReverseRevsetGraphIterator { /// /// [Git]: https://github.blog/2022-08-30-gits-database-internals-ii-commit-history-queries/#topological-sorting #[derive(Clone, Debug)] -pub struct TopoGroupedRevsetGraphIterator { +pub struct TopoGroupedGraphIterator { input_iter: I, /// Graph nodes read from the input iterator but not yet emitted. - nodes: HashMap, + nodes: HashMap>, /// Stack of graph nodes to be emitted. - emittable_ids: Vec, + emittable_ids: Vec, /// List of new head nodes found while processing unpopulated nodes. - new_head_ids: VecDeque, + new_head_ids: VecDeque, /// Set of nodes which may be ancestors of `new_head_ids`. - blocked_ids: HashSet, + blocked_ids: HashSet, } -#[derive(Clone, Debug, Default)] -struct TopoGroupedGraphNode { +#[derive(Clone, Debug)] +struct TopoGroupedGraphNode { /// Graph nodes which must be emitted before. - child_ids: HashSet, + child_ids: HashSet, /// Graph edges to parent nodes. `None` until this node is populated. - edges: Option>, + edges: Option>>, } -impl TopoGroupedRevsetGraphIterator +impl Default for TopoGroupedGraphNode { + fn default() -> Self { + Self { + child_ids: Default::default(), + edges: None, + } + } +} + +impl TopoGroupedGraphIterator where - I: Iterator)>, + N: Clone + Hash + Eq, + I: Iterator>)>, { /// Wraps the given iterator to group topological branches. The input /// iterator must be topologically ordered. pub fn new(input_iter: I) -> Self { - TopoGroupedRevsetGraphIterator { + TopoGroupedGraphIterator { input_iter, nodes: HashMap::new(), emittable_ids: Vec::new(), @@ -189,7 +198,7 @@ where } // Mark descendant nodes reachable from the blocking nodes - let mut to_visit: Vec<&CommitId> = self + let mut to_visit: Vec<&N> = self .blocked_ids .iter() .map(|id| { @@ -198,7 +207,7 @@ where id }) .collect(); - let mut visited: HashSet<&CommitId> = to_visit.iter().copied().collect(); + let mut visited: HashSet<&N> = to_visit.iter().copied().collect(); while let Some(id) = to_visit.pop() { let node = self.nodes.get(id).unwrap(); to_visit.extend(node.child_ids.iter().filter(|id| visited.insert(id))); @@ -229,7 +238,7 @@ where } #[must_use] - fn next_node(&mut self) -> Option<(CommitId, Vec)> { + fn next_node(&mut self) -> Option<(N, Vec>)> { // Based on Kahn's algorithm loop { if let Some(current_id) = self.emittable_ids.last() { @@ -274,11 +283,12 @@ where } } -impl Iterator for TopoGroupedRevsetGraphIterator +impl Iterator for TopoGroupedGraphIterator where - I: Iterator)>, + N: Clone + Hash + Eq, + I: Iterator>)>, { - type Item = (CommitId, Vec); + type Item = (N, Vec>); fn next(&mut self) -> Option { if let Some(node) = self.next_node() { @@ -296,37 +306,29 @@ mod tests { use renderdag::{Ancestor, GraphRowRenderer, Renderer as _}; use super::*; - use crate::object_id::ObjectId; - - fn id(c: char) -> CommitId { - let d = u8::try_from(c).unwrap(); - CommitId::new(vec![d]) - } - fn missing(c: char) -> RevsetGraphEdge { - RevsetGraphEdge::missing(id(c)) + fn missing(c: char) -> GraphEdge { + GraphEdge::missing(c) } - fn direct(c: char) -> RevsetGraphEdge { - RevsetGraphEdge::direct(id(c)) + fn direct(c: char) -> GraphEdge { + GraphEdge::direct(c) } - fn indirect(c: char) -> RevsetGraphEdge { - RevsetGraphEdge::indirect(id(c)) + fn indirect(c: char) -> GraphEdge { + GraphEdge::indirect(c) } - fn format_edge(edge: &RevsetGraphEdge) -> String { - let c = char::from(edge.target.as_bytes()[0]); + fn format_edge(edge: &GraphEdge) -> String { + let c = edge.target; match edge.edge_type { - RevsetGraphEdgeType::Missing => format!("missing({c})"), - RevsetGraphEdgeType::Direct => format!("direct({c})"), - RevsetGraphEdgeType::Indirect => format!("indirect({c})"), + GraphEdgeType::Missing => format!("missing({c})"), + GraphEdgeType::Direct => format!("direct({c})"), + GraphEdgeType::Indirect => format!("indirect({c})"), } } - fn format_graph( - graph_iter: impl IntoIterator)>, - ) -> String { + fn format_graph(graph_iter: impl IntoIterator>)>) -> String { let mut renderer = GraphRowRenderer::new() .output() .with_min_row_height(2) @@ -334,14 +336,14 @@ mod tests { graph_iter .into_iter() .map(|(id, edges)| { - let glyph = char::from(id.as_bytes()[0]).to_string(); + let glyph = id.to_string(); let message = edges.iter().map(format_edge).join(", "); let parents = edges .into_iter() .map(|edge| match edge.edge_type { - RevsetGraphEdgeType::Missing => Ancestor::Anonymous, - RevsetGraphEdgeType::Direct => Ancestor::Parent(edge.target), - RevsetGraphEdgeType::Indirect => Ancestor::Ancestor(edge.target), + GraphEdgeType::Missing => Ancestor::Anonymous, + GraphEdgeType::Direct => Ancestor::Parent(edge.target), + GraphEdgeType::Indirect => Ancestor::Ancestor(edge.target), }) .collect(); renderer.next_row(id, parents, glyph, message) @@ -352,10 +354,10 @@ mod tests { #[test] fn test_format_graph() { let graph = vec![ - (id('D'), vec![direct('C'), indirect('B')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![missing('X')]), - (id('A'), vec![]), + ('D', vec![direct('C'), indirect('B')]), + ('C', vec![direct('A')]), + ('B', vec![missing('X')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph), @r###" D direct(C), indirect(B) @@ -371,19 +373,19 @@ mod tests { "###); } - fn topo_grouped(graph_iter: I) -> TopoGroupedRevsetGraphIterator + fn topo_grouped(graph_iter: I) -> TopoGroupedGraphIterator where - I: IntoIterator)>, + I: IntoIterator>)>, { - TopoGroupedRevsetGraphIterator::new(graph_iter.into_iter()) + TopoGroupedGraphIterator::new(graph_iter.into_iter()) } #[test] fn test_topo_grouped_multiple_roots() { let graph = [ - (id('C'), vec![missing('Y')]), - (id('B'), vec![missing('X')]), - (id('A'), vec![]), + ('C', vec![missing('Y')]), + ('B', vec![missing('X')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" C missing(Y) @@ -410,20 +412,20 @@ mod tests { // All nodes can be lazily emitted. let mut iter = topo_grouped(graph.iter().cloned().peekable()); - assert_eq!(iter.next().unwrap().0, id('C')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('B')); - assert_eq!(iter.next().unwrap().0, id('B')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('A')); + assert_eq!(iter.next().unwrap().0, 'C'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'B'); + assert_eq!(iter.next().unwrap().0, 'B'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'A'); } #[test] fn test_topo_grouped_trivial_fork() { - let graph = vec![ - (id('E'), vec![direct('B')]), - (id('D'), vec![direct('A')]), - (id('C'), vec![direct('B')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + let graph = [ + ('E', vec![direct('B')]), + ('D', vec![direct('A')]), + ('C', vec![direct('B')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" E direct(B) @@ -454,23 +456,23 @@ mod tests { // E can be lazy, then D and C will be queued. let mut iter = topo_grouped(graph.iter().cloned().peekable()); - assert_eq!(iter.next().unwrap().0, id('E')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('D')); - assert_eq!(iter.next().unwrap().0, id('C')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('B')); - assert_eq!(iter.next().unwrap().0, id('B')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('A')); + assert_eq!(iter.next().unwrap().0, 'E'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'D'); + assert_eq!(iter.next().unwrap().0, 'C'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'B'); + assert_eq!(iter.next().unwrap().0, 'B'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'A'); } #[test] fn test_topo_grouped_fork_interleaved() { - let graph = vec![ - (id('F'), vec![direct('D')]), - (id('E'), vec![direct('C')]), - (id('D'), vec![direct('B')]), - (id('C'), vec![direct('B')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + let graph = [ + ('F', vec![direct('D')]), + ('E', vec![direct('C')]), + ('D', vec![direct('B')]), + ('C', vec![direct('B')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" F direct(D) @@ -503,26 +505,26 @@ mod tests { // F can be lazy, then E will be queued, then C. let mut iter = topo_grouped(graph.iter().cloned().peekable()); - assert_eq!(iter.next().unwrap().0, id('F')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('E')); - assert_eq!(iter.next().unwrap().0, id('D')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('C')); - assert_eq!(iter.next().unwrap().0, id('E')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('B')); + assert_eq!(iter.next().unwrap().0, 'F'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'E'); + assert_eq!(iter.next().unwrap().0, 'D'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'C'); + assert_eq!(iter.next().unwrap().0, 'E'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'B'); } #[test] fn test_topo_grouped_fork_multiple_heads() { let graph = vec![ - (id('I'), vec![direct('E')]), - (id('H'), vec![direct('C')]), - (id('G'), vec![direct('A')]), - (id('F'), vec![direct('E')]), - (id('E'), vec![direct('C')]), - (id('D'), vec![direct('C')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('I', vec![direct('E')]), + ('H', vec![direct('C')]), + ('G', vec![direct('A')]), + ('F', vec![direct('E')]), + ('E', vec![direct('C')]), + ('D', vec![direct('C')]), + ('C', vec![direct('A')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" I direct(E) @@ -567,28 +569,28 @@ mod tests { // I can be lazy, then H, G, and F will be queued. let mut iter = topo_grouped(graph.iter().cloned().peekable()); - assert_eq!(iter.next().unwrap().0, id('I')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('H')); - assert_eq!(iter.next().unwrap().0, id('F')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('E')); + assert_eq!(iter.next().unwrap().0, 'I'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'H'); + assert_eq!(iter.next().unwrap().0, 'F'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'E'); } #[test] fn test_topo_grouped_fork_parallel() { let graph = vec![ // Pull all sub graphs in reverse order: - (id('I'), vec![direct('A')]), - (id('H'), vec![direct('C')]), - (id('G'), vec![direct('E')]), + ('I', vec![direct('A')]), + ('H', vec![direct('C')]), + ('G', vec![direct('E')]), // Orphan sub graph G,F-E: - (id('F'), vec![direct('E')]), - (id('E'), vec![missing('Y')]), + ('F', vec![direct('E')]), + ('E', vec![missing('Y')]), // Orphan sub graph H,D-C: - (id('D'), vec![direct('C')]), - (id('C'), vec![missing('X')]), + ('D', vec![direct('C')]), + ('C', vec![missing('X')]), // Orphan sub graph I,B-A: - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" I direct(A) @@ -643,23 +645,23 @@ mod tests { fn test_topo_grouped_fork_nested() { fn sub_graph( chars: impl IntoIterator, - root_edges: Vec, - ) -> Vec<(CommitId, Vec)> { + root_edges: Vec>, + ) -> Vec<(char, Vec>)> { let [b, c, d, e, f]: [char; 5] = chars.into_iter().collect_vec().try_into().unwrap(); vec![ - (id(f), vec![direct(c)]), - (id(e), vec![direct(b)]), - (id(d), vec![direct(c)]), - (id(c), vec![direct(b)]), - (id(b), root_edges), + (f, vec![direct(c)]), + (e, vec![direct(b)]), + (d, vec![direct(c)]), + (c, vec![direct(b)]), + (b, root_edges), ] } // One nested fork sub graph from A let graph = itertools::chain!( - vec![(id('G'), vec![direct('A')])], + vec![('G', vec![direct('A')])], sub_graph('B'..='F', vec![direct('A')]), - vec![(id('A'), vec![])], + vec![('A', vec![])], ) .collect_vec(); insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" @@ -698,10 +700,10 @@ mod tests { // Two nested fork sub graphs from A let graph = itertools::chain!( - vec![(id('L'), vec![direct('A')])], + vec![('L', vec![direct('A')])], sub_graph('G'..='K', vec![direct('A')]), sub_graph('B'..='F', vec![direct('A')]), - vec![(id('A'), vec![])], + vec![('A', vec![])], ) .collect_vec(); insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" @@ -760,10 +762,10 @@ mod tests { // Two nested fork sub graphs from A, interleaved let graph = itertools::chain!( - vec![(id('L'), vec![direct('A')])], + vec![('L', vec![direct('A')])], sub_graph(['C', 'E', 'G', 'I', 'K'], vec![direct('A')]), sub_graph(['B', 'D', 'F', 'H', 'J'], vec![direct('A')]), - vec![(id('A'), vec![])], + vec![('A', vec![])], ) .sorted_by(|(id1, _), (id2, _)| id2.cmp(id1)) .collect_vec(); @@ -823,7 +825,7 @@ mod tests { // Merged fork sub graphs at K let graph = itertools::chain!( - vec![(id('K'), vec![direct('E'), direct('J')])], + vec![('K', vec![direct('E'), direct('J')])], sub_graph('F'..='J', vec![missing('Y')]), sub_graph('A'..='E', vec![missing('X')]), ) @@ -887,7 +889,7 @@ mod tests { // Merged fork sub graphs at K, interleaved let graph = itertools::chain!( - vec![(id('K'), vec![direct('I'), direct('J')])], + vec![('K', vec![direct('I'), direct('J')])], sub_graph(['B', 'D', 'F', 'H', 'J'], vec![missing('Y')]), sub_graph(['A', 'C', 'E', 'G', 'I'], vec![missing('X')]), ) @@ -953,13 +955,13 @@ mod tests { #[test] fn test_topo_grouped_merge_interleaved() { - let graph = vec![ - (id('F'), vec![direct('E')]), - (id('E'), vec![direct('C'), direct('D')]), - (id('D'), vec![direct('B')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + let graph = [ + ('F', vec![direct('E')]), + ('E', vec![direct('C'), direct('D')]), + ('D', vec![direct('B')]), + ('C', vec![direct('A')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" F direct(E) @@ -992,24 +994,24 @@ mod tests { // F, E, and D can be lazy, then C will be queued, then B. let mut iter = topo_grouped(graph.iter().cloned().peekable()); - assert_eq!(iter.next().unwrap().0, id('F')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('E')); - assert_eq!(iter.next().unwrap().0, id('E')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('D')); - assert_eq!(iter.next().unwrap().0, id('D')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('C')); - assert_eq!(iter.next().unwrap().0, id('B')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('A')); + assert_eq!(iter.next().unwrap().0, 'F'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'E'); + assert_eq!(iter.next().unwrap().0, 'E'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'D'); + assert_eq!(iter.next().unwrap().0, 'D'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'C'); + assert_eq!(iter.next().unwrap().0, 'B'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'A'); } #[test] fn test_topo_grouped_merge_but_missing() { - let graph = vec![ - (id('E'), vec![direct('D')]), - (id('D'), vec![missing('Y'), direct('C')]), - (id('C'), vec![direct('B'), missing('X')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + let graph = [ + ('E', vec![direct('D')]), + ('D', vec![missing('Y'), direct('C')]), + ('C', vec![direct('B'), missing('X')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" E direct(D) @@ -1050,26 +1052,26 @@ mod tests { // All nodes can be lazily emitted. let mut iter = topo_grouped(graph.iter().cloned().peekable()); - assert_eq!(iter.next().unwrap().0, id('E')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('D')); - assert_eq!(iter.next().unwrap().0, id('D')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('C')); - assert_eq!(iter.next().unwrap().0, id('C')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('B')); - assert_eq!(iter.next().unwrap().0, id('B')); - assert_eq!(iter.input_iter.peek().unwrap().0, id('A')); + assert_eq!(iter.next().unwrap().0, 'E'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'D'); + assert_eq!(iter.next().unwrap().0, 'D'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'C'); + assert_eq!(iter.next().unwrap().0, 'C'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'B'); + assert_eq!(iter.next().unwrap().0, 'B'); + assert_eq!(iter.input_iter.peek().unwrap().0, 'A'); } #[test] fn test_topo_grouped_merge_criss_cross() { let graph = vec![ - (id('G'), vec![direct('E')]), - (id('F'), vec![direct('D')]), - (id('E'), vec![direct('B'), direct('C')]), - (id('D'), vec![direct('B'), direct('C')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('G', vec![direct('E')]), + ('F', vec![direct('D')]), + ('E', vec![direct('B'), direct('C')]), + ('D', vec![direct('B'), direct('C')]), + ('C', vec![direct('A')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" G direct(E) @@ -1108,14 +1110,14 @@ mod tests { #[test] fn test_topo_grouped_merge_descendants_interleaved() { let graph = vec![ - (id('H'), vec![direct('F')]), - (id('G'), vec![direct('E')]), - (id('F'), vec![direct('D')]), - (id('E'), vec![direct('C')]), - (id('D'), vec![direct('C'), direct('B')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('H', vec![direct('F')]), + ('G', vec![direct('E')]), + ('F', vec![direct('D')]), + ('E', vec![direct('C')]), + ('D', vec![direct('C'), direct('B')]), + ('C', vec![direct('A')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" H direct(F) @@ -1158,10 +1160,10 @@ mod tests { #[test] fn test_topo_grouped_merge_multiple_roots() { let graph = [ - (id('D'), vec![direct('C')]), - (id('C'), vec![direct('B'), direct('A')]), - (id('B'), vec![missing('X')]), - (id('A'), vec![]), + ('D', vec![direct('C')]), + ('C', vec![direct('B'), direct('A')]), + ('B', vec![missing('X')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" D direct(C) @@ -1193,18 +1195,18 @@ mod tests { fn test_topo_grouped_merge_stairs() { let graph = vec![ // Merge topic branches one by one: - (id('J'), vec![direct('I'), direct('G')]), - (id('I'), vec![direct('H'), direct('E')]), - (id('H'), vec![direct('D'), direct('F')]), + ('J', vec![direct('I'), direct('G')]), + ('I', vec![direct('H'), direct('E')]), + ('H', vec![direct('D'), direct('F')]), // Topic branches: - (id('G'), vec![direct('D')]), - (id('F'), vec![direct('C')]), - (id('E'), vec![direct('B')]), + ('G', vec![direct('D')]), + ('F', vec![direct('C')]), + ('E', vec![direct('B')]), // Base nodes: - (id('D'), vec![direct('C')]), - (id('C'), vec![direct('B')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('D', vec![direct('C')]), + ('C', vec![direct('B')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" J direct(I), direct(G) @@ -1256,16 +1258,16 @@ mod tests { #[test] fn test_topo_grouped_merge_and_fork() { let graph = vec![ - (id('J'), vec![direct('F')]), - (id('I'), vec![direct('E')]), - (id('H'), vec![direct('G')]), - (id('G'), vec![direct('D'), direct('E')]), - (id('F'), vec![direct('C')]), - (id('E'), vec![direct('B')]), - (id('D'), vec![direct('B')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('J', vec![direct('F')]), + ('I', vec![direct('E')]), + ('H', vec![direct('G')]), + ('G', vec![direct('D'), direct('E')]), + ('F', vec![direct('C')]), + ('E', vec![direct('B')]), + ('D', vec![direct('B')]), + ('C', vec![direct('A')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" J direct(F) @@ -1316,16 +1318,16 @@ mod tests { #[test] fn test_topo_grouped_merge_and_fork_multiple_roots() { let graph = vec![ - (id('J'), vec![direct('F')]), - (id('I'), vec![direct('G')]), - (id('H'), vec![direct('E')]), - (id('G'), vec![direct('E'), direct('B')]), - (id('F'), vec![direct('D')]), - (id('E'), vec![direct('C')]), - (id('D'), vec![direct('A')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![missing('X')]), - (id('A'), vec![]), + ('J', vec![direct('F')]), + ('I', vec![direct('G')]), + ('H', vec![direct('E')]), + ('G', vec![direct('E'), direct('B')]), + ('F', vec![direct('D')]), + ('E', vec![direct('C')]), + ('D', vec![direct('A')]), + ('C', vec![direct('A')]), + ('B', vec![missing('X')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" J direct(F) @@ -1379,12 +1381,12 @@ mod tests { #[test] fn test_topo_grouped_parallel_interleaved() { - let graph = vec![ - (id('E'), vec![direct('C')]), - (id('D'), vec![direct('B')]), - (id('C'), vec![direct('A')]), - (id('B'), vec![missing('X')]), - (id('A'), vec![]), + let graph = [ + ('E', vec![direct('C')]), + ('D', vec![direct('B')]), + ('C', vec![direct('A')]), + ('B', vec![missing('X')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" E direct(C) @@ -1418,15 +1420,15 @@ mod tests { #[test] fn test_topo_grouped_multiple_child_dependencies() { let graph = vec![ - (id('I'), vec![direct('H'), direct('G')]), - (id('H'), vec![direct('D')]), - (id('G'), vec![direct('B')]), - (id('F'), vec![direct('E'), direct('C')]), - (id('E'), vec![direct('D')]), - (id('D'), vec![direct('B')]), - (id('C'), vec![direct('B')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('I', vec![direct('H'), direct('G')]), + ('H', vec![direct('D')]), + ('G', vec![direct('B')]), + ('F', vec![direct('E'), direct('C')]), + ('E', vec![direct('D')]), + ('D', vec![direct('B')]), + ('C', vec![direct('B')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" I direct(H), direct(G) @@ -1475,9 +1477,9 @@ mod tests { #[test] fn test_topo_grouped_requeue_unpopulated() { let graph = [ - (id('C'), vec![direct('A'), direct('B')]), - (id('B'), vec![direct('A')]), - (id('A'), vec![]), + ('C', vec![direct('A'), direct('B')]), + ('B', vec![direct('A')]), + ('A', vec![]), ]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" C direct(A), direct(B) @@ -1500,11 +1502,11 @@ mod tests { // B is the second parent, B-A is processed next and A is queued again. So // one of them in the queue has to be ignored. let mut iter = topo_grouped(graph.iter().cloned()); - assert_eq!(iter.next().unwrap().0, id('C')); - assert_eq!(iter.emittable_ids, vec![id('A'), id('B')]); - assert_eq!(iter.next().unwrap().0, id('B')); - assert_eq!(iter.emittable_ids, vec![id('A'), id('A')]); - assert_eq!(iter.next().unwrap().0, id('A')); + assert_eq!(iter.next().unwrap().0, 'C'); + assert_eq!(iter.emittable_ids, vec!['A', 'B']); + assert_eq!(iter.next().unwrap().0, 'B'); + assert_eq!(iter.emittable_ids, vec!['A', 'A']); + assert_eq!(iter.next().unwrap().0, 'A'); assert!(iter.next().is_none()); assert!(iter.emittable_ids.is_empty()); } @@ -1513,7 +1515,7 @@ mod tests { fn test_topo_grouped_duplicated_edges() { // The graph shouldn't have duplicated parent->child edges, but topo-grouped // iterator can handle it anyway. - let graph = [(id('B'), vec![direct('A'), direct('A')]), (id('A'), vec![])]; + let graph = [('B', vec![direct('A'), direct('A')]), ('A', vec![])]; insta::assert_snapshot!(format_graph(graph.iter().cloned()), @r###" B direct(A), direct(A) │ @@ -1528,9 +1530,9 @@ mod tests { "###); let mut iter = topo_grouped(graph.iter().cloned()); - assert_eq!(iter.next().unwrap().0, id('B')); - assert_eq!(iter.emittable_ids, vec![id('A'), id('A')]); - assert_eq!(iter.next().unwrap().0, id('A')); + assert_eq!(iter.next().unwrap().0, 'B'); + assert_eq!(iter.emittable_ids, vec!['A', 'A']); + assert_eq!(iter.next().unwrap().0, 'A'); assert!(iter.next().is_none()); assert!(iter.emittable_ids.is_empty()); } diff --git a/lib/src/lib.rs b/lib/src/lib.rs index e96d1d1788..7806518b28 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -48,6 +48,7 @@ pub mod git; pub mod git_backend; pub mod gitignore; pub mod gpg_signing; +pub mod graph; pub mod hex_util; pub mod id_prefix; pub mod index; @@ -68,7 +69,6 @@ pub mod refs; pub mod repo; pub mod repo_path; pub mod revset; -pub mod revset_graph; mod revset_parser; pub mod rewrite; pub mod settings; diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 50c82a29be..3bd3135620 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -33,13 +33,13 @@ use crate::commit::Commit; use crate::dsl_util::{collect_similar, AliasId}; use crate::fileset::{FilePattern, FilesetExpression}; use crate::git; +use crate::graph::GraphEdge; use crate::hex_util::to_forward_hex; use crate::id_prefix::IdPrefixContext; use crate::object_id::{HexPrefix, PrefixResolution}; use crate::op_store::WorkspaceId; use crate::repo::Repo; use crate::repo_path::RepoPathUiConverter; -use crate::revset_graph::RevsetGraphEdge; // TODO: introduce AST types and remove parse_expression_rule, Rule from the // re-exports pub use crate::revset_parser::{ @@ -1908,7 +1908,7 @@ pub trait Revset: fmt::Debug { where Self: 'a; - fn iter_graph<'a>(&self) -> Box)> + 'a> + fn iter_graph<'a>(&self) -> Box>)> + 'a> where Self: 'a; diff --git a/lib/tests/test_default_revset_graph_iterator.rs b/lib/tests/test_default_revset_graph_iterator.rs index 951c422caf..7c750f12bb 100644 --- a/lib/tests/test_default_revset_graph_iterator.rs +++ b/lib/tests/test_default_revset_graph_iterator.rs @@ -13,12 +13,13 @@ // limitations under the License. use itertools::Itertools; +use jj_lib::backend::CommitId; use jj_lib::commit::Commit; use jj_lib::default_index::revset_engine::{evaluate, RevsetImpl}; use jj_lib::default_index::DefaultReadonlyIndex; +use jj_lib::graph::GraphEdge; use jj_lib::repo::{ReadonlyRepo, Repo as _}; use jj_lib::revset::ResolvedExpression; -use jj_lib::revset_graph::RevsetGraphEdge; use test_case::test_case; use testutils::{CommitGraphBuilder, TestRepo}; @@ -36,16 +37,16 @@ fn revset_for_commits( evaluate(&expression, repo.store(), index.clone()).unwrap() } -fn direct(commit: &Commit) -> RevsetGraphEdge { - RevsetGraphEdge::direct(commit.id().clone()) +fn direct(commit: &Commit) -> GraphEdge { + GraphEdge::direct(commit.id().clone()) } -fn indirect(commit: &Commit) -> RevsetGraphEdge { - RevsetGraphEdge::indirect(commit.id().clone()) +fn indirect(commit: &Commit) -> GraphEdge { + GraphEdge::indirect(commit.id().clone()) } -fn missing(commit: &Commit) -> RevsetGraphEdge { - RevsetGraphEdge::missing(commit.id().clone()) +fn missing(commit: &Commit) -> GraphEdge { + GraphEdge::missing(commit.id().clone()) } #[test_case(false ; "keep transitive edges")] diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 53be2381fd..27207084fc 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -21,6 +21,7 @@ use jj_lib::commit::Commit; use jj_lib::fileset::FilesetExpression; use jj_lib::git; use jj_lib::git_backend::GitBackend; +use jj_lib::graph::{GraphEdge, ReverseGraphIterator}; use jj_lib::object_id::ObjectId; use jj_lib::op_store::{RefTarget, RemoteRef, RemoteRefState, WorkspaceId}; use jj_lib::repo::Repo; @@ -30,7 +31,6 @@ use jj_lib::revset::{ RevsetAliasesMap, RevsetExpression, RevsetExtensions, RevsetFilterPredicate, RevsetParseContext, RevsetResolutionError, RevsetWorkspaceContext, SymbolResolverExtension, }; -use jj_lib::revset_graph::{ReverseRevsetGraphIterator, RevsetGraphEdge}; use jj_lib::settings::GitSettings; use jj_lib::workspace::Workspace; use test_case::test_case; @@ -2966,7 +2966,7 @@ fn test_reverse_graph_iterator() { repo.as_ref(), &[&commit_a, &commit_c, &commit_d, &commit_e, &commit_f], ); - let commits = ReverseRevsetGraphIterator::new(revset.iter_graph()).collect_vec(); + let commits = ReverseGraphIterator::new(revset.iter_graph()).collect_vec(); assert_eq!(commits.len(), 5); assert_eq!(commits[0].0, *commit_a.id()); assert_eq!(commits[1].0, *commit_c.id()); @@ -2975,23 +2975,17 @@ fn test_reverse_graph_iterator() { assert_eq!(commits[4].0, *commit_f.id()); assert_eq!( commits[0].1, - vec![RevsetGraphEdge::indirect(commit_c.id().clone())] + vec![GraphEdge::indirect(commit_c.id().clone())] ); assert_eq!( commits[1].1, vec![ - RevsetGraphEdge::direct(commit_e.id().clone()), - RevsetGraphEdge::direct(commit_d.id().clone()), + GraphEdge::direct(commit_e.id().clone()), + GraphEdge::direct(commit_d.id().clone()), ] ); - assert_eq!( - commits[2].1, - vec![RevsetGraphEdge::direct(commit_f.id().clone())] - ); - assert_eq!( - commits[3].1, - vec![RevsetGraphEdge::direct(commit_f.id().clone())] - ); + assert_eq!(commits[2].1, vec![GraphEdge::direct(commit_f.id().clone())]); + assert_eq!(commits[3].1, vec![GraphEdge::direct(commit_f.id().clone())]); assert_eq!(commits[4].1, vec![]); }