diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index c283b2b7b..39698735e 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -2240,14 +2240,7 @@ pub fn check_stale_working_copy( // The working copy isn't stale, and no need to reload the repo. Ok(WorkingCopyFreshness::Fresh) } else { - let wc_operation_data = repo - .op_store() - .read_operation(locked_wc.old_operation_id())?; - let wc_operation = Operation::new( - repo.op_store().clone(), - locked_wc.old_operation_id().clone(), - wc_operation_data, - ); + let wc_operation = repo.loader().load_operation(locked_wc.old_operation_id())?; let repo_operation = repo.operation(); let ancestor_op = dag_walk::closest_common_node_ok( [Ok(wc_operation.clone())], diff --git a/cli/src/commands/operation/abandon.rs b/cli/src/commands/operation/abandon.rs index 4a8e9ddc0..d49ba149c 100644 --- a/cli/src/commands/operation/abandon.rs +++ b/cli/src/commands/operation/abandon.rs @@ -18,7 +18,6 @@ use std::slice; use itertools::Itertools as _; use jj_lib::op_walk; -use jj_lib::operation::Operation; use crate::cli_util::short_operation_hash; use crate::cli_util::CommandHelper; @@ -65,9 +64,7 @@ pub fn cmd_op_abandon( let (abandon_root_op, abandon_head_ops) = if let Some((root_op_str, head_op_str)) = args.operation.split_once("..") { let root_op = if root_op_str.is_empty() { - let id = op_store.root_operation_id(); - let data = op_store.read_operation(id)?; - Operation::new(op_store.clone(), id.clone(), data) + repo_loader.root_operation() } else { resolve_op(root_op_str)? }; diff --git a/cli/src/commands/workspace/update_stale.rs b/cli/src/commands/workspace/update_stale.rs index 59218e61d..58c1f56db 100644 --- a/cli/src/commands/workspace/update_stale.rs +++ b/cli/src/commands/workspace/update_stale.rs @@ -16,7 +16,6 @@ use std::sync::Arc; use jj_lib::object_id::ObjectId; use jj_lib::op_store::OpStoreError; -use jj_lib::operation::Operation; use jj_lib::repo::ReadonlyRepo; use jj_lib::repo::Repo; use tracing::instrument; @@ -148,18 +147,10 @@ fn for_stale_working_copy( command: &CommandHelper, ) -> Result<(WorkspaceCommandHelper, bool), CommandError> { let workspace = command.load_workspace()?; - let op_store = workspace.repo_loader().op_store(); let (repo, recovered) = { let op_id = workspace.working_copy().operation_id(); - match op_store.read_operation(op_id) { - Ok(op_data) => ( - workspace.repo_loader().load_at(&Operation::new( - op_store.clone(), - op_id.clone(), - op_data, - ))?, - false, - ), + match workspace.repo_loader().load_operation(op_id) { + Ok(op) => (workspace.repo_loader().load_at(&op)?, false), Err(e @ OpStoreError::ObjectNotFound { .. }) => { writeln!( ui.status(), diff --git a/lib/src/repo.rs b/lib/src/repo.rs index dc3686be1..65594d28a 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -65,6 +65,7 @@ use crate::op_heads_store::OpHeadsStore; use crate::op_store; use crate::op_store::OpStore; use crate::op_store::OpStoreError; +use crate::op_store::OpStoreResult; use crate::op_store::OperationId; use crate::op_store::RefTarget; use crate::op_store::RemoteRef; @@ -226,15 +227,7 @@ impl ReadonlyRepo { 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( - loader.op_store.clone(), - loader.op_store.root_operation_id().clone(), - root_operation_data, - ); + let root_operation = loader.root_operation(); let root_view = root_operation.view().expect("failed to read root view"); let index = loader .index_store @@ -738,6 +731,21 @@ impl RepoLoader { Arc::new(repo) } + // If we add a higher-level abstraction of OpStore, root_operation() and + // load_operation() will be moved there. + + /// Returns the root operation. + pub fn root_operation(&self) -> Operation { + self.load_operation(self.op_store.root_operation_id()) + .expect("failed to read root operation") + } + + /// Loads the specified operation from the operation store. + pub fn load_operation(&self, id: &OperationId) -> OpStoreResult { + let data = self.op_store.read_operation(id)?; + Ok(Operation::new(self.op_store.clone(), id.clone(), data)) + } + /// Merges the given `operations` into a single operation. Returns the root /// operation if the `operations` is empty. pub fn merge_operations( @@ -749,9 +757,7 @@ impl RepoLoader { let num_operations = operations.len(); let mut operations = operations.into_iter(); let Some(base_op) = operations.next() else { - let id = self.op_store.root_operation_id(); - let data = self.op_store.read_operation(id)?; - return Ok(Operation::new(self.op_store.clone(), id.clone(), data)); + return Ok(self.root_operation()); }; let final_op = if num_operations > 1 { let base_repo = self.load_at(&base_op)?; diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index 6e23f7000..1ba2fb736 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -198,12 +198,10 @@ fn test_reparent_range_linear() { let settings = testutils::user_settings(); let test_repo = TestRepo::init(); let repo_0 = test_repo.repo; + let loader = repo_0.loader(); let op_store = repo_0.op_store(); - let read_op = |id| { - let data = op_store.read_operation(id).unwrap(); - Operation::new(op_store.clone(), id.clone(), data) - }; + let read_op = |id| loader.load_operation(id).unwrap(); fn op_parents(op: &Operation) -> [Operation; N] { let parents: Vec<_> = op.parents().try_collect().unwrap(); @@ -267,12 +265,10 @@ fn test_reparent_range_bookmarky() { let settings = testutils::user_settings(); let test_repo = TestRepo::init(); let repo_0 = test_repo.repo; + let loader = repo_0.loader(); let op_store = repo_0.op_store(); - let read_op = |id| { - let data = op_store.read_operation(id).unwrap(); - Operation::new(op_store.clone(), id.clone(), data) - }; + let read_op = |id| loader.load_operation(id).unwrap(); fn op_parents(op: &Operation) -> [Operation; N] { let parents: Vec<_> = op.parents().try_collect().unwrap(); @@ -421,7 +417,7 @@ fn test_resolve_op_id() { let settings = stable_op_id_settings(); let test_repo = TestRepo::init_with_settings(&settings); let repo = test_repo.repo; - let op_store = repo.op_store(); + let loader = repo.loader(); let mut operations = Vec::new(); // The actual value of `i` doesn't matter, we just need to make sure we end @@ -481,11 +477,7 @@ fn test_resolve_op_id() { )) ); // Virtual root id - let root_operation = { - let id = op_store.root_operation_id(); - let data = op_store.read_operation(id).unwrap(); - Operation::new(op_store.clone(), id.clone(), data) - }; + let root_operation = loader.root_operation(); assert_eq!(resolve(&root_operation.id().hex()).unwrap(), root_operation); assert_eq!(resolve("000").unwrap(), root_operation); assert_eq!(resolve("002").unwrap(), operations[6]); diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 3034c1293..54f6ea768 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -34,7 +34,6 @@ use jj_lib::op_store::RefTarget; use jj_lib::op_store::RemoteRef; use jj_lib::op_store::RemoteRefState; use jj_lib::op_store::WorkspaceId; -use jj_lib::operation::Operation; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; use jj_lib::repo_path::RepoPathUiConverter; @@ -943,12 +942,7 @@ fn test_evaluate_expression_root_and_checkout() { let test_workspace = TestWorkspace::init(&settings); let repo = &test_workspace.repo; - let root_operation = { - let op_store = repo.op_store(); - let id = op_store.root_operation_id(); - let data = op_store.read_operation(id).unwrap(); - Operation::new(op_store.clone(), id.clone(), data) - }; + let root_operation = repo.loader().root_operation(); let root_repo = repo.reload_at(&root_operation).unwrap(); let mut tx = repo.start_transaction(&settings);