index: make ChangeIdIndex methods fallible

cli/src/commit_templater.rs

@@ -46,6 +46,7 @@ use jj_lib::fileset::FilesetDiagnostics;
 use jj_lib::fileset::FilesetExpression;
 use jj_lib::id_prefix::IdPrefixContext;
 use jj_lib::id_prefix::IdPrefixIndex;
+use jj_lib::index::IndexResult;
 use jj_lib::matchers::Matcher;
 use jj_lib::merge::Diff;
 use jj_lib::merge::MergedTreeValue;
@@ -1147,10 +1148,11 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
         |language, _diagnostics, _build_ctx, self_property, function| {
             function.expect_no_arguments()?;
             let repo = language.repo;
-            let out_property = self_property.map(|commit| {
+            let out_property = self_property.and_then(|commit| {
                 // The given commit could be hidden in e.g. `jj evolog`.
-                let maybe_entries = repo.resolve_change_id(commit.change_id());
-                maybe_entries.map_or(0, |entries| entries.len()) > 1
+                let maybe_entries = repo.resolve_change_id(commit.change_id())?;
+                let divergent = maybe_entries.map_or(0, |entries| entries.len()) > 1;
+                Ok(divergent)
             });
             Ok(out_property.into_dyn_wrapped())
         },
@@ -1160,7 +1162,7 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
         |language, _diagnostics, _build_ctx, self_property, function| {
             function.expect_no_arguments()?;
             let repo = language.repo;
-            let out_property = self_property.map(|commit| commit.is_hidden(repo));
+            let out_property = self_property.and_then(|commit| Ok(commit.is_hidden(repo)?));
             Ok(out_property.into_dyn_wrapped())
         },
     );
@@ -1901,11 +1903,11 @@ fn builtin_repo_path_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, R
 }
 
 trait ShortestIdPrefixLen {
-    fn shortest_prefix_len(&self, repo: &dyn Repo, index: &IdPrefixIndex) -> usize;
+    fn shortest_prefix_len(&self, repo: &dyn Repo, index: &IdPrefixIndex) -> IndexResult<usize>;
 }
 
 impl ShortestIdPrefixLen for ChangeId {
-    fn shortest_prefix_len(&self, repo: &dyn Repo, index: &IdPrefixIndex) -> usize {
+    fn shortest_prefix_len(&self, repo: &dyn Repo, index: &IdPrefixIndex) -> IndexResult<usize> {
         index.shortest_change_prefix_len(repo, self)
     }
 }
@@ -1933,8 +1935,8 @@ fn builtin_change_id_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, C
 }
 
 impl ShortestIdPrefixLen for CommitId {
-    fn shortest_prefix_len(&self, repo: &dyn Repo, index: &IdPrefixIndex) -> usize {
-        index.shortest_commit_prefix_len(repo, self)
+    fn shortest_prefix_len(&self, repo: &dyn Repo, index: &IdPrefixIndex) -> IndexResult<usize> {
+        Ok(index.shortest_commit_prefix_len(repo, self))
     }
 }
 
@@ -2020,11 +2022,11 @@ where
             };
             // The length of the id printed will be the maximum of the minimum
             // `len` and the length of the shortest unique prefix.
