From 30a348344b98455be5f41e76eaf68af45e6dacc5 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 7 Oct 2024 10:05:54 +0900 Subject: [PATCH] repo: pack common ReadonlyRepo fields into RepoLoader I'll add a few helper methods to RepoLoader. It seems also nicer that repo.loader() doesn't allocate new RepoLoader. --- lib/src/repo.rs | 84 ++++++++++++++++-------------------- lib/src/transaction.rs | 2 +- lib/src/workspace.rs | 9 +++- lib/tests/test_operations.rs | 2 +- lib/testutils/src/lib.rs | 2 +- 5 files changed, 48 insertions(+), 51 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index c01f05c24..dc3686be1 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -122,13 +122,8 @@ pub trait Repo { } pub struct ReadonlyRepo { - store: Arc, - op_store: Arc, - op_heads_store: Arc, + loader: RepoLoader, operation: Operation, - settings: RepoSettings, - index_store: Arc, - submodule_store: Arc, index: Box, change_id_index: OnceCell>, // TODO: This should eventually become part of the index and not be stored fully in memory. @@ -138,7 +133,7 @@ pub struct ReadonlyRepo { impl Debug for ReadonlyRepo { fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> { f.debug_struct("ReadonlyRepo") - .field("store", &self.store) + .field("store", &self.loader.store) .finish_non_exhaustive() } } @@ -222,31 +217,37 @@ impl ReadonlyRepo { .context(&submodule_store_type_path)?; let submodule_store = Arc::from(submodule_store); - let root_operation_data = op_store - .read_operation(op_store.root_operation_id()) + let loader = RepoLoader { + repo_settings, + store, + op_store, + op_heads_store, + index_store, + submodule_store, + }; + + let root_operation_data = loader + .op_store + .read_operation(loader.op_store.root_operation_id()) .expect("failed to read root operation"); let root_operation = Operation::new( - op_store.clone(), - op_store.root_operation_id().clone(), + loader.op_store.clone(), + loader.op_store.root_operation_id().clone(), root_operation_data, ); let root_view = root_operation.view().expect("failed to read root view"); - let index = index_store - .get_index_at_op(&root_operation, &store) + let index = loader + .index_store + .get_index_at_op(&root_operation, &loader.store) // If the root op index couldn't be read, the index backend wouldn't // be initialized properly. .map_err(|err| BackendInitError(err.into()))?; let repo = Arc::new(ReadonlyRepo { - store, - op_store, - op_heads_store, + loader, operation: root_operation, - settings: repo_settings, - index_store, index, change_id_index: OnceCell::new(), view: root_view, - submodule_store, }); let mut tx = repo.start_transaction(user_settings); tx.repo_mut() @@ -255,15 +256,8 @@ impl ReadonlyRepo { Ok(tx.commit("initialize repo")) } - pub fn loader(&self) -> RepoLoader { - RepoLoader { - repo_settings: self.settings.clone(), - store: self.store.clone(), - op_store: self.op_store.clone(), - op_heads_store: self.op_heads_store.clone(), - index_store: self.index_store.clone(), - submodule_store: self.submodule_store.clone(), - } + pub fn loader(&self) -> &RepoLoader { + &self.loader } pub fn op_id(&self) -> &OperationId { @@ -292,15 +286,15 @@ impl ReadonlyRepo { } pub fn op_heads_store(&self) -> &Arc { - &self.op_heads_store + self.loader.op_heads_store() } pub fn index_store(&self) -> &Arc { - &self.index_store + self.loader.index_store() } pub fn settings(&self) -> &RepoSettings { - &self.settings + self.loader.settings() } pub fn start_transaction( @@ -326,11 +320,11 @@ impl ReadonlyRepo { impl Repo for ReadonlyRepo { fn store(&self) -> &Arc { - &self.store + self.loader.store() } fn op_store(&self) -> &Arc { - &self.op_store + self.loader.op_store() } fn index(&self) -> &dyn Index { @@ -342,7 +336,7 @@ impl Repo for ReadonlyRepo { } fn submodule_store(&self) -> &Arc { - &self.submodule_store + self.loader.submodule_store() } fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution> { @@ -685,6 +679,10 @@ impl RepoLoader { }) } + pub fn settings(&self) -> &RepoSettings { + &self.repo_settings + } + pub fn store(&self) -> &Arc { &self.store } @@ -701,6 +699,10 @@ impl RepoLoader { &self.op_heads_store } + pub fn submodule_store(&self) -> &Arc { + &self.submodule_store + } + pub fn load_at_head( &self, user_settings: &UserSettings, @@ -727,13 +729,8 @@ impl RepoLoader { index: Box, ) -> Arc { let repo = ReadonlyRepo { - store: self.store.clone(), - op_store: self.op_store.clone(), - op_heads_store: self.op_heads_store.clone(), + loader: self.clone(), operation, - settings: self.repo_settings.clone(), - index_store: self.index_store.clone(), - submodule_store: self.submodule_store.clone(), index, change_id_index: OnceCell::new(), view, @@ -796,13 +793,8 @@ impl RepoLoader { ) -> Result, RepoLoaderError> { let index = self.index_store.get_index_at_op(&operation, &self.store)?; let repo = ReadonlyRepo { - store: self.store.clone(), - op_store: self.op_store.clone(), - op_heads_store: self.op_heads_store.clone(), + loader: self.clone(), operation, - settings: self.repo_settings.clone(), - index_store: self.index_store.clone(), - submodule_store: self.submodule_store.clone(), index, change_id_index: OnceCell::new(), view, diff --git a/lib/src/transaction.rs b/lib/src/transaction.rs index a85d1e7c6..fc2cb4a53 100644 --- a/lib/src/transaction.rs +++ b/lib/src/transaction.rs @@ -138,7 +138,7 @@ impl Transaction { .index_store() .write_index(mut_index, &operation) .unwrap(); - UnpublishedOperation::new(&base_repo.loader(), operation, view, index) + UnpublishedOperation::new(base_repo.loader(), operation, view, index) } } diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index 446b1a0a9..dbcd359c6 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -315,7 +315,7 @@ impl Workspace { working_copy_factory, workspace_id, )?; - let repo_loader = repo.loader(); + let repo_loader = repo.loader().clone(); let workspace = Workspace::new(workspace_root, repo_dir, working_copy, repo_loader)?; Ok((workspace, repo)) })() @@ -374,7 +374,12 @@ impl Workspace { working_copy_factory, workspace_id, )?; - let workspace = Workspace::new(workspace_root, repo_dir, working_copy, repo.loader())?; + let workspace = Workspace::new( + workspace_root, + repo_dir, + working_copy, + repo.loader().clone(), + )?; Ok((workspace, repo)) } diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index 076f847e6..6e23f7000 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -445,7 +445,7 @@ fn test_resolve_op_id() { "###); let repo_loader = repo.loader(); - let resolve = |op_str: &str| op_walk::resolve_op_for_load(&repo_loader, op_str); + let resolve = |op_str: &str| op_walk::resolve_op_for_load(repo_loader, op_str); // Full id assert_eq!(resolve(&operations[0].id().hex()).unwrap(), operations[0]); diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index d2966a541..b6750e9c3 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -287,7 +287,7 @@ pub fn load_repo_at_head(settings: &UserSettings, repo_path: &Path) -> Arc) -> Arc { - let repo_loader = txs[0].base_repo().loader(); + let repo_loader = txs[0].base_repo().loader().clone(); let mut op_ids = vec![]; for tx in txs { op_ids.push(tx.commit("test").op_id().clone());