diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 028b4713cc..7c41c523fa 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -41,7 +41,6 @@ use jj_lib::git_backend::GitBackend; use jj_lib::gitignore::GitIgnoreFile; use jj_lib::hex_util::to_reverse_hex; use jj_lib::id_prefix::IdPrefixContext; -use jj_lib::local_working_copy::LockedLocalWorkingCopy; use jj_lib::matchers::{EverythingMatcher, Matcher, PrefixMatcher, Visit}; use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder}; use jj_lib::op_heads_store::{self, OpHeadResolutionError, OpHeadsStore}; @@ -859,7 +858,7 @@ impl WorkspaceCommandHelper { pub fn unchecked_start_working_copy_mutation( &self, - ) -> Result<(LockedLocalWorkingCopy, Commit), CommandError> { + ) -> Result<(Box, Commit), CommandError> { self.check_working_copy_writable()?; let wc_commit = if let Some(wc_commit_id) = self.get_wc_commit_id() { self.repo().store().get_commit(wc_commit_id)? @@ -874,7 +873,7 @@ impl WorkspaceCommandHelper { pub fn start_working_copy_mutation( &self, - ) -> Result<(LockedLocalWorkingCopy, Commit), CommandError> { + ) -> Result<(Box, Commit), CommandError> { let (locked_working_copy, wc_commit) = self.unchecked_start_working_copy_mutation()?; if wc_commit.tree_id() != locked_working_copy.old_tree_id() { return Err(user_error("Concurrent working copy operation. Try again.")); @@ -1302,45 +1301,47 @@ Set which revision the branch points to with `jj branch set {branch_name} -r (repo, wc_commit), - Ok(Some(wc_operation)) => { - let repo = repo.reload_at(&wc_operation)?; - let wc_commit = if let Some(wc_commit) = get_wc_commit(&repo)? { - wc_commit - } else { - return Ok(()); // The workspace has been deleted (see above) - }; - (repo, wc_commit) - } - Err(StaleWorkingCopyError::WorkingCopyStale) => { - return Err(user_error_with_hint( - format!( - "The working copy is stale (not updated since operation {}).", - short_operation_hash(&old_op_id) - ), - "Run `jj workspace update-stale` to update it. + let (repo, wc_commit) = + match check_stale_working_copy(locked_wc.as_ref(), &wc_commit, &repo) { + Ok(None) => (repo, wc_commit), + Ok(Some(wc_operation)) => { + let repo = repo.reload_at(&wc_operation)?; + let wc_commit = if let Some(wc_commit) = get_wc_commit(&repo)? { + wc_commit + } else { + return Ok(()); // The workspace has been deleted (see + // above) + }; + (repo, wc_commit) + } + Err(StaleWorkingCopyError::WorkingCopyStale) => { + return Err(user_error_with_hint( + format!( + "The working copy is stale (not updated since operation {}).", + short_operation_hash(&old_op_id) + ), + "Run `jj workspace update-stale` to update it. See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-working-copy \ - for more information.", - )); - } - Err(StaleWorkingCopyError::SiblingOperation) => { - return Err(CommandError::InternalError(format!( - "The repo was loaded at operation {}, which seems to be a sibling of the \ - working copy's operation {}", - short_operation_hash(repo.op_id()), - short_operation_hash(&old_op_id) - ))); - } - Err(StaleWorkingCopyError::UnrelatedOperation) => { - return Err(CommandError::InternalError(format!( - "The repo was loaded at operation {}, which seems unrelated to the working \ - copy's operation {}", - short_operation_hash(repo.op_id()), - short_operation_hash(&old_op_id) - ))); - } - }; + for more information.", + )); + } + Err(StaleWorkingCopyError::SiblingOperation) => { + return Err(CommandError::InternalError(format!( + "The repo was loaded at operation {}, which seems to be a sibling of the \ + working copy's operation {}", + short_operation_hash(repo.op_id()), + short_operation_hash(&old_op_id) + ))); + } + Err(StaleWorkingCopyError::UnrelatedOperation) => { + return Err(CommandError::InternalError(format!( + "The repo was loaded at operation {}, which seems unrelated to the \ + working copy's operation {}", + short_operation_hash(repo.op_id()), + short_operation_hash(&old_op_id) + ))); + } + }; self.user_repo = ReadonlyUserRepo::new(repo); let progress = crate::progress::snapshot_progress(ui); let new_tree_id = locked_wc.snapshot(SnapshotOptions { @@ -1724,7 +1725,7 @@ pub enum StaleWorkingCopyError { #[instrument(skip_all)] pub fn check_stale_working_copy( - locked_wc: &LockedLocalWorkingCopy, + locked_wc: &dyn LockedWorkingCopy, wc_commit: &Commit, repo: &ReadonlyRepo, ) -> Result, StaleWorkingCopyError> { diff --git a/cli/src/commands/debug.rs b/cli/src/commands/debug.rs index 4f61b6cde1..89d5504c47 100644 --- a/cli/src/commands/debug.rs +++ b/cli/src/commands/debug.rs @@ -20,7 +20,7 @@ use jj_lib::backend::ObjectId; use jj_lib::default_index_store::{DefaultIndexStore, ReadonlyIndexWrapper}; use jj_lib::local_working_copy::LocalWorkingCopy; use jj_lib::revset; -use jj_lib::working_copy::{LockedWorkingCopy, WorkingCopy}; +use jj_lib::working_copy::WorkingCopy; use crate::cli_util::{resolve_op_for_load, user_error, CommandError, CommandHelper}; use crate::template_parser; diff --git a/cli/src/commands/mod.rs b/cli/src/commands/mod.rs index e38b240903..62e2f2b040 100644 --- a/cli/src/commands/mod.rs +++ b/cli/src/commands/mod.rs @@ -47,7 +47,7 @@ use jj_lib::revset_graph::{ }; use jj_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser}; use jj_lib::settings::UserSettings; -use jj_lib::working_copy::{LockedWorkingCopy, SnapshotOptions}; +use jj_lib::working_copy::SnapshotOptions; use jj_lib::workspace::Workspace; use jj_lib::{conflicts, file_util, revset}; use maplit::{hashmap, hashset}; @@ -3903,7 +3903,7 @@ fn cmd_workspace_update_stale( let repo = workspace_command.repo().clone(); let (mut locked_wc, desired_wc_commit) = workspace_command.unchecked_start_working_copy_mutation()?; - match check_stale_working_copy(&locked_wc, &desired_wc_commit, &repo) { + match check_stale_working_copy(locked_wc.as_ref(), &desired_wc_commit, &repo) { Ok(_) => { writeln!( ui.stderr(), diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs index 291d153c8e..a208974c1f 100644 --- a/lib/src/local_working_copy.rs +++ b/lib/src/local_working_copy.rs @@ -1312,7 +1312,7 @@ impl WorkingCopy for LocalWorkingCopy { Ok(self.tree_state()?.sparse_patterns()) } - fn start_mutation(&self) -> Result { + fn start_mutation(&self) -> Result, WorkingCopyStateError> { let lock_path = self.state_path.join("working_copy.lock"); let lock = FileLock::lock(lock_path); @@ -1328,13 +1328,13 @@ impl WorkingCopy for LocalWorkingCopy { }; let old_operation_id = wc.operation_id().clone(); let old_tree_id = wc.tree_id()?.clone(); - Ok(LockedLocalWorkingCopy { + Ok(Box::new(LockedLocalWorkingCopy { wc, lock, old_operation_id, old_tree_id, tree_state_dirty: false, - }) + })) } } @@ -1565,7 +1565,7 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy { #[instrument(skip_all)] fn finish( - mut self, + mut self: Box, operation_id: OperationId, ) -> Result, WorkingCopyStateError> { assert!(self.tree_state_dirty || &self.old_tree_id == self.wc.tree_id()?); diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 8407ba0d82..95111b5f2a 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -25,7 +25,6 @@ use thiserror::Error; use crate::backend::{BackendError, MergedTreeId}; use crate::fsmonitor::FsmonitorKind; use crate::gitignore::GitIgnoreFile; -use crate::local_working_copy::LockedLocalWorkingCopy; use crate::merged_tree::MergedTree; use crate::op_store::{OperationId, WorkspaceId}; use crate::repo_path::RepoPath; @@ -60,8 +59,7 @@ pub trait WorkingCopy { /// Locks the working copy and returns an instance with methods for updating /// the working copy files and state. - // TODO: return a `Box` instead - fn start_mutation(&self) -> Result; + fn start_mutation(&self) -> Result, WorkingCopyStateError>; } /// A working copy that's being modified. @@ -105,7 +103,7 @@ pub trait LockedWorkingCopy { /// Finish the modifications to the working copy by writing the updated /// states to disk. Returns the new (unlocked) working copy. fn finish( - self, + self: Box, operation_id: OperationId, ) -> Result, WorkingCopyStateError>; } diff --git a/lib/src/workspace.rs b/lib/src/workspace.rs index cc44a5f10b..e21db65f45 100644 --- a/lib/src/workspace.rs +++ b/lib/src/workspace.rs @@ -37,9 +37,7 @@ use crate::repo::{ }; use crate::settings::UserSettings; use crate::submodule_store::SubmoduleStore; -use crate::working_copy::{ - CheckoutError, CheckoutStats, LockedWorkingCopy, WorkingCopy, WorkingCopyStateError, -}; +use crate::working_copy::{CheckoutError, CheckoutStats, WorkingCopy, WorkingCopyStateError}; #[derive(Error, Debug)] pub enum WorkspaceInitError { diff --git a/lib/tests/test_local_working_copy.rs b/lib/tests/test_local_working_copy.rs index bf5703994c..dd2e0e5c0a 100644 --- a/lib/tests/test_local_working_copy.rs +++ b/lib/tests/test_local_working_copy.rs @@ -27,7 +27,7 @@ use std::sync::Arc; use itertools::Itertools; use jj_lib::backend::{MergedTreeId, ObjectId, TreeId, TreeValue}; use jj_lib::fsmonitor::FsmonitorKind; -use jj_lib::local_working_copy::{LocalWorkingCopy, LockedLocalWorkingCopy}; +use jj_lib::local_working_copy::LocalWorkingCopy; use jj_lib::merge::Merge; use jj_lib::merged_tree::{MergedTree, MergedTreeBuilder}; use jj_lib::op_store::{OperationId, WorkspaceId}; @@ -925,7 +925,7 @@ fn test_fsmonitor() { testutils::write_working_copy_file(&workspace_root, &ignored_path, "ignored\n"); testutils::write_working_copy_file(&workspace_root, &gitignore_path, "to/ignored\n"); - let snapshot = |locked_wc: &mut LockedLocalWorkingCopy, paths: &[&RepoPath]| { + let snapshot = |locked_wc: &mut dyn LockedWorkingCopy, paths: &[&RepoPath]| { let fs_paths = paths .iter() .map(|p| p.to_fs_path(&workspace_root)) @@ -942,13 +942,13 @@ fn test_fsmonitor() { { let mut locked_wc = ws.working_copy().start_mutation().unwrap(); - let tree_id = snapshot(&mut locked_wc, &[]); + let tree_id = snapshot(locked_wc.as_mut(), &[]); assert_eq!(tree_id, repo.store().empty_merged_tree_id()); } { let mut locked_wc = ws.working_copy().start_mutation().unwrap(); - let tree_id = snapshot(&mut locked_wc, &[&foo_path]); + let tree_id = snapshot(locked_wc.as_mut(), &[&foo_path]); insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###" tree d5e38c0a1b0ee5de47c5 file "foo" (e99c2057c15160add351): "foo\n" @@ -958,7 +958,7 @@ fn test_fsmonitor() { { let mut locked_wc = ws.working_copy().start_mutation().unwrap(); let tree_id = snapshot( - &mut locked_wc, + locked_wc.as_mut(), &[&foo_path, &bar_path, &nested_path, &ignored_path], ); insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###" @@ -974,7 +974,7 @@ fn test_fsmonitor() { testutils::write_working_copy_file(&workspace_root, &foo_path, "updated foo\n"); testutils::write_working_copy_file(&workspace_root, &bar_path, "updated bar\n"); let mut locked_wc = ws.working_copy().start_mutation().unwrap(); - let tree_id = snapshot(&mut locked_wc, &[&foo_path]); + let tree_id = snapshot(locked_wc.as_mut(), &[&foo_path]); insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###" tree e994a93c46f41dc91704 file "bar" (94cc973e7e1aefb7eff6): "bar\n" @@ -986,7 +986,7 @@ fn test_fsmonitor() { { std::fs::remove_file(foo_path.to_fs_path(&workspace_root)).unwrap(); let mut locked_wc = ws.working_copy().start_mutation().unwrap(); - let tree_id = snapshot(&mut locked_wc, &[&foo_path]); + let tree_id = snapshot(locked_wc.as_mut(), &[&foo_path]); insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###" tree 1df764981d4d74a4ecfa file "bar" (94cc973e7e1aefb7eff6): "bar\n" diff --git a/lib/tests/test_local_working_copy_concurrent.rs b/lib/tests/test_local_working_copy_concurrent.rs index c8b684cec3..10bac274ba 100644 --- a/lib/tests/test_local_working_copy_concurrent.rs +++ b/lib/tests/test_local_working_copy_concurrent.rs @@ -18,7 +18,7 @@ use std::thread; use assert_matches::assert_matches; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; -use jj_lib::working_copy::{CheckoutError, LockedWorkingCopy, SnapshotOptions}; +use jj_lib::working_copy::{CheckoutError, SnapshotOptions}; use jj_lib::workspace::Workspace; use testutils::{create_tree, write_working_copy_file, TestRepo, TestWorkspace}; diff --git a/lib/tests/test_local_working_copy_sparse.rs b/lib/tests/test_local_working_copy_sparse.rs index 7b83910553..4413a8f2cb 100644 --- a/lib/tests/test_local_working_copy_sparse.rs +++ b/lib/tests/test_local_working_copy_sparse.rs @@ -17,7 +17,7 @@ use jj_lib::local_working_copy::LocalWorkingCopy; use jj_lib::matchers::EverythingMatcher; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; -use jj_lib::working_copy::{CheckoutStats, LockedWorkingCopy, WorkingCopy}; +use jj_lib::working_copy::{CheckoutStats, WorkingCopy}; use testutils::{create_tree, TestWorkspace}; #[test] diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index af5543fe32..bce37b191a 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -32,7 +32,7 @@ use jj_lib::store::Store; use jj_lib::transaction::Transaction; use jj_lib::tree::Tree; use jj_lib::tree_builder::TreeBuilder; -use jj_lib::working_copy::{LockedWorkingCopy, SnapshotError, SnapshotOptions}; +use jj_lib::working_copy::{SnapshotError, SnapshotOptions}; use jj_lib::workspace::Workspace; use tempfile::TempDir;