working copy: return Box<dyn LockedWorkingCopy> from start_mutation()

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};
@@ -1725,7 +1724,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<Option<Operation>, StaleWorkingCopyError> {

cli/src/commands/debug.rs

@@ -21,7 +21,7 @@ use jj_lib::backend::ObjectId;
 use jj_lib::default_index_store::{DefaultIndexStore, ReadonlyIndexWrapper};
 use jj_lib::local_working_copy::{LocalWorkingCopy, LockedLocalWorkingCopy};
 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;

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};

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<LockedLocalWorkingCopy, WorkingCopyStateError> {
+    fn start_mutation(&self) -> Result<Box<dyn LockedWorkingCopy>, 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<Self>,
         operation_id: OperationId,
     ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError> {
         assert!(self.tree_state_dirty || &self.old_tree_id == self.wc.tree_id()?);

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<dyn LockedWorkingCopy>` instead
-    fn start_mutation(&self) -> Result<LockedLocalWorkingCopy, WorkingCopyStateError>;
+    fn start_mutation(&self) -> Result<Box<dyn LockedWorkingCopy>, 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<Self>,
         operation_id: OperationId,
     ) -> Result<Box<dyn WorkingCopy>, WorkingCopyStateError>;
 }

lib/src/workspace.rs

@@ -27,7 +27,7 @@ use crate::file_util::{self, IoResultExt as _, PathError};
 use crate::git_backend::GitBackend;
 use crate::index::IndexStore;
 use crate::local_backend::LocalBackend;
-use crate::local_working_copy::{LocalWorkingCopy, LockedLocalWorkingCopy};
+use crate::local_working_copy::LocalWorkingCopy;
 use crate::merged_tree::MergedTree;
 use crate::op_heads_store::OpHeadsStore;
 use crate::op_store::{OpStore, OperationId, WorkspaceId};
@@ -343,12 +343,12 @@ impl Workspace {
 
 pub struct LockedWorkspace<'a> {
     base: &'a mut Workspace,
-    locked_wc: LockedLocalWorkingCopy,
+    locked_wc: Box<dyn LockedWorkingCopy>,
 }
 
 impl<'a> LockedWorkspace<'a> {
-    pub fn locked_wc(&mut self) -> &mut LockedLocalWorkingCopy {
-        &mut self.locked_wc
+    pub fn locked_wc(&mut self) -> &mut dyn LockedWorkingCopy {
+        self.locked_wc.as_mut()
     }
 
     pub fn finish(self, operation_id: OperationId) -> Result<(), WorkingCopyStateError> {

lib/tests/test_local_working_copy.rs

@@ -34,7 +34,7 @@ use jj_lib::op_store::{OperationId, WorkspaceId};
 use jj_lib::repo::{ReadonlyRepo, Repo};
 use jj_lib::repo_path::{RepoPath, RepoPathComponent, RepoPathJoin};
 use jj_lib::settings::UserSettings;
-use jj_lib::working_copy::{CheckoutStats, LockedWorkingCopy, SnapshotError, SnapshotOptions};
+use jj_lib::working_copy::{CheckoutStats, SnapshotError, SnapshotOptions};
 use jj_lib::workspace::LockedWorkspace;
 use test_case::test_case;
 use testutils::{create_tree, write_random_commit, TestRepoBackend, TestWorkspace};

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};
 

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]

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;