revset: move graph iteration onto Revset trait

We want to allow custom revset engines define their own graph
iterator. This commit helps with that by adding a
`Revset::iter_graph()` function that returns an abstract iterator.

The current `RevsetGraphIterator` can be configured to skip or include
transitive edges. It skips them by default and we don't expose option
in the CLI. I didn't bother including that functionality in the new
`iter_graph()` either. At least for now, it will be up to the
implementation whether it includes such edges (it would of course be
free to ignore the caller's request even if we added an option for it
in the API).

lib/src/default_revset_engine.rs

@@ -27,9 +27,10 @@ use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher};
 use crate::op_store::WorkspaceId;
 use crate::repo::Repo;
 use crate::revset::{
-    Revset, RevsetError, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt,
+    Revset, RevsetError, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge,
-    RevsetWorkspaceContext, GENERATION_RANGE_FULL,
+    RevsetIteratorExt, RevsetWorkspaceContext, GENERATION_RANGE_FULL,
 };
+use crate::revset_graph_iterator::RevsetGraphIterator;
 use crate::rewrite;
 
 fn resolve_git_ref(repo: &dyn Repo, symbol: &str) -> Option<Vec<CommitId>> {
@@ -215,6 +216,12 @@ impl<'index> Revset<'index> for RevsetImpl<'index> {
         self.inner.iter()
     }
 
