index: organize Index and IndexStore errors into enums

lib/src/default_index/composite.rs

@@ -41,10 +41,9 @@ use super::revset_engine;
 use crate::backend::ChangeId;
 use crate::backend::CommitId;
 use crate::hex_util;
-use crate::index::AllHeadsForGcUnsupported;
 use crate::index::ChangeIdIndex;
 use crate::index::Index;
-use crate::index::IndexError;
+use crate::index::IndexResult;
 use crate::object_id::HexPrefix;
 use crate::object_id::ObjectId as _;
 use crate::object_id::PrefixResolution;
@@ -590,23 +589,21 @@ impl Index for CompositeIndex {
         self.commits().common_ancestors(set1, set2)
     }
 
-    fn all_heads_for_gc(
-        &self,
-    ) -> Result<Box<dyn Iterator<Item = CommitId> + '_>, AllHeadsForGcUnsupported> {
+    fn all_heads_for_gc(&self) -> IndexResult<Box<dyn Iterator<Item = CommitId> + '_>> {
         Ok(Box::new(self.commits().all_heads()))
     }
 
     fn heads(
         &self,
         candidate_ids: &mut dyn Iterator<Item = &CommitId>,
-    ) -> Result<Vec<CommitId>, IndexError> {
+    ) -> IndexResult<Vec<CommitId>> {
         Ok(self.commits().heads(candidate_ids))
     }
 
     fn changed_paths_in_commit(
         &self,
         commit_id: &CommitId,
-    ) -> Result<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>, IndexError> {
+    ) -> IndexResult<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>> {
         let Some(paths) = self
             .commits()
             .commit_id_to_pos(commit_id)

lib/src/default_index/mutable.rs

@@ -56,10 +56,10 @@ use crate::commit::Commit;
 use crate::file_util::IoResultExt as _;
 use crate::file_util::PathError;
 use crate::file_util::persist_content_addressed_temp_file;
-use crate::index::AllHeadsForGcUnsupported;
 use crate::index::ChangeIdIndex;
 use crate::index::Index;
 use crate::index::IndexError;
+use crate::index::IndexResult;
 use crate::index::MutableIndex;
 use crate::index::ReadonlyIndex;
 use crate::object_id::HexPrefix;
@@ -565,23 +565,18 @@ impl Index for DefaultMutableIndex {
         self.0.common_ancestors(set1, set2)
     }
 
-    fn all_heads_for_gc(
-        &self,
-    ) -> Result<Box<dyn Iterator<Item = CommitId> + '_>, AllHeadsForGcUnsupported> {
+    fn all_heads_for_gc(&self) -> IndexResult<Box<dyn Iterator<Item = CommitId> + '_>> {
         self.0.all_heads_for_gc()
     }
 
