index: extract a ReadonlyIndex trait

I didn't make `ReadonlyIndex` extend `Index` because it needed an
`as_index()` to convert to `&dyn Index` trait object
anyway. Separating the types also gives us flexibility to implement
the two traits on different types.

lib/src/default_index_store.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::any::Any;
 use std::cmp::{max, min, Ordering};
 use std::collections::{BTreeMap, BTreeSet, BinaryHeap, Bound, HashMap, HashSet};
 use std::fmt::{Debug, Formatter};
@@ -33,7 +34,9 @@ use thiserror::Error;
 use crate::backend::{ChangeId, CommitId, ObjectId};
 use crate::commit::Commit;
 use crate::file_util::persist_content_addressed_temp_file;
-use crate::index::{HexPrefix, Index, IndexStore, IndexWriteError, PrefixResolution};
+use crate::index::{
+    HexPrefix, Index, IndexStore, IndexWriteError, PrefixResolution, ReadonlyIndex,
+};
 #[cfg(not(feature = "map_first_last"))]
 // This import is used on Rust 1.61, but not on recent version.
 // TODO: Remove it when our MSRV becomes recent enough.
@@ -166,7 +169,7 @@ impl IndexStore for DefaultIndexStore {
         "default"
     }
 
-    fn get_index_at_op(&self, op: &Operation, store: &Arc<Store>) -> ReadonlyIndex {
+    fn get_index_at_op(&self, op: &Operation, store: &Arc<Store>) -> Box<dyn ReadonlyIndex> {
         let op_id_hex = op.id().hex();
         let op_id_file = self.dir.join("operations").join(op_id_hex);
         let index_impl = if op_id_file.exists() {
@@ -189,14 +192,14 @@ impl IndexStore for DefaultIndexStore {
         } else {
             self.index_at_operation(store, op).unwrap()
         };
-        ReadonlyIndex(index_impl)
+        Box::new(ReadonlyIndexWrapper(index_impl))
     }
 
     fn write_index(
         &self,
         index: MutableIndex,
         op_id: &OperationId,
-    ) -> Result<ReadonlyIndex, IndexWriteError> {
+    ) -> Result<Box<dyn ReadonlyIndex>, IndexWriteError> {
         let index = index.save_in(self.dir.clone()).map_err(|err| {
             IndexWriteError::Other(format!("Failed to write commit index file: {err:?}"))
         })?;
@@ -206,7 +209,7 @@ impl IndexStore for DefaultIndexStore {
                     "Failed to associate commit index file with a operation {op_id:?}: {err:?}"
                 ))
             })?;
-        Ok(ReadonlyIndex(index))
+        Ok(Box::new(ReadonlyIndexWrapper(index)))
     }
 }
 
@@ -401,14 +404,18 @@ pub struct ReadonlyIndexImpl {
     overflow_parent: Vec<u8>,
 }
 
-pub struct ReadonlyIndex(Arc<ReadonlyIndexImpl>);
+pub struct ReadonlyIndexWrapper(Arc<ReadonlyIndexImpl>);
 
