revset: iterate over commit ids instead of index entries

There are no remaining places where we iterate over a revset and need
the `IndexEntry`s, so we can now make `Revset::iter()` yield
`CommitId`s instead.

lib/src/default_revset_engine.rs

@@ -26,7 +26,7 @@ use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher};
 use crate::repo::Repo;
 use crate::revset::{
     ChangeIdIndex, Revset, RevsetError, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge,
-    RevsetIteratorExt, GENERATION_RANGE_FULL,
+    GENERATION_RANGE_FULL,
 };
 use crate::{backend, rewrite};
 
@@ -73,8 +73,8 @@ impl<'index> RevsetImpl<'index> {
 }
 
 impl<'index> Revset<'index> for RevsetImpl<'index> {
-    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_> {
-        self.inner.iter()
+    fn iter(&self) -> Box<dyn Iterator<Item = CommitId> + '_> {
+        Box::new(RevsetCommitIdIterator(self.inner.iter()))
     }
 
     fn iter_graph(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)> + '_> {
@@ -99,6 +99,16 @@ impl<'index> Revset<'index> for RevsetImpl<'index> {
     }
 }
 
+struct RevsetCommitIdIterator<I>(I);
+
+impl<'index, I: Iterator<Item = IndexEntry<'index>>> Iterator for RevsetCommitIdIterator<I> {
+    type Item = CommitId;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next().map(|index_entry| index_entry.commit_id())
+    }
+}
+
 struct ChangeIdIndexImpl<'index> {
     index: CompositeIndex<'index>,
     pos_by_change: IdIndex<ChangeId, IndexPosition>,
