From 3871efd2f9ca6541e12ff9386457e10e306e6bc2 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 13 Mar 2023 22:14:06 -0700 Subject: [PATCH] revset: move `ReverseRevsetGraphIterator` into `revset` module The iterator is not specific to the implementation in `revset_graph_iterator`, so it belongs in the standard `revset` module. --- lib/src/revset.rs | 43 ++++++++++++++++ lib/src/revset_graph_iterator.rs | 45 +--------------- lib/tests/test_revset.rs | 68 +++++++++++++++++++++++-- lib/tests/test_revset_graph_iterator.rs | 63 +---------------------- src/commands/mod.rs | 5 +- 5 files changed, 112 insertions(+), 112 deletions(-) diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 483b11a5f..66d34db59 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1488,6 +1488,49 @@ pub struct RevsetWorkspaceContext<'a> { pub workspace_root: &'a Path, } +pub struct ReverseRevsetGraphIterator<'index> { + items: Vec<(IndexEntry<'index>, Vec)>, +} + +impl<'index> ReverseRevsetGraphIterator<'index> { + pub fn new<'revset>( + input: Box, Vec)> + 'revset>, + ) -> 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<'index> Iterator for ReverseRevsetGraphIterator<'index> { + type Item = (IndexEntry<'index>, Vec); + + fn next(&mut self) -> Option { + self.items.pop() + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/lib/src/revset_graph_iterator.rs b/lib/src/revset_graph_iterator.rs index 47b04a70f..ccc0cee41 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, HashMap, HashSet}; +use std::collections::{BTreeMap, HashSet}; use crate::default_index_store::{IndexEntry, IndexPosition}; use crate::nightly_shims::BTreeMapExt; @@ -270,46 +270,3 @@ impl<'revset, 'index> Iterator for RevsetGraphIterator<'revset, 'index> { Some((index_entry, edges)) } } - -pub struct ReverseRevsetGraphIterator<'index> { - items: Vec<(IndexEntry<'index>, Vec)>, -} - -impl<'index> ReverseRevsetGraphIterator<'index> { - pub fn new<'revset>( - input: Box, Vec)> + 'revset>, - ) -> 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<'index> Iterator for ReverseRevsetGraphIterator<'index> { - type Item = (IndexEntry<'index>, Vec); - - fn next(&mut self) -> Option { - self.items.pop() - } -} diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index fc1856e9d..ad9862dc6 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -15,15 +15,16 @@ use std::path::Path; use assert_matches::assert_matches; +use itertools::Itertools; use jujutsu_lib::backend::{CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp}; -use jujutsu_lib::default_revset_engine::resolve_symbol; +use jujutsu_lib::default_revset_engine::{resolve_symbol, revset_for_commits}; use jujutsu_lib::git; use jujutsu_lib::op_store::{RefTarget, WorkspaceId}; use jujutsu_lib::repo::Repo; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::revset::{ - optimize, parse, RevsetAliasesMap, RevsetError, RevsetExpression, RevsetFilterPredicate, - RevsetIteratorExt, RevsetWorkspaceContext, + optimize, parse, ReverseRevsetGraphIterator, RevsetAliasesMap, RevsetError, RevsetExpression, + RevsetFilterPredicate, RevsetGraphEdge, RevsetIteratorExt, RevsetWorkspaceContext, }; use jujutsu_lib::settings::GitSettings; use jujutsu_lib::workspace::Workspace; @@ -2011,3 +2012,64 @@ fn test_evaluate_expression_file(use_git: bool) { vec![commit4.id().clone()] ); } + +#[test] +fn test_reverse_graph_iterator() { + let settings = testutils::user_settings(); + let test_repo = TestRepo::init(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(&settings, "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, + &[&commit_a, &commit_c, &commit_d, &commit_e, &commit_f], + ); + 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].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![]); +} diff --git a/lib/tests/test_revset_graph_iterator.rs b/lib/tests/test_revset_graph_iterator.rs index 49816bb60..7cc29133a 100644 --- a/lib/tests/test_revset_graph_iterator.rs +++ b/lib/tests/test_revset_graph_iterator.rs @@ -16,7 +16,7 @@ use itertools::Itertools; use jujutsu_lib::default_revset_engine::revset_for_commits; use jujutsu_lib::repo::Repo; use jujutsu_lib::revset::RevsetGraphEdge; -use jujutsu_lib::revset_graph_iterator::{ReverseRevsetGraphIterator, RevsetGraphIterator}; +use jujutsu_lib::revset_graph_iterator::RevsetGraphIterator; use test_case::test_case; use testutils::{CommitGraphBuilder, TestRepo}; @@ -367,64 +367,3 @@ 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 = TestRepo::init(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(&settings, "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, - &[&commit_a, &commit_c, &commit_d, &commit_e, &commit_f], - ); - 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].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![]); -} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 00dfb4ba8..0d0e9a850 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -36,10 +36,9 @@ use jujutsu_lib::op_store::{RefTarget, WorkspaceId}; use jujutsu_lib::repo::{ReadonlyRepo, Repo}; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::revset::{ - RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge, - RevsetGraphEdgeType, RevsetIteratorExt, + ReverseRevsetGraphIterator, RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, + RevsetGraphEdge, RevsetGraphEdgeType, RevsetIteratorExt, }; -use jujutsu_lib::revset_graph_iterator::ReverseRevsetGraphIterator; use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser}; use jujutsu_lib::settings::UserSettings; use jujutsu_lib::tree::{merge_trees, Tree};