-impl ReadonlyIndex {
-    pub fn as_index(&self) -> &dyn Index {
+impl ReadonlyIndex for ReadonlyIndexWrapper {
+    fn as_any(&self) -> &dyn Any {
+        self
+    }
+
+    fn as_index(&self) -> &dyn Index {
         self.0.as_ref()
     }
 
-    pub fn start_modification(&self) -> MutableIndex {
+    fn start_modification(&self) -> MutableIndex {
         MutableIndex::incremental(self.0.clone())
     }
 }
@@ -518,7 +525,12 @@ impl MutableIndex {
         }
     }
 
-    pub fn merge_in(&mut self, other: &ReadonlyIndex) {
+    pub fn merge_in(&mut self, other: &dyn ReadonlyIndex) {
+        let other = other
+            .as_any()
+            .downcast_ref::<ReadonlyIndexWrapper>()
+            .expect("index to merge in must be a ReadonlyIndexWrapper");
+
         let mut maybe_own_ancestor = self.parent_file.clone();
         let mut maybe_other_ancestor = Some(other.0.clone());
         let mut files_to_add = vec![];
@@ -1758,7 +1770,7 @@ impl Index for ReadonlyIndexImpl {
     }
 }
 
-impl Index for ReadonlyIndex {
+impl Index for ReadonlyIndexWrapper {
     fn num_commits(&self) -> u32 {
         self.0.num_commits()
     }

lib/src/index.rs

@@ -12,15 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::any::Any;
 use std::fmt::Debug;
 use std::sync::Arc;
 
 use thiserror::Error;
 
 use crate::backend::{CommitId, ObjectId};
-use crate::default_index_store::{
-    IndexEntry, IndexPosition, IndexStats, MutableIndex, ReadonlyIndex, RevWalk,
-};
+use crate::default_index_store::{IndexEntry, IndexPosition, IndexStats, MutableIndex, RevWalk};
 use crate::op_store::OperationId;
 use crate::operation::Operation;
 use crate::store::Store;
@@ -34,13 +33,13 @@ pub enum IndexWriteError {
 pub trait IndexStore: Send + Sync + Debug {
     fn name(&self) -> &str;
 
-    fn get_index_at_op(&self, op: &Operation, store: &Arc<Store>) -> ReadonlyIndex;
+    fn get_index_at_op(&self, op: &Operation, store: &Arc<Store>) -> Box<dyn ReadonlyIndex>;
 
     fn write_index(
         &self,
         index: MutableIndex,
         op_id: &OperationId,
-    ) -> Result<ReadonlyIndex, IndexWriteError>;
+    ) -> Result<Box<dyn ReadonlyIndex>, IndexWriteError>;
 }
 
 pub trait Index {
@@ -72,6 +71,14 @@ pub trait Index {
     fn topo_order(&self, input: &mut dyn Iterator<Item = &CommitId>) -> Vec<IndexEntry>;
 }
 
+pub trait ReadonlyIndex: Send + Sync {
+    fn as_any(&self) -> &dyn Any;
+
+    fn as_index(&self) -> &dyn Index;
+
+    fn start_modification(&self) -> MutableIndex;
+}
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct HexPrefix {
     // For odd-length prefix, lower 4 bits of the last byte is padded with 0

lib/src/repo.rs

@@ -28,11 +28,9 @@ use crate::backend::{Backend, BackendError, BackendResult, ChangeId, CommitId, O
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
 use crate::dag_walk::topo_order_reverse;
-use crate::default_index_store::{
-    DefaultIndexStore, IndexEntry, IndexPosition, MutableIndex, ReadonlyIndex,
-};
+use crate::default_index_store::{DefaultIndexStore, IndexEntry, IndexPosition, MutableIndex};
 use crate::git_backend::GitBackend;
-use crate::index::{HexPrefix, Index, IndexStore, PrefixResolution};
+use crate::index::{HexPrefix, Index, IndexStore, PrefixResolution, ReadonlyIndex};
 use crate::local_backend::LocalBackend;
 use crate::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore};
 use crate::op_store::{BranchTarget, OpStore, OperationId, RefTarget, WorkspaceId};
@@ -81,7 +79,7 @@ pub struct ReadonlyRepo {
     operation: Operation,
     settings: RepoSettings,
     index_store: Arc<dyn IndexStore>,
-    index: OnceCell<ReadonlyIndex>,
+    index: OnceCell<Box<dyn ReadonlyIndex>>,
     // TODO: This should eventually become part of the index and not be stored fully in memory.
     change_id_index: OnceCell<ChangeIdIndex>,
     view: View,
@@ -210,17 +208,19 @@ impl ReadonlyRepo {
         &self.view
     }
 
-    pub fn readonly_index(&self) -> &ReadonlyIndex {
-        self.index.get_or_init(|| {
-            self.index_store
-                .get_index_at_op(&self.operation, &self.store)
-        })
+    pub fn readonly_index(&self) -> &dyn ReadonlyIndex {
+        self.index
+            .get_or_init(|| {
+                self.index_store
+                    .get_index_at_op(&self.operation, &self.store)
+            })
+            .as_ref()
     }
 
     fn change_id_index(&self) -> &ChangeIdIndex {
         self.change_id_index.get_or_init(|| {
             let heads = self.view().heads().iter().cloned().collect_vec();
-            let walk = self.readonly_index().walk_revs(&heads, &[]);
+            let walk = self.readonly_index().as_index().walk_revs(&heads, &[]);
             IdIndex::from_vec(
                 walk.map(|entry| (entry.change_id(), entry.position()))
                     .collect(),
@@ -574,7 +574,7 @@ impl RepoLoader {
         &self,
         operation: Operation,
         view: View,
-        index: ReadonlyIndex,
+        index: Box<dyn ReadonlyIndex>,
     ) -> Arc<ReadonlyRepo> {
         let repo = ReadonlyRepo {
             repo_path: self.repo_path.clone(),
@@ -632,7 +632,11 @@ pub struct MutableRepo {
 }
 
 impl MutableRepo {
-    pub fn new(base_repo: Arc<ReadonlyRepo>, index: &ReadonlyIndex, view: &View) -> MutableRepo {
+    pub fn new(
+        base_repo: Arc<ReadonlyRepo>,
+        index: &dyn ReadonlyIndex,
+        view: &View,
+    ) -> MutableRepo {
         let mut_view = view.clone();
         let mut_index = index.start_modification();
         MutableRepo {

lib/src/transaction.rs

@@ -16,7 +16,7 @@ use std::sync::Arc;
 
 use crate::backend::Timestamp;
 use crate::dag_walk::closest_common_node;
-use crate::default_index_store::ReadonlyIndex;
+use crate::index::ReadonlyIndex;
 use crate::op_store;
 use crate::op_store::OperationMetadata;
 use crate::operation::Operation;
@@ -140,7 +140,7 @@ pub fn create_op_metadata(user_settings: &UserSettings, description: String) ->
 struct NewRepoData {
     operation: Operation,
     view: View,
-    index: ReadonlyIndex,
+    index: Box<dyn ReadonlyIndex>,
 }
 
 pub struct UnpublishedOperation {
@@ -154,7 +154,7 @@ impl UnpublishedOperation {
         repo_loader: RepoLoader,
         operation: Operation,
         view: View,
-        index: ReadonlyIndex,
+        index: Box<dyn ReadonlyIndex>,
     ) -> Self {
         let data = Some(NewRepoData {
             operation,