-    fn heads(
-        &self,
-        candidates: &mut dyn Iterator<Item = &CommitId>,
-    ) -> Result<Vec<CommitId>, IndexError> {
+    fn heads(&self, candidates: &mut dyn Iterator<Item = &CommitId>) -> IndexResult<Vec<CommitId>> {
         self.0.heads(candidates)
     }
 
     fn changed_paths_in_commit(
         &self,
         commit_id: &CommitId,
-    ) -> Result<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>, IndexError> {
+    ) -> IndexResult<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>> {
         self.0.changed_paths_in_commit(commit_id)
     }
 
@@ -606,10 +601,10 @@ impl MutableIndex for DefaultMutableIndex {
         Box::new(ChangeIdIndexImpl::new(self, heads))
     }
 
-    fn add_commit(&mut self, commit: &Commit) -> Result<(), IndexError> {
+    fn add_commit(&mut self, commit: &Commit) -> IndexResult<()> {
         Self::add_commit(self, commit)
             .block_on()
-            .map_err(|err| IndexError(err.into()))
+            .map_err(|err| IndexError::Other(err.into()))
     }
 
     fn merge_in(&mut self, other: &dyn ReadonlyIndex) {

lib/src/default_index/readonly.rs

@@ -47,10 +47,9 @@ use super::revset_engine::RevsetImpl;
 use crate::backend::ChangeId;
 use crate::backend::CommitId;
 use crate::graph::GraphNode;
-use crate::index::AllHeadsForGcUnsupported;
 use crate::index::ChangeIdIndex;
 use crate::index::Index;
-use crate::index::IndexError;
+use crate::index::IndexResult;
 use crate::index::MutableIndex;
 use crate::index::ReadonlyIndex;
 use crate::object_id::HexPrefix;
@@ -733,23 +732,18 @@ impl Index for DefaultReadonlyIndex {
         self.0.common_ancestors(set1, set2)
     }
 
-    fn all_heads_for_gc(
-        &self,
-    ) -> Result<Box<dyn Iterator<Item = CommitId> + '_>, AllHeadsForGcUnsupported> {
+    fn all_heads_for_gc(&self) -> IndexResult<Box<dyn Iterator<Item = CommitId> + '_>> {
         self.0.all_heads_for_gc()
     }
 
-    fn heads(
-        &self,
-        candidates: &mut dyn Iterator<Item = &CommitId>,
-    ) -> Result<Vec<CommitId>, IndexError> {
+    fn heads(&self, candidates: &mut dyn Iterator<Item = &CommitId>) -> IndexResult<Vec<CommitId>> {
         self.0.heads(candidates)
     }
 
     fn changed_paths_in_commit(
         &self,
         commit_id: &CommitId,
-    ) -> Result<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>, IndexError> {
+    ) -> IndexResult<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>> {
         self.0.changed_paths_in_commit(commit_id)
     }
 

lib/src/default_index/store.rs

@@ -51,9 +51,9 @@ use crate::file_util::IoResultExt as _;
 use crate::file_util::PathError;
 use crate::file_util::persist_temp_file;
 use crate::index::Index as _;
-use crate::index::IndexReadError;
 use crate::index::IndexStore;
-use crate::index::IndexWriteError;
+use crate::index::IndexStoreError;
+use crate::index::IndexStoreResult;
 use crate::index::MutableIndex;
 use crate::index::ReadonlyIndex;
 use crate::object_id::ObjectId as _;
@@ -562,7 +562,7 @@ impl IndexStore for DefaultIndexStore {
         &self,
         op: &Operation,
         store: &Arc<Store>,
-    ) -> Result<Box<dyn ReadonlyIndex>, IndexReadError> {
+    ) -> IndexStoreResult<Box<dyn ReadonlyIndex>> {
         let field_lengths = FieldLengths {
             commit_id: store.commit_id_length(),
             change_id: store.change_id_length(),
@@ -591,26 +591,27 @@ impl IndexStore for DefaultIndexStore {
                         eprintln!("{err} (maybe the format has changed): {error}. Reindexing...");
                     }
                 }
-                self.reinit().map_err(|err| IndexReadError(err.into()))?;
+                self.reinit()
+                    .map_err(|err| IndexStoreError::Read(err.into()))?;
                 self.build_index_at_operation(op, store).block_on()
             }
             result => result,
         }
-        .map_err(|err| IndexReadError(err.into()))?;
+        .map_err(|err| IndexStoreError::Read(err.into()))?;
         Ok(Box::new(index))
     }
 
     fn write_index(
         &self,
         index: Box<dyn MutableIndex>,
         op: &Operation,
-    ) -> Result<Box<dyn ReadonlyIndex>, IndexWriteError> {
+    ) -> IndexStoreResult<Box<dyn ReadonlyIndex>> {
         let index: Box<DefaultMutableIndex> = index
             .downcast()
             .expect("index to merge in must be a DefaultMutableIndex");
         let index = self
             .save_mutable_index(*index, op.id())
-            .map_err(|err| IndexWriteError(err.into()))?;
+            .map_err(|err| IndexStoreError::Write(err.into()))?;
         Ok(Box::new(index))
     }
 }

lib/src/index.rs

@@ -32,26 +32,34 @@ use crate::revset::Revset;
 use crate::revset::RevsetEvaluationError;
 use crate::store::Store;
 
