From 0747da0491f6eabbd6874ee144dc053c101e41ec Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 9 May 2022 10:38:23 -0700 Subject: [PATCH] revset_graph_iterator: add a mode for generating reverse graph The request to show the log output with more recent commits at the bottom comes up once in a while (among Mercurial users, and now also for jj from @arxanas). It's pretty easy to implement by adding an adapter to the current `RevsetGraphIterator`. It works by first collecting all nodes and edges into a vector and then yielding them in reverse order and with reversed edges. That means it's no longer lazy, but that seems fine since the feature is optional. Also, it's only the subset of nodes that are in the selected revset that will be collected. Making the CLI use the new iterator adapter will come in a later patch. --- lib/src/revset_graph_iterator.rs | 47 +++++++++++++++++- lib/tests/test_revset_graph_iterator.rs | 65 ++++++++++++++++++++++++- 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/lib/src/revset_graph_iterator.rs b/lib/src/revset_graph_iterator.rs index 178823860..f9a8654b5 100644 --- a/lib/src/revset_graph_iterator.rs +++ b/lib/src/revset_graph_iterator.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::cmp::min; -use std::collections::{BTreeMap, HashSet}; +use std::collections::{BTreeMap, HashMap, HashSet}; use crate::index::{IndexEntry, IndexPosition}; use crate::nightly_shims::BTreeMapExt; @@ -149,6 +149,10 @@ impl<'revset, 'repo> RevsetGraphIterator<'revset, 'repo> { self } + pub fn reversed(self) -> ReverseRevsetGraphIterator<'repo> { + ReverseRevsetGraphIterator::new(self) + } + fn next_index_entry(&mut self) -> Option> { if let Some(index_entry) = self.look_ahead.pop_last_value() { return Some(index_entry); @@ -304,3 +308,44 @@ impl<'revset, 'repo> Iterator for RevsetGraphIterator<'revset, 'repo> { Some((index_entry, edges)) } } + +pub struct ReverseRevsetGraphIterator<'repo> { + items: Vec<(IndexEntry<'repo>, Vec)>, +} + +impl<'repo> ReverseRevsetGraphIterator<'repo> { + fn new<'revset>(input: RevsetGraphIterator<'revset, 'repo>) -> Self { + let mut entries = vec![]; + let mut reverse_edges: HashMap> = HashMap::new(); + for (entry, edges) in input { + for RevsetGraphEdge { target, edge_type } in edges { + reverse_edges + .entry(target) + .or_default() + .push(RevsetGraphEdge { + target: entry.position(), + edge_type, + }) + } + entries.push(entry); + } + + let mut items = vec![]; + for entry in entries.into_iter() { + let edges = reverse_edges + .get(&entry.position()) + .cloned() + .unwrap_or_default(); + items.push((entry, edges)); + } + Self { items } + } +} + +impl<'repo> Iterator for ReverseRevsetGraphIterator<'repo> { + type Item = (IndexEntry<'repo>, Vec); + + fn next(&mut self) -> Option { + self.items.pop() + } +} diff --git a/lib/tests/test_revset_graph_iterator.rs b/lib/tests/test_revset_graph_iterator.rs index db88b0559..8ec37510f 100644 --- a/lib/tests/test_revset_graph_iterator.rs +++ b/lib/tests/test_revset_graph_iterator.rs @@ -69,7 +69,9 @@ fn test_graph_iterator_virtual_octopus(skip_transitive_edges: bool) { let repo = &test_repo.repo; // Tests that merges outside the set can result in more parent edges than there - // was in the input: F + // was in the input: + // + // F // |\ // d e F // |\|\ => /|\ @@ -379,3 +381,64 @@ fn test_graph_iterator_edge_escapes_from_(skip_transitive_edges: bool) { assert_eq!(commits[3].1, vec![RevsetGraphEdge::indirect(pos_a)]); assert_eq!(commits[4].1, vec![RevsetGraphEdge::missing(pos_root)]); } + +#[test] +fn test_reverse_graph_iterator() { + let settings = testutils::user_settings(); + let test_repo = testutils::init_repo(&settings, true); + let repo = &test_repo.repo; + + // Tests that merges, forks, direct edges, indirect edges, and "missing" edges + // are correct in reversed graph. "Missing" edges (i.e. edges to commits not + // in the input set) won't be part of the reversed graph. Conversely, there + // won't be missing edges to children not in the input. + // + // F + // |\ + // D E + // |/ + // C + // | + // b + // | + // A + // | + // root + let mut tx = repo.start_transaction("test"); + let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo()); + let commit_a = graph_builder.initial_commit(); + let commit_b = graph_builder.commit_with_parents(&[&commit_a]); + let commit_c = graph_builder.commit_with_parents(&[&commit_b]); + let commit_d = graph_builder.commit_with_parents(&[&commit_c]); + let commit_e = graph_builder.commit_with_parents(&[&commit_c]); + let commit_f = graph_builder.commit_with_parents(&[&commit_d, &commit_e]); + let repo = tx.commit(); + + let pos_c = repo.index().commit_id_to_pos(commit_c.id()).unwrap(); + let pos_d = repo.index().commit_id_to_pos(commit_d.id()).unwrap(); + let pos_e = repo.index().commit_id_to_pos(commit_e.id()).unwrap(); + let pos_f = repo.index().commit_id_to_pos(commit_f.id()).unwrap(); + + let revset = revset_for_commits( + repo.as_repo_ref(), + &[&commit_a, &commit_c, &commit_d, &commit_e, &commit_f], + ); + let commits = revset.iter().graph().reversed().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].1, vec![RevsetGraphEdge::indirect(pos_c)]); + assert_eq!( + commits[1].1, + vec![ + RevsetGraphEdge::direct(pos_e), + RevsetGraphEdge::direct(pos_d), + ] + ); + assert_eq!(commits[2].1, vec![RevsetGraphEdge::direct(pos_f)]); + assert_eq!(commits[3].1, vec![RevsetGraphEdge::direct(pos_f)]); + assert_eq!(commits[4].1, vec![]); +}