+    fn iter_graph(
+        &self,
+    ) -> Box<dyn Iterator<Item = (IndexEntry<'index>, Vec<RevsetGraphEdge>)> + '_> {
+        Box::new(RevsetGraphIterator::new(self))
+    }
+
     fn is_empty(&self) -> bool {
         self.iter().next().is_none()
     }

lib/src/revset.rs

@@ -29,7 +29,7 @@ use thiserror::Error;
 
 use crate::backend::{BackendError, BackendResult, CommitId};
 use crate::commit::Commit;
-use crate::default_index_store::IndexEntry;
+use crate::default_index_store::{IndexEntry, IndexPosition};
 use crate::op_store::WorkspaceId;
 use crate::repo::Repo;
 use crate::repo_path::{FsPathParseError, RepoPath};
@@ -1377,9 +1377,47 @@ pub trait Revset<'index> {
     // All revsets currently iterate in order of descending index position
     fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_>;
 
+    fn iter_graph(
+        &self,
+    ) -> Box<dyn Iterator<Item = (IndexEntry<'index>, Vec<RevsetGraphEdge>)> + '_>;
+
     fn is_empty(&self) -> bool;
 }
 
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
+pub struct RevsetGraphEdge {
+    pub target: IndexPosition,
+    pub edge_type: RevsetGraphEdgeType,
+}
+
+impl RevsetGraphEdge {
+    pub fn missing(target: IndexPosition) -> Self {
+        Self {
+            target,
+            edge_type: RevsetGraphEdgeType::Missing,
+        }
+    }
+    pub fn direct(target: IndexPosition) -> Self {
+        Self {
+            target,
+            edge_type: RevsetGraphEdgeType::Direct,
+        }
+    }
+    pub fn indirect(target: IndexPosition) -> Self {
+        Self {
+            target,
+            edge_type: RevsetGraphEdgeType::Indirect,
+        }
+    }
+}
+
+#[derive(Debug, PartialEq, Eq, Clone, Hash)]
+pub enum RevsetGraphEdgeType {
+    Missing,
+    Direct,
+    Indirect,
+}
+
 pub trait RevsetIteratorExt<'index, I> {
     fn commit_ids(self) -> RevsetCommitIdIterator<I>;
     fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I>;

lib/src/revset_graph_iterator.rs

@@ -17,41 +17,7 @@ use std::collections::{BTreeMap, HashMap, HashSet};
 
 use crate::default_index_store::{IndexEntry, IndexPosition};
 use crate::nightly_shims::BTreeMapExt;
-use crate::revset::Revset;
+use crate::revset::{Revset, RevsetGraphEdge, RevsetGraphEdgeType};
-
-#[derive(Debug, PartialEq, Eq, Clone, Hash)]
-pub struct RevsetGraphEdge {
-    pub target: IndexPosition,
-    pub edge_type: RevsetGraphEdgeType,
-}
-
-impl RevsetGraphEdge {
-    pub fn missing(target: IndexPosition) -> Self {
-        Self {
-            target,
-            edge_type: RevsetGraphEdgeType::Missing,
-        }
-    }
-    pub fn direct(target: IndexPosition) -> Self {
-        Self {
-            target,
-            edge_type: RevsetGraphEdgeType::Direct,
-        }
-    }
-    pub fn indirect(target: IndexPosition) -> Self {
-        Self {
-            target,
-            edge_type: RevsetGraphEdgeType::Indirect,
-        }
-    }
-}
-
-#[derive(Debug, PartialEq, Eq, Clone, Hash)]
-pub enum RevsetGraphEdgeType {
-    Missing,
-    Direct,
-    Indirect,
-}
 
 // Given an iterator over some set of revisions, yields the same revisions with
 // associated edge types.
@@ -149,10 +115,6 @@ impl<'revset, 'index> RevsetGraphIterator<'revset, 'index> {
         self
     }
 
-    pub fn reversed(self) -> ReverseRevsetGraphIterator<'index> {
-        ReverseRevsetGraphIterator::new(self)
-    }
-
     fn next_index_entry(&mut self) -> Option<IndexEntry<'index>> {
         if let Some(index_entry) = self.look_ahead.pop_last_value() {
             return Some(index_entry);
@@ -314,7 +276,9 @@ pub struct ReverseRevsetGraphIterator<'index> {
 }
 
 impl<'index> ReverseRevsetGraphIterator<'index> {
-    fn new<'revset>(input: RevsetGraphIterator<'revset, 'index>) -> Self {
+    pub fn new<'revset>(
+        input: Box<dyn Iterator<Item = (IndexEntry<'index>, Vec<RevsetGraphEdge>)> + 'revset>,
+    ) -> Self {
         let mut entries = vec![];
         let mut reverse_edges: HashMap<IndexPosition, Vec<RevsetGraphEdge>> = HashMap::new();
         for (entry, edges) in input {

lib/tests/test_revset_graph_iterator.rs

@@ -15,7 +15,8 @@
 use itertools::Itertools;
 use jujutsu_lib::default_revset_engine::revset_for_commits;
 use jujutsu_lib::repo::Repo;
-use jujutsu_lib::revset_graph_iterator::{RevsetGraphEdge, RevsetGraphIterator};
+use jujutsu_lib::revset::RevsetGraphEdge;
+use jujutsu_lib::revset_graph_iterator::{ReverseRevsetGraphIterator, RevsetGraphIterator};
 use test_case::test_case;
 use testutils::{CommitGraphBuilder, TestRepo};
 
@@ -408,9 +409,7 @@ fn test_reverse_graph_iterator() {
         &repo,
         &[&commit_a, &commit_c, &commit_d, &commit_e, &commit_f],
     );
-    let commits = RevsetGraphIterator::new(revset.as_ref())
+    let commits = ReverseRevsetGraphIterator::new(revset.iter_graph()).collect_vec();
-        .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());

src/commands/mod.rs

@@ -36,11 +36,10 @@ 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, RevsetIteratorExt,
+    RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge,
-};
+    RevsetGraphEdgeType, RevsetIteratorExt,
-use jujutsu_lib::revset_graph_iterator::{
-    RevsetGraphEdge, RevsetGraphEdgeType, RevsetGraphIterator,
 };
+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};
@@ -1487,9 +1486,9 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
             let default_node_symbol = graph.default_node_symbol().to_owned();
             let iter: Box<dyn Iterator<Item = (IndexEntry, Vec<RevsetGraphEdge>)>> =
                 if args.reversed {
-                    Box::new(RevsetGraphIterator::new(revset.as_ref()).reversed())
+                    Box::new(ReverseRevsetGraphIterator::new(revset.iter_graph()))
                 } else {
-                    Box::new(RevsetGraphIterator::new(revset.as_ref()))
+                    revset.iter_graph()
                 };
             for (index_entry, edges) in iter {
                 let mut graphlog_edges = vec![];