-/// Returned if an error occurs while reading an index from the [`IndexStore`].
+/// Returned by [`IndexStore`] in the event of an error.
 #[derive(Debug, Error)]
-#[error(transparent)]
-pub struct IndexReadError(pub Box<dyn std::error::Error + Send + Sync>);
+pub enum IndexStoreError {
+    /// Error reading a [`ReadonlyIndex`] from the [`IndexStore`].
+    #[error("Failed to read index")]
+    Read(#[source] Box<dyn std::error::Error + Send + Sync>),
+    /// Error writing a [`MutableIndex`] to the [`IndexStore`].
+    #[error("Failed to write index")]
+    Write(#[source] Box<dyn std::error::Error + Send + Sync>),
+}
 
-/// Returned if an error occurs while writing an index to the [`IndexStore`].
-#[derive(Debug, Error)]
-#[error(transparent)]
-pub struct IndexWriteError(pub Box<dyn std::error::Error + Send + Sync>);
+/// Result of [`IndexStore`] operations.
+pub type IndexStoreResult<T> = Result<T, IndexStoreError>;
 
-/// Returned by [`Index`] backend in case of an error.
+/// Returned by [`Index`] backend in the event of an error.
 #[derive(Debug, Error)]
-#[error(transparent)]
-pub struct IndexError(pub Box<dyn std::error::Error + Send + Sync>);
+pub enum IndexError {
+    /// Error returned if [`Index::all_heads_for_gc()`] is not supported by the
+    /// [`Index`] backend.
+    #[error("Cannot collect all heads by index of this type")]
+    AllHeadsForGcUnsupported,
+    /// Some other index error.
+    #[error(transparent)]
+    Other(Box<dyn std::error::Error + Send + Sync>),
+}
 
-/// An error returned if `Index::all_heads_for_gc()` is not supported by the
-/// index backend.
-#[derive(Debug, Error)]
-#[error("Cannot collect all heads by index of this type")]
-pub struct AllHeadsForGcUnsupported;
+/// Result of [`Index`] operations.
+pub type IndexResult<T> = Result<T, IndexError>;
 
 /// Defines the interface for types that provide persistent storage for an
 /// index.
@@ -66,15 +74,15 @@ pub trait IndexStore: Any + Send + Sync + Debug {
         &self,
         op: &Operation,
         store: &Arc<Store>,
-    ) -> Result<Box<dyn ReadonlyIndex>, IndexReadError>;
+    ) -> IndexStoreResult<Box<dyn ReadonlyIndex>>;
 
     /// Writes `index` to the index store and returns a read-only version of the
     /// index.
     fn write_index(
         &self,
         index: Box<dyn MutableIndex>,
         op: &Operation,
-    ) -> Result<Box<dyn ReadonlyIndex>, IndexWriteError>;
+    ) -> IndexStoreResult<Box<dyn ReadonlyIndex>>;
 }
 
 impl dyn IndexStore {
@@ -119,24 +127,19 @@ pub trait Index: Send + Sync {
     /// that should be preserved on garbage collection.
     ///
     /// The iteration order is unspecified.
-    fn all_heads_for_gc(
-        &self,
-    ) -> Result<Box<dyn Iterator<Item = CommitId> + '_>, AllHeadsForGcUnsupported>;
+    fn all_heads_for_gc(&self) -> IndexResult<Box<dyn Iterator<Item = CommitId> + '_>>;
 
     /// Returns the subset of commit IDs in `candidates` which are not ancestors
     /// of other commits in `candidates`. If a commit id is duplicated in the
     /// `candidates` list it will appear at most once in the output.
-    fn heads(
-        &self,
-        candidates: &mut dyn Iterator<Item = &CommitId>,
-    ) -> Result<Vec<CommitId>, IndexError>;
+    fn heads(&self, candidates: &mut dyn Iterator<Item = &CommitId>) -> IndexResult<Vec<CommitId>>;
 
     /// Returns iterator over paths changed at the specified commit. The paths
     /// are sorted. Returns `None` if the commit wasn't indexed.
     fn changed_paths_in_commit(
         &self,
         commit_id: &CommitId,
-    ) -> Result<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>, IndexError>;
+    ) -> IndexResult<Option<Box<dyn Iterator<Item = RepoPathBuf> + '_>>>;
 
     /// Resolves the revset `expression` against the index and corresponding
     /// `store`.
@@ -173,7 +176,7 @@ pub trait MutableIndex: Any {
         heads: &mut dyn Iterator<Item = &CommitId>,
     ) -> Box<dyn ChangeIdIndex + '_>;
 
-    fn add_commit(&mut self, commit: &Commit) -> Result<(), IndexError>;
+    fn add_commit(&mut self, commit: &Commit) -> IndexResult<()>;
 
     fn merge_in(&mut self, other: &dyn ReadonlyIndex);
 }

