From 01d7239732041d390888b98cf3dbca7d008513ce Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 19 Mar 2023 17:44:50 -0700 Subject: [PATCH] revset: make graph iterator yield commit ids (not index entries) We only need `CommitId`s, and `IndexEntry` is specific to the default index implementation. --- lib/src/default_revset_engine.rs | 4 +- lib/src/default_revset_graph_iterator.rs | 5 ++- lib/src/revset.rs | 31 +++++++-------- .../test_default_revset_graph_iterator.rs | 38 +++++++++---------- lib/tests/test_revset.rs | 10 ++--- src/commands/mod.rs | 15 ++++---- 6 files changed, 48 insertions(+), 55 deletions(-) diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 347ce6346..0c7e83e8e 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -72,9 +72,7 @@ impl<'index> Revset<'index> for RevsetImpl<'index> { self.inner.iter() } - fn iter_graph( - &self, - ) -> Box, Vec)> + '_> { + fn iter_graph(&self) -> Box)> + '_> { Box::new(RevsetGraphIterator::new(self)) } diff --git a/lib/src/default_revset_graph_iterator.rs b/lib/src/default_revset_graph_iterator.rs index ef9bf4735..ebebec40d 100644 --- a/lib/src/default_revset_graph_iterator.rs +++ b/lib/src/default_revset_graph_iterator.rs @@ -15,6 +15,7 @@ use std::cmp::min; use std::collections::{BTreeMap, HashSet}; +use crate::backend::CommitId; use crate::default_index_store::{IndexEntry, IndexPosition}; use crate::nightly_shims::BTreeMapExt; use crate::revset::{Revset, RevsetGraphEdge, RevsetGraphEdgeType}; @@ -267,7 +268,7 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> { } impl<'revset, 'index> Iterator for RevsetGraphIterator<'revset, 'index> { - type Item = (IndexEntry<'index>, Vec); + type Item = (CommitId, Vec); fn next(&mut self) -> Option { let index_entry = self.next_index_entry()?; @@ -278,6 +279,6 @@ impl<'revset, 'index> Iterator for RevsetGraphIterator<'revset, 'index> { let mut edges: Vec<_> = edges.into_iter().collect(); edges.sort_by(|(target_pos1, _), (target_pos2, _)| target_pos2.cmp(target_pos1)); let edges = edges.into_iter().map(|(_, edge)| edge).collect(); - Some((index_entry, edges)) + Some((index_entry.commit_id(), edges)) } } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 8c241b604..bb4daca70 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1559,9 +1559,7 @@ pub trait Revset<'index> { /// Iterate in topological order with children before parents. fn iter(&self) -> Box> + '_>; - fn iter_graph( - &self, - ) -> Box, Vec)> + '_>; + fn iter_graph(&self) -> Box)> + '_>; fn is_empty(&self) -> bool; } @@ -1670,43 +1668,40 @@ pub struct RevsetWorkspaceContext<'a> { pub workspace_root: &'a Path, } -pub struct ReverseRevsetGraphIterator<'index> { - items: Vec<(IndexEntry<'index>, Vec)>, +pub struct ReverseRevsetGraphIterator { + items: Vec<(CommitId, Vec)>, } -impl<'index> ReverseRevsetGraphIterator<'index> { +impl ReverseRevsetGraphIterator { pub fn new<'revset>( - input: Box, Vec)> + 'revset>, + input: Box)> + 'revset>, ) -> Self { let mut entries = vec![]; let mut reverse_edges: HashMap> = HashMap::new(); - for (entry, edges) in input { + for (commit_id, edges) in input { for RevsetGraphEdge { target, edge_type } in edges { reverse_edges .entry(target) .or_default() .push(RevsetGraphEdge { - target: entry.commit_id(), + target: commit_id.clone(), edge_type, }) } - entries.push(entry); + entries.push(commit_id); } let mut items = vec![]; - for entry in entries.into_iter() { - let edges = reverse_edges - .get(&entry.commit_id()) - .cloned() - .unwrap_or_default(); - items.push((entry, edges)); + for commit_id in entries.into_iter() { + let edges = reverse_edges.get(&commit_id).cloned().unwrap_or_default(); + items.push((commit_id, edges)); } Self { items } } } -impl<'index> Iterator for ReverseRevsetGraphIterator<'index> { - type Item = (IndexEntry<'index>, Vec); +impl Iterator for ReverseRevsetGraphIterator { + type Item = (CommitId, Vec); fn next(&mut self) -> Option { self.items.pop() diff --git a/lib/tests/test_default_revset_graph_iterator.rs b/lib/tests/test_default_revset_graph_iterator.rs index 32c6fecc2..128b275a1 100644 --- a/lib/tests/test_default_revset_graph_iterator.rs +++ b/lib/tests/test_default_revset_graph_iterator.rs @@ -62,8 +62,8 @@ fn test_graph_iterator_linearized(skip_transitive_edges: bool) { .set_skip_transitive_edges(skip_transitive_edges) .collect_vec(); assert_eq!(commits.len(), 2); - assert_eq!(commits[0].0.commit_id(), *commit_d.id()); - assert_eq!(commits[1].0.commit_id(), *commit_a.id()); + assert_eq!(commits[0].0, *commit_d.id()); + assert_eq!(commits[1].0, *commit_a.id()); assert_eq!(commits[0].1, vec![indirect(&commit_a)]); assert_eq!(commits[1].1, vec![missing(&root_commit)]); } @@ -101,10 +101,10 @@ fn test_graph_iterator_virtual_octopus(skip_transitive_edges: bool) { .set_skip_transitive_edges(skip_transitive_edges) .collect_vec(); assert_eq!(commits.len(), 4); - assert_eq!(commits[0].0.commit_id(), *commit_f.id()); - assert_eq!(commits[1].0.commit_id(), *commit_c.id()); - assert_eq!(commits[2].0.commit_id(), *commit_b.id()); - assert_eq!(commits[3].0.commit_id(), *commit_a.id()); + assert_eq!(commits[0].0, *commit_f.id()); + assert_eq!(commits[1].0, *commit_c.id()); + assert_eq!(commits[2].0, *commit_b.id()); + assert_eq!(commits[3].0, *commit_a.id()); assert_eq!( commits[0].1, vec![ @@ -151,9 +151,9 @@ fn test_graph_iterator_simple_fork(skip_transitive_edges: bool) { .set_skip_transitive_edges(skip_transitive_edges) .collect_vec(); assert_eq!(commits.len(), 3); - assert_eq!(commits[0].0.commit_id(), *commit_e.id()); - assert_eq!(commits[1].0.commit_id(), *commit_c.id()); - assert_eq!(commits[2].0.commit_id(), *commit_a.id()); + assert_eq!(commits[0].0, *commit_e.id()); + assert_eq!(commits[1].0, *commit_c.id()); + assert_eq!(commits[2].0, *commit_a.id()); assert_eq!(commits[0].1, vec![indirect(&commit_a)]); assert_eq!(commits[1].1, vec![indirect(&commit_a)]); assert_eq!(commits[2].1, vec![missing(&root_commit)]); @@ -191,8 +191,8 @@ fn test_graph_iterator_multiple_missing(skip_transitive_edges: bool) { .set_skip_transitive_edges(skip_transitive_edges) .collect_vec(); assert_eq!(commits.len(), 2); - assert_eq!(commits[0].0.commit_id(), *commit_f.id()); - assert_eq!(commits[1].0.commit_id(), *commit_b.id()); + assert_eq!(commits[0].0, *commit_f.id()); + assert_eq!(commits[1].0, *commit_b.id()); assert_eq!( commits[0].1, vec![missing(&commit_c), indirect(&commit_b), missing(&commit_a),] @@ -234,9 +234,9 @@ fn test_graph_iterator_edge_to_ancestor(skip_transitive_edges: bool) { .set_skip_transitive_edges(skip_transitive_edges) .collect_vec(); assert_eq!(commits.len(), 3); - assert_eq!(commits[0].0.commit_id(), *commit_f.id()); - assert_eq!(commits[1].0.commit_id(), *commit_d.id()); - assert_eq!(commits[2].0.commit_id(), *commit_c.id()); + assert_eq!(commits[0].0, *commit_f.id()); + assert_eq!(commits[1].0, *commit_d.id()); + assert_eq!(commits[2].0, *commit_c.id()); if skip_transitive_edges { assert_eq!(commits[0].1, vec![direct(&commit_d)]); } else { @@ -292,11 +292,11 @@ fn test_graph_iterator_edge_escapes_from_(skip_transitive_edges: bool) { .set_skip_transitive_edges(skip_transitive_edges) .collect_vec(); assert_eq!(commits.len(), 5); - assert_eq!(commits[0].0.commit_id(), *commit_j.id()); - assert_eq!(commits[1].0.commit_id(), *commit_h.id()); - assert_eq!(commits[2].0.commit_id(), *commit_g.id()); - assert_eq!(commits[3].0.commit_id(), *commit_d.id()); - assert_eq!(commits[4].0.commit_id(), *commit_a.id()); + assert_eq!(commits[0].0, *commit_j.id()); + assert_eq!(commits[1].0, *commit_h.id()); + assert_eq!(commits[2].0, *commit_g.id()); + assert_eq!(commits[3].0, *commit_d.id()); + assert_eq!(commits[4].0, *commit_a.id()); if skip_transitive_edges { assert_eq!(commits[0].1, vec![indirect(&commit_h), direct(&commit_g),]); assert_eq!(commits[1].1, vec![indirect(&commit_d)]); diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index c4a1aea73..d0f9ba2ff 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -2052,11 +2052,11 @@ fn test_reverse_graph_iterator() { ); let commits = ReverseRevsetGraphIterator::new(revset.iter_graph()).collect_vec(); assert_eq!(commits.len(), 5); - assert_eq!(commits[0].0.commit_id(), *commit_a.id()); - assert_eq!(commits[1].0.commit_id(), *commit_c.id()); - assert_eq!(commits[2].0.commit_id(), *commit_d.id()); - assert_eq!(commits[3].0.commit_id(), *commit_e.id()); - assert_eq!(commits[4].0.commit_id(), *commit_f.id()); + assert_eq!(commits[0].0, *commit_a.id()); + assert_eq!(commits[1].0, *commit_c.id()); + assert_eq!(commits[2].0, *commit_d.id()); + assert_eq!(commits[3].0, *commit_e.id()); + assert_eq!(commits[4].0, *commit_f.id()); assert_eq!( commits[0].1, vec![RevsetGraphEdge::indirect(commit_c.id().clone())] diff --git a/src/commands/mod.rs b/src/commands/mod.rs index b91938c36..0b1614f33 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -1499,13 +1499,13 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C if !args.no_graph { let mut graph = get_graphlog(command.settings(), formatter.raw()); let default_node_symbol = graph.default_node_symbol().to_owned(); - let iter: Box)>> = - if args.reversed { - Box::new(ReverseRevsetGraphIterator::new(revset.iter_graph())) - } else { - revset.iter_graph() - }; - for (index_entry, edges) in iter { + let iter: Box)>> = if args.reversed + { + Box::new(ReverseRevsetGraphIterator::new(revset.iter_graph())) + } else { + revset.iter_graph() + }; + for (commit_id, edges) in iter { let mut graphlog_edges = vec![]; // TODO: Should we update RevsetGraphIterator to yield this flag instead of all // the missing edges since we don't care about where they point here @@ -1530,7 +1530,6 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C graphlog_edges.push(Edge::Missing); } let mut buffer = vec![]; - let commit_id = index_entry.commit_id(); let commit = store.get_commit(&commit_id)?; with_content_format.write_graph_text( ui.new_formatter(&mut buffer).as_mut(),