repo: let repo types directly have an OpStore

I'd like to make `ReadonlyView` and `MutableView` focused on just the
state of the view (i.e. the set of heads, git refs, etc.). The
responsibility for managing the `.jj/view/op_heads/` directory should
be moved out of it. This prepares for that.

lib/src/repo.rs

@@ -77,6 +77,13 @@ impl<'a, 'r> RepoRef<'a, 'r> {
         }
     }
 
+    pub fn op_store(&self) -> &'a Arc<dyn OpStore> {
+        match self {
+            RepoRef::Readonly(repo) => repo.op_store(),
+            RepoRef::Mutable(repo) => repo.op_store(),
+        }
+    }
+
     pub fn index(&self) -> IndexRef {
         match self {
             RepoRef::Readonly(repo) => IndexRef::Readonly(repo.index()),
@@ -103,6 +110,7 @@ pub struct ReadonlyRepo {
     repo_path: PathBuf,
     wc_path: PathBuf,
     store: Arc<StoreWrapper>,
+    op_store: Arc<dyn OpStore>,
     settings: RepoSettings,
     index_store: Arc<IndexStore>,
     index: Mutex<Option<Arc<ReadonlyIndex>>>,
@@ -208,7 +216,7 @@ impl ReadonlyRepo {
 
         let view = ReadonlyView::init(
             store.clone(),
-            op_store,
+            op_store.clone(),
             index_store.clone(),
             repo_path.join("view"),
             checkout_commit.id().clone(),
@@ -218,6 +226,7 @@ impl ReadonlyRepo {
             repo_path,
             wc_path,
             store,
+            op_store,
             settings: repo_settings,
             index_store,
             index: Mutex::new(None),
@@ -278,8 +287,8 @@ impl ReadonlyRepo {
         let mut locked_index = self.index.lock().unwrap();
         if locked_index.is_none() {
             let op_id = self.view.op_id().clone();
-            let op = self.view.op_store().read_operation(&op_id).unwrap();
-            let op = Operation::new(self.view.op_store().clone(), op_id, op);
+            let op = self.op_store.read_operation(&op_id).unwrap();
+            let op = Operation::new(self.op_store.clone(), op_id, op);
             locked_index.replace(self.index_store.get_index_at_op(&op, self.store.as_ref()));
         }
         locked_index.as_ref().unwrap().clone()
@@ -306,6 +315,10 @@ impl ReadonlyRepo {
         &self.store
     }
 
+    pub fn op_store(&self) -> &Arc<dyn OpStore> {
+        &self.op_store
+    }
+
     pub fn index_store(&self) -> &Arc<IndexStore> {
         &self.index_store
     }
@@ -433,6 +446,7 @@ impl RepoLoader {
             repo_path: self.repo_path,
             wc_path: self.wc_path,
             store: self.store,
+            op_store: self.op_store,
             settings: self.repo_settings,
             index_store: self.index_store,
             index: Mutex::new(None),
@@ -485,16 +499,20 @@ impl<'r> MutableRepo<'r> {
         RepoRef::Mutable(&self)
     }
 
+    pub fn base_repo(&self) -> &'r ReadonlyRepo {
+        self.repo
+    }
+
     pub fn store(&self) -> &Arc<StoreWrapper> {
         self.repo.store()
     }
 
-    pub fn index(&self) -> &MutableIndex {
-        &self.index
+    pub fn op_store(&self) -> &Arc<dyn OpStore> {
+        self.repo.op_store()
     }
 
-    pub fn base_repo(&self) -> &'r ReadonlyRepo {
-        self.repo
+    pub fn index(&self) -> &MutableIndex {
+        &self.index
     }
 
     pub fn view(&self) -> &MutableView {

lib/src/view.rs

@@ -36,7 +36,6 @@ pub enum ViewRef<'a> {
     Mutable(&'a MutableView),
 }
 
-// TODO: Move OpStore out of View?
 impl<'a> ViewRef<'a> {
     pub fn checkout(&self) -> &'a CommitId {
         match self {
@@ -66,10 +65,10 @@ impl<'a> ViewRef<'a> {
         }
     }
 
-    pub fn op_store(&self) -> Arc<dyn OpStore> {
+    fn op_store(&self) -> &Arc<dyn OpStore> {
         match self {
-            ViewRef::Readonly(view) => view.op_store(),
-            ViewRef::Mutable(view) => view.op_store(),
+            ViewRef::Readonly(view) => &view.op_store,
+            ViewRef::Mutable(view) => &view.op_store,
         }
     }
 
@@ -80,7 +79,7 @@ impl<'a> ViewRef<'a> {
         }
     }
 
-    pub fn get_operation(&self, id: &OperationId) -> OpStoreResult<Operation> {
+    fn get_operation(&self, id: &OperationId) -> OpStoreResult<Operation> {
         let data = self.op_store().read_operation(id)?;
         Ok(Operation::new(self.op_store().clone(), id.clone(), data))
     }
@@ -490,10 +489,6 @@ impl ReadonlyView {
         &self.data.git_refs
     }
 
-    pub fn op_store(&self) -> Arc<dyn OpStore> {
-        self.op_store.clone()
-    }
-
     pub fn op_id(&self) -> &OperationId {
         &self.op_id
     }
@@ -520,10 +515,6 @@ impl MutableView {
         &self.data.git_refs
     }
 
-    pub fn op_store(&self) -> Arc<dyn OpStore> {
-        self.op_store.clone()
-    }
-
     pub fn base_op_id(&self) -> &OperationId {
         &self.base_op_id
     }

lib/tests/test_bad_locking.rs

@@ -131,11 +131,7 @@ fn test_bad_locking_children(use_git: bool) {
     assert!(merged_repo.view().heads().contains(child1.id()));
     assert!(merged_repo.view().heads().contains(child2.id()));
     let op_id = merged_repo.view().op_id().clone();
-    let op = merged_repo
-        .view()
-        .op_store()
-        .read_operation(&op_id)
-        .unwrap();
+    let op = merged_repo.op_store().read_operation(&op_id).unwrap();
     assert_eq!(op.parents.len(), 2);
 }
 

lib/tests/test_commit_concurrent.rs

@@ -21,9 +21,8 @@ use std::sync::Arc;
 use test_case::test_case;
 
 fn count_non_merge_operations(repo: &ReadonlyRepo) -> u32 {
-    let view = repo.view();
-    let op_store = view.op_store();
-    let op_id = view.op_id().clone();
+    let op_store = repo.op_store();
+    let op_id = repo.view().op_id().clone();
     let mut num_ops = 0;
 
     for op_id in dag_walk::bfs(

src/commands.rs

@@ -207,7 +207,7 @@ fn resolve_single_op(repo: &ReadonlyRepo, op_str: &str) -> Result<Operation, Com
     if op_str == "@" {
         Ok(view.as_view_ref().base_op())
     } else {
-        resolve_single_op_from_store(&repo.view().op_store(), op_str)
+        resolve_single_op_from_store(&repo.op_store(), op_str)
     }
 }