lib/src/repo.rs

@@ -52,8 +52,8 @@ use crate::file_util::IoResultExt as _;
 use crate::file_util::PathError;
 use crate::index::ChangeIdIndex;
 use crate::index::Index;
-use crate::index::IndexReadError;
 use crate::index::IndexStore;
+use crate::index::IndexStoreError;
 use crate::index::MutableIndex;
 use crate::index::ReadonlyIndex;
 use crate::merge::MergeBuilder;
@@ -630,7 +630,7 @@ pub enum RepoLoaderError {
     #[error(transparent)]
     Backend(#[from] BackendError),
     #[error(transparent)]
-    IndexRead(#[from] IndexReadError),
+    IndexStore(#[from] IndexStoreError),
     #[error(transparent)]
     OpHeadResolution(#[from] OpHeadResolutionError),
     #[error(transparent)]

lib/src/revset.rs

@@ -2480,7 +2480,7 @@ fn reload_repo_at_operation(
         .map_err(|err| RevsetResolutionError::Other(err.into()))?;
     base_repo.reload_at(&operation).map_err(|err| match err {
         RepoLoaderError::Backend(err) => RevsetResolutionError::Backend(err),
-        RepoLoaderError::IndexRead(_)
+        RepoLoaderError::IndexStore(_)
         | RepoLoaderError::OpHeadResolution(_)
         | RepoLoaderError::OpHeadsStoreError(_)
         | RepoLoaderError::OpStore(_)

lib/src/rewrite.rs

@@ -36,7 +36,7 @@ use crate::commit::Commit;
 use crate::commit::CommitIteratorExt as _;
 use crate::commit_builder::CommitBuilder;
 use crate::index::Index;
-use crate::index::IndexError;
+use crate::index::IndexResult;
 use crate::matchers::Matcher;
 use crate::matchers::Visit;
 use crate::merge::Merge;
@@ -218,7 +218,7 @@ impl<'repo> CommitRewriter<'repo> {
 
     /// If a merge commit would end up with one parent being an ancestor of the
     /// other, then filter out the ancestor.
-    pub fn simplify_ancestor_merge(&mut self) -> Result<(), IndexError> {
+    pub fn simplify_ancestor_merge(&mut self) -> IndexResult<()> {
         let head_set: HashSet<_> = self
             .mut_repo
             .index()

lib/src/transaction.rs

@@ -22,7 +22,7 @@ use thiserror::Error;
 
 use crate::backend::Timestamp;
 use crate::dag_walk;
-use crate::index::IndexWriteError;
+use crate::index::IndexStoreError;
 use crate::index::ReadonlyIndex;
 use crate::op_heads_store::OpHeadsStore;
 use crate::op_heads_store::OpHeadsStoreError;
@@ -43,7 +43,7 @@ use crate::view::View;
 #[derive(Debug, Error)]
 #[error("Failed to commit new operation")]
 pub enum TransactionCommitError {
-    IndexWrite(#[from] IndexWriteError),
+    IndexStore(#[from] IndexStoreError),
     OpHeadsStore(#[from] OpHeadsStoreError),
     OpStore(#[from] OpStoreError),
 }