-            let out_property = (self_property, len_property).map(move |(id, len)| {
-                let prefix_len = id.shortest_prefix_len(repo, &index);
+            let out_property = (self_property, len_property).and_then(move |(id, len)| {
+                let prefix_len = id.shortest_prefix_len(repo, &index)?;
                 let mut hex = format!("{id:.len$}", len = max(prefix_len, len.unwrap_or(0)));
                 let rest = hex.split_off(prefix_len);
-                ShortestIdPrefix { prefix: hex, rest }
+                Ok(ShortestIdPrefix { prefix: hex, rest })
             });
             Ok(out_property.into_dyn_wrapped())
         },

lib/src/commit.rs

@@ -33,6 +33,7 @@ use crate::backend::ChangeId;
 use crate::backend::CommitId;
 use crate::backend::MergedTreeId;
 use crate::backend::Signature;
+use crate::index::IndexResult;
 use crate::merged_tree::MergedTree;
 use crate::repo::Repo;
 use crate::rewrite::merge_commit_trees;
@@ -176,9 +177,9 @@ impl Commit {
     }
 
     ///  A commit is hidden if its commit id is not in the change id index.
-    pub fn is_hidden(&self, repo: &dyn Repo) -> bool {
-        let maybe_entries = repo.resolve_change_id(self.change_id());
-        maybe_entries.is_none_or(|entries| !entries.contains(&self.id))
+    pub fn is_hidden(&self, repo: &dyn Repo) -> IndexResult<bool> {
+        let maybe_entries = repo.resolve_change_id(self.change_id())?;
+        Ok(maybe_entries.is_none_or(|entries| !entries.contains(&self.id)))
     }
 
     /// A commit is discardable if it has no change from its parent, and an

lib/src/default_index/composite.rs

@@ -650,9 +650,9 @@ impl<I: AsCompositeIndex + Send + Sync> ChangeIdIndex for ChangeIdIndexImpl<I> {
     // If `SingleMatch` is returned, the commits including in the set are all
     // visible. `AmbiguousMatch` may be returned even if the prefix is unique
     // within the visible entries.
-    fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
+    fn resolve_prefix(&self, prefix: &HexPrefix) -> IndexResult<PrefixResolution<Vec<CommitId>>> {
         let index = self.index.as_composite().commits();
-        match index.resolve_change_id_prefix(prefix) {
+        let prefix = match index.resolve_change_id_prefix(prefix) {
             PrefixResolution::NoMatch => PrefixResolution::NoMatch,
             PrefixResolution::SingleMatch((_change_id, positions)) => {
                 debug_assert!(positions.is_sorted_by(|a, b| a < b));
@@ -670,7 +670,8 @@ impl<I: AsCompositeIndex + Send + Sync> ChangeIdIndex for ChangeIdIndexImpl<I> {
                 }
             }
             PrefixResolution::AmbiguousMatch => PrefixResolution::AmbiguousMatch,
-        }
+        };
+        Ok(prefix)
     }
 
     // Calculates the shortest prefix length of the given `change_id` among all
@@ -679,9 +680,9 @@ impl<I: AsCompositeIndex + Send + Sync> ChangeIdIndex for ChangeIdIndexImpl<I> {
     // The returned length is usually a few digits longer than the minimum
     // length necessary to disambiguate within the visible entries since hidden
     // entries are also considered when determining the prefix length.
-    fn shortest_unique_prefix_len(&self, change_id: &ChangeId) -> usize {
+    fn shortest_unique_prefix_len(&self, change_id: &ChangeId) -> IndexResult<usize> {
         let index = self.index.as_composite().commits();
-        index.shortest_unique_change_id_prefix_len(change_id)
+        Ok(index.shortest_unique_change_id_prefix_len(change_id))
     }
 }
 

lib/src/id_prefix.rs

@@ -25,6 +25,7 @@ use thiserror::Error;
 use crate::backend::ChangeId;
 use crate::backend::CommitId;
 use crate::hex_util;
+use crate::index::IndexResult;
 use crate::object_id::HexPrefix;
 use crate::object_id::ObjectId;
 use crate::object_id::PrefixResolution;
@@ -208,7 +209,7 @@ impl IdPrefixIndex<'_> {
         &self,
         repo: &dyn Repo,
         prefix: &HexPrefix,
-    ) -> PrefixResolution<Vec<CommitId>> {
+    ) -> IndexResult<PrefixResolution<Vec<CommitId>>> {
         if let Some(indexes) = self.indexes {
             let resolution = indexes
                 .change_index
@@ -218,15 +219,15 @@ impl IdPrefixIndex<'_> {
                     // Fall back to resolving in entire repo
                 }
                 PrefixResolution::SingleMatch(change_id) => {
-                    return match repo.resolve_change_id(&change_id) {
+                    return match repo.resolve_change_id(&change_id)? {
                         // There may be more commits with this change id outside the narrower sets.
-                        Some(commit_ids) => PrefixResolution::SingleMatch(commit_ids),
+                        Some(commit_ids) => Ok(PrefixResolution::SingleMatch(commit_ids)),
                         // The disambiguation set may contain hidden commits.
-                        None => PrefixResolution::NoMatch,
+                        None => Ok(PrefixResolution::NoMatch),
                     };
                 }
                 PrefixResolution::AmbiguousMatch => {
-                    return PrefixResolution::AmbiguousMatch;
+                    return Ok(PrefixResolution::AmbiguousMatch);
                 }
             }
         }
@@ -235,18 +236,30 @@ impl IdPrefixIndex<'_> {
 
     /// Returns the shortest length of a prefix of `change_id` that can still be
     /// resolved by `resolve_change_prefix()` and [`SymbolResolver`].
-    pub fn shortest_change_prefix_len(&self, repo: &dyn Repo, change_id: &ChangeId) -> usize {
-        let len = self.shortest_change_prefix_len_exact(repo, change_id);
-        disambiguate_prefix_with_refs(repo.view(), &change_id.to_string(), len)
+    pub fn shortest_change_prefix_len(
+        &self,
+        repo: &dyn Repo,
+        change_id: &ChangeId,
+    ) -> IndexResult<usize> {
+        let len = self.shortest_change_prefix_len_exact(repo, change_id)?;
+        Ok(disambiguate_prefix_with_refs(
+            repo.view(),
+            &change_id.to_string(),
+            len,
+        ))
     }
 
-    fn shortest_change_prefix_len_exact(&self, repo: &dyn Repo, change_id: &ChangeId) -> usize {
+    fn shortest_change_prefix_len_exact(
+        &self,
+        repo: &dyn Repo,
+        change_id: &ChangeId,
+    ) -> IndexResult<usize> {
         if let Some(indexes) = self.indexes
             && let Some(lookup) = indexes
                 .change_index
                 .lookup_exact(&*indexes.commit_change_ids, change_id)
         {
-            return lookup.shortest_unique_prefix_len();
+            return Ok(lookup.shortest_unique_prefix_len());
         }
         repo.shortest_unique_change_id_prefix_len(change_id)
     }

lib/src/index.rs

@@ -199,7 +199,7 @@ pub trait ChangeIdIndex: Send + Sync {
     /// Resolve an unambiguous change ID prefix to the commit IDs in the index.
     ///
     /// The order of the returned commit IDs is unspecified.
-    fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>>;
+    fn resolve_prefix(&self, prefix: &HexPrefix) -> IndexResult<PrefixResolution<Vec<CommitId>>>;
 
     /// This function returns the shortest length of a prefix of `key` that
     /// disambiguates it from every other key in the index.
@@ -215,5 +215,5 @@ pub trait ChangeIdIndex: Send + Sync {
     ///   order to disambiguate, you need every letter of the key *and* the
     ///   additional fact that it's the entire key). This case is extremely
     ///   unlikely for hashes with 12+ hexadecimal characters.
-    fn shortest_unique_prefix_len(&self, change_id: &ChangeId) -> usize;
+    fn shortest_unique_prefix_len(&self, change_id: &ChangeId) -> IndexResult<usize>;
 }

lib/src/repo.rs

@@ -52,6 +52,7 @@ use crate::file_util::IoResultExt as _;
 use crate::file_util::PathError;
 use crate::index::ChangeIdIndex;
 use crate::index::Index;
+use crate::index::IndexResult;
 use crate::index::IndexStore;
 use crate::index::IndexStoreError;
 use crate::index::MutableIndex;
@@ -125,19 +126,25 @@ pub trait Repo {
 
     fn submodule_store(&self) -> &Arc<dyn SubmoduleStore>;
 
-    fn resolve_change_id(&self, change_id: &ChangeId) -> Option<Vec<CommitId>> {
+    fn resolve_change_id(&self, change_id: &ChangeId) -> IndexResult<Option<Vec<CommitId>>> {
         // Replace this if we added more efficient lookup method.
         let prefix = HexPrefix::from_id(change_id);
-        match self.resolve_change_id_prefix(&prefix) {
-            PrefixResolution::NoMatch => None,
-            PrefixResolution::SingleMatch(entries) => Some(entries),
+        match self.resolve_change_id_prefix(&prefix)? {
+            PrefixResolution::NoMatch => Ok(None),
+            PrefixResolution::SingleMatch(entries) => Ok(Some(entries)),
             PrefixResolution::AmbiguousMatch => panic!("complete change_id should be unambiguous"),
         }
     }
 
-    fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>>;
+    fn resolve_change_id_prefix(
+        &self,
+        prefix: &HexPrefix,
+    ) -> IndexResult<PrefixResolution<Vec<CommitId>>>;
 
-    fn shortest_unique_change_id_prefix_len(&self, target_id_bytes: &ChangeId) -> usize;
+    fn shortest_unique_change_id_prefix_len(
+        &self,
+        target_id_bytes: &ChangeId,
+    ) -> IndexResult<usize>;
 }
 
 pub struct ReadonlyRepo {
@@ -351,11 +358,14 @@ impl Repo for ReadonlyRepo {
         self.loader.submodule_store()
     }
 
-    fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
+    fn resolve_change_id_prefix(
+        &self,
+        prefix: &HexPrefix,
+    ) -> IndexResult<PrefixResolution<Vec<CommitId>>> {
         self.change_id_index().resolve_prefix(prefix)
     }
 
-    fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
+    fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> IndexResult<usize> {
         self.change_id_index().shortest_unique_prefix_len(target_id)
     }
 }
@@ -1976,12 +1986,15 @@ impl Repo for MutableRepo {
         self.base_repo.submodule_store()
     }
 
-    fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> {
+    fn resolve_change_id_prefix(
+        &self,
+        prefix: &HexPrefix,
+    ) -> IndexResult<PrefixResolution<Vec<CommitId>>> {
         let change_id_index = self.index.change_id_index(&mut self.view().heads().iter());
         change_id_index.resolve_prefix(prefix)
     }
 
-    fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize {
+    fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> IndexResult<usize> {
         let change_id_index = self.index.change_id_index(&mut self.view().heads().iter());
         change_id_index.shortest_unique_prefix_len(target_id)
     }

lib/src/revset.rs

@@ -2660,7 +2660,10 @@ impl ChangePrefixResolver<'_> {
             .transpose()
             .map_err(|err| RevsetResolutionError::Other(err.into()))?
             .unwrap_or(IdPrefixIndex::empty());
-        match index.resolve_change_prefix(repo, prefix) {
+        match index
+            .resolve_change_prefix(repo, prefix)
+            .map_err(|err| RevsetResolutionError::Other(err.into()))?
+        {
             PrefixResolution::AmbiguousMatch => Err(
                 RevsetResolutionError::AmbiguousChangeIdPrefix(prefix.reverse_hex()),
             ),