@@ -543,9 +553,9 @@ fn internal_evaluate<'index>(
             generation,
         } => {
             let root_set = internal_evaluate(repo, roots)?;
-            let root_ids = root_set.iter().commit_ids().collect_vec();
+            let root_ids = root_set.iter().map(|entry| entry.commit_id()).collect_vec();
             let head_set = internal_evaluate(repo, heads)?;
-            let head_ids = head_set.iter().commit_ids().collect_vec();
+            let head_ids = head_set.iter().map(|entry| entry.commit_id()).collect_vec();
             let walk = repo.index().walk_revs(&head_ids, &root_ids);
             if generation == &GENERATION_RANGE_FULL {
                 Ok(Box::new(RevWalkRevset { walk }))
@@ -582,7 +592,10 @@ fn internal_evaluate<'index>(
         )),
         RevsetExpression::Heads(candidates) => {
             let candidate_set = internal_evaluate(repo, candidates)?;
-            let candidate_ids = candidate_set.iter().commit_ids().collect_vec();
+            let candidate_ids = candidate_set
+                .iter()
+                .map(|entry| entry.commit_id())
+                .collect_vec();
             Ok(revset_for_commit_ids(
                 repo,
                 &repo.index().heads(&mut candidate_ids.iter()),

lib/src/revset.rs

@@ -29,7 +29,6 @@ use thiserror::Error;
 
 use crate::backend::{BackendError, BackendResult, ChangeId, CommitId, ObjectId};
 use crate::commit::Commit;
-use crate::default_index_store::IndexEntry;
 use crate::hex_util::to_forward_hex;
 use crate::index::{HexPrefix, PrefixResolution};
 use crate::op_store::WorkspaceId;
@@ -1573,7 +1572,7 @@ pub fn resolve_symbols(
 
 pub trait Revset<'index> {
     /// Iterate in topological order with children before parents.
-    fn iter(&self) -> Box<dyn Iterator<Item = IndexEntry<'index>> + '_>;
+    fn iter(&self) -> Box<dyn Iterator<Item = CommitId> + '_>;
 
     fn iter_graph(&self) -> Box<dyn Iterator<Item = (CommitId, Vec<RevsetGraphEdge>)> + '_>;
 
@@ -1638,16 +1637,11 @@ pub enum RevsetGraphEdgeType {
 }
 
 pub trait RevsetIteratorExt<'index, I> {
-    fn commit_ids(self) -> RevsetCommitIdIterator<I>;
     fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I>;
-    fn reversed(self) -> ReverseRevsetIterator<'index>;
+    fn reversed(self) -> ReverseRevsetIterator;
 }
 
-impl<'index, I: Iterator<Item = IndexEntry<'index>>> RevsetIteratorExt<'index, I> for I {
-    fn commit_ids(self) -> RevsetCommitIdIterator<I> {
-        RevsetCommitIdIterator(self)
-    }
-
+impl<'index, I: Iterator<Item = CommitId>> RevsetIteratorExt<'index, I> for I {
     fn commits(self, store: &Arc<Store>) -> RevsetCommitIterator<I> {
         RevsetCommitIterator {
             iter: self,
@@ -1655,44 +1649,34 @@ impl<'index, I: Iterator<Item = IndexEntry<'index>>> RevsetIteratorExt<'index, I
         }
     }
 
-    fn reversed(self) -> ReverseRevsetIterator<'index> {
+    fn reversed(self) -> ReverseRevsetIterator {
         ReverseRevsetIterator {
             entries: self.into_iter().collect_vec(),
         }
     }
 }
 
-pub struct RevsetCommitIdIterator<I>(I);
-
-impl<'index, I: Iterator<Item = IndexEntry<'index>>> Iterator for RevsetCommitIdIterator<I> {
-    type Item = CommitId;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        self.0.next().map(|index_entry| index_entry.commit_id())
-    }
-}
-
 pub struct RevsetCommitIterator<I> {
     store: Arc<Store>,
     iter: I,
 }
 
-impl<'index, I: Iterator<Item = IndexEntry<'index>>> Iterator for RevsetCommitIterator<I> {
+impl<I: Iterator<Item = CommitId>> Iterator for RevsetCommitIterator<I> {
     type Item = BackendResult<Commit>;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.iter
             .next()
-            .map(|index_entry| self.store.get_commit(&index_entry.commit_id()))
+            .map(|commit_id| self.store.get_commit(&commit_id))
     }
 }
 
-pub struct ReverseRevsetIterator<'index> {
-    entries: Vec<IndexEntry<'index>>,
+pub struct ReverseRevsetIterator {
+    entries: Vec<CommitId>,
 }
 
-impl<'index> Iterator for ReverseRevsetIterator<'index> {
-    type Item = IndexEntry<'index>;
+impl Iterator for ReverseRevsetIterator {
+    type Item = CommitId;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.entries.pop()

lib/src/rewrite.rs

@@ -161,7 +161,6 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
             .evaluate(mut_repo)
             .unwrap()
             .iter()
-            .commit_ids()
             .collect();
 
         let to_visit_expression = old_commits_expression.descendants();

lib/tests/test_revset.rs

@@ -26,7 +26,7 @@ use jujutsu_lib::repo_path::RepoPath;
 use jujutsu_lib::revset::{
     optimize, parse, resolve_symbol, resolve_symbols, ReverseRevsetGraphIterator, Revset,
     RevsetAliasesMap, RevsetError, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge,
-    RevsetIteratorExt, RevsetWorkspaceContext,
+    RevsetWorkspaceContext,
 };
 use jujutsu_lib::settings::GitSettings;
 use jujutsu_lib::workspace::Workspace;
@@ -502,12 +502,7 @@ fn test_resolve_symbol_git_refs() {
 fn resolve_commit_ids(repo: &dyn Repo, revset_str: &str) -> Vec<CommitId> {
     let expression = optimize(parse(revset_str, &RevsetAliasesMap::new(), None).unwrap());
     let expression = resolve_symbols(repo, expression, None).unwrap();
-    expression
-        .evaluate(repo)
-        .unwrap()
-        .iter()
-        .commit_ids()
-        .collect()
+    expression.evaluate(repo).unwrap().iter().collect()
 }
 
 fn resolve_commit_ids_in_workspace(
@@ -524,12 +519,7 @@ fn resolve_commit_ids_in_workspace(
     let expression =
         optimize(parse(revset_str, &RevsetAliasesMap::new(), Some(&workspace_ctx)).unwrap());
     let expression = resolve_symbols(repo, expression, Some(&workspace_ctx)).unwrap();
-    expression
-        .evaluate(repo)
-        .unwrap()
-        .iter()
-        .commit_ids()
-        .collect()
+    expression.evaluate(repo).unwrap().iter().collect()
 }
 
 #[test_case(false ; "local backend")]
@@ -1993,8 +1983,7 @@ fn test_evaluate_expression_file(use_git: bool) {
         let expression =
             RevsetExpression::filter(RevsetFilterPredicate::File(Some(vec![file_path.clone()])));
         let revset = expression.evaluate(mut_repo).unwrap();
-        let commit_ids = revset.iter().commit_ids().collect();
-        commit_ids
+        revset.iter().collect()
     };
 
     assert_eq!(resolve(&added_clean_clean), vec![commit1.id().clone()]);

src/commands/mod.rs

@@ -30,7 +30,7 @@ use itertools::Itertools;
 use jujutsu_lib::backend::{CommitId, ObjectId, TreeValue};
 use jujutsu_lib::commit::Commit;
 use jujutsu_lib::dag_walk::topo_order_reverse;
-use jujutsu_lib::default_index_store::{DefaultIndexStore, IndexEntry, ReadonlyIndexWrapper};
+use jujutsu_lib::default_index_store::{DefaultIndexStore, ReadonlyIndexWrapper};
 use jujutsu_lib::matchers::EverythingMatcher;
 use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
 use jujutsu_lib::repo::{ReadonlyRepo, Repo};
@@ -1563,7 +1563,7 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
                 )?;
             }
         } else {
-            let iter: Box<dyn Iterator<Item = IndexEntry>> = if args.reversed {
+            let iter: Box<dyn Iterator<Item = CommitId>> = if args.reversed {
                 Box::new(revset.iter().reversed())
             } else {
                 Box::new(revset.iter())
@@ -2054,7 +2054,6 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
             .dag_range_to(&new_parents)
             .evaluate(tx.repo())?
             .iter()
-            .commit_ids()
             .next()
         {
             return Err(user_error(format!(