revset: change the commits() iterator to return RevsetEvaluationError

This facilatates a change to the Revset trait in the next commit.

cli/src/revset_util.rs

@@ -19,7 +19,6 @@ use std::rc::Rc;
 use std::sync::Arc;
 
 use itertools::Itertools as _;
-use jj_lib::backend::BackendResult;
 use jj_lib::backend::CommitId;
 use jj_lib::commit::Commit;
 use jj_lib::id_prefix::IdPrefixContext;
@@ -114,8 +113,10 @@ impl<'repo> RevsetExpressionEvaluator<'repo> {
     /// sorted in reverse topological order.
     pub fn evaluate_to_commits(
         &self,
-    ) -> Result<impl Iterator<Item = BackendResult<Commit>> + 'repo, UserRevsetEvaluationError>
-    {
+    ) -> Result<
+        impl Iterator<Item = Result<Commit, RevsetEvaluationError>> + 'repo,
+        UserRevsetEvaluationError,
+    > {
         Ok(self.evaluate()?.iter().commits(self.repo.store()))
     }
 }

lib/src/annotate.rs

@@ -36,7 +36,6 @@ use crate::graph::GraphEdgeType;
 use crate::merged_tree::MergedTree;
 use crate::repo::Repo;
 use crate::repo_path::RepoPath;
-use crate::revset::RevsetEvaluationError;
 use crate::revset::RevsetExpression;
 use crate::revset::RevsetFilterPredicate;
 use crate::store::Store;
@@ -164,12 +163,7 @@ fn process_commits(
                 .filtered(predicate),
         )
         .evaluate_programmatic(repo)
-        .map_err(|e| match e {
-            RevsetEvaluationError::StoreError(backend_error) => backend_error,
-            RevsetEvaluationError::Other(_) => {
-                panic!("Unable to evaluate internal revset")
-            }
-        })?;
+        .map_err(|e| e.expect_backend_error())?;
     let mut commit_line_map = get_initial_commit_line_map(starting_commit_id, num_lines);
     let mut original_line_map = HashMap::new();
 

lib/src/repo.rs

@@ -78,7 +78,6 @@ use crate::refs::diff_named_remote_refs;
 use crate::refs::merge_ref_targets;
 use crate::refs::merge_remote_refs;
 use crate::revset;
-use crate::revset::RevsetEvaluationError;
 use crate::revset::RevsetExpression;
 use crate::revset::RevsetIteratorExt;
 use crate::rewrite::merge_commit_trees;
@@ -1194,11 +1193,13 @@ impl MutableRepo {
                 ));
         let to_visit_revset = to_visit_expression
             .evaluate_programmatic(self)
-            .map_err(|err| match err {
-                RevsetEvaluationError::StoreError(err) => err,
-                RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
-            })?;
-        let to_visit: Vec<_> = to_visit_revset.iter().commits(store).try_collect()?;
+            .map_err(|err| err.expect_backend_error())?;
+        let to_visit: Vec<_> = to_visit_revset
+            .iter()
+            .commits(store)
+            .try_collect()
+            // TODO: Return evaluation error to caller
+            .map_err(|err| err.expect_backend_error())?;
         drop(to_visit_revset);
         let to_visit_set: HashSet<CommitId> =
             to_visit.iter().map(|commit| commit.id().clone()).collect();

lib/src/revset.rs

@@ -28,7 +28,6 @@ use once_cell::sync::Lazy;
 use thiserror::Error;
 
 use crate::backend::BackendError;
-use crate::backend::BackendResult;
 use crate::backend::ChangeId;
 use crate::backend::CommitId;
 use crate::commit::Commit;
@@ -98,6 +97,15 @@ pub enum RevsetEvaluationError {
     Other(Box<dyn std::error::Error + Send + Sync>),
 }
 
+impl RevsetEvaluationError {
+    pub fn expect_backend_error(self) -> BackendError {
+        match self {
+            Self::StoreError(err) => err,
+            Self::Other(err) => panic!("Unexpected revset error: {err}"),
+        }
+    }
+}
+
 // assumes index has less than u64::MAX entries.
 pub const GENERATION_RANGE_FULL: Range<u64> = 0..u64::MAX;
 pub const GENERATION_RANGE_EMPTY: Range<u64> = 0..0;
@@ -2246,12 +2254,14 @@ pub struct RevsetCommitIterator<I> {
 }
 
 impl<I: Iterator<Item = CommitId>> Iterator for RevsetCommitIterator<I> {
-    type Item = BackendResult<Commit>;
+    type Item = Result<Commit, RevsetEvaluationError>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.iter
-            .next()
-            .map(|commit_id| self.store.get_commit(&commit_id))
+        self.iter.next().map(|commit_id| {
+            self.store
+                .get_commit(&commit_id)
+                .map_err(RevsetEvaluationError::StoreError)
+        })
     }
 }
 

lib/src/rewrite.rs

@@ -42,7 +42,6 @@ use crate::merged_tree::TreeDiffEntry;
 use crate::repo::MutableRepo;
 use crate::repo::Repo;
 use crate::repo_path::RepoPath;
-use crate::revset::RevsetEvaluationError;
 use crate::revset::RevsetExpression;
 use crate::revset::RevsetIteratorExt;
 use crate::settings::UserSettings;
@@ -500,13 +499,12 @@ pub fn move_commits(
         RevsetExpression::commits(target_commits.iter().ids().cloned().collect_vec())
             .connected()
             .evaluate_programmatic(mut_repo)
-            .map_err(|err| match err {
-                RevsetEvaluationError::StoreError(err) => err,
-                RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
-            })?
+            .map_err(|err| err.expect_backend_error())?
             .iter()
             .commits(mut_repo.store())
-            .try_collect()?;
+            .try_collect()
+            // TODO: Return evaluation error to caller
+            .map_err(|err| err.expect_backend_error())?;
 
     // Compute the parents of all commits in the connected target set, allowing only
     // commits in the target set as parents. The parents of each commit are
@@ -588,13 +586,12 @@ pub fn move_commits(
                         .children(),
                 )
                 .evaluate_programmatic(mut_repo)
-                .map_err(|err| match err {
-                    RevsetEvaluationError::StoreError(err) => err,
-                    RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
-                })?
+                .map_err(|err| err.expect_backend_error())?
                 .iter()
                 .commits(mut_repo.store())
-                .try_collect()?;
+                .try_collect()
+                // TODO: Return evaluation error to caller
+                .map_err(|err| err.expect_backend_error())?;
 
         // For all commits in the target set, compute its transitive descendant commits
         // which are outside of the target set by up to 1 generation.
@@ -717,13 +714,12 @@ pub fn move_commits(
     let to_visit_expression = RevsetExpression::commits(roots).descendants();
     let to_visit: Vec<_> = to_visit_expression
         .evaluate_programmatic(mut_repo)
-        .map_err(|err| match err {
-            RevsetEvaluationError::StoreError(err) => err,
-            RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
-        })?
+        .map_err(|err| err.expect_backend_error())?
         .iter()
         .commits(mut_repo.store())
-        .try_collect()?;
+        .try_collect()
+        // TODO: Return evaluation error to caller
+        .map_err(|err| err.expect_backend_error())?;
     let to_visit_commits: IndexMap<_, _> = to_visit
         .into_iter()
         .map(|commit| (commit.id().clone(), commit))