From a733fceba909e77a4e11bad304f672ff30378082 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Fri, 13 Oct 2023 22:18:16 -0700
Subject: [PATCH] working copy: add functions to start/finish modification to
 backend trait

To keep this patch small, the functions still return the concrete
local-disk implementations. I'll fix that soon.
---
 lib/src/local_working_copy.rs | 80 +++++++++++++++++------------------
 lib/src/working_copy.rs       | 11 +++++
 2 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/lib/src/local_working_copy.rs b/lib/src/local_working_copy.rs
index 8c2302e13..d29f7b632 100644
--- a/lib/src/local_working_copy.rs
+++ b/lib/src/local_working_copy.rs
@@ -1307,6 +1307,31 @@ impl WorkingCopy for LocalWorkingCopy {
     fn sparse_patterns(&self) -> Result<&[RepoPath], WorkingCopyStateError> {
         Ok(self.tree_state()?.sparse_patterns())
     }
+
+    fn start_mutation(&self) -> Result<LockedLocalWorkingCopy, WorkingCopyStateError> {
+        let lock_path = self.state_path.join("working_copy.lock");
+        let lock = FileLock::lock(lock_path);
+
+        let wc = LocalWorkingCopy {
+            store: self.store.clone(),
+            working_copy_path: self.working_copy_path.clone(),
+            state_path: self.state_path.clone(),
+            // Empty so we re-read the state after taking the lock
+            checkout_state: OnceCell::new(),
+            // TODO: It's expensive to reload the whole tree. We should copy it from `self` if it
+            // hasn't changed.
+            tree_state: OnceCell::new(),
+        };
+        let old_operation_id = wc.operation_id().clone();
+        let old_tree_id = wc.current_tree_id()?.clone();
+        Ok(LockedLocalWorkingCopy {
+            wc,
+            lock,
+            old_operation_id,
+            old_tree_id,
+            tree_state_dirty: false,
+        })
+    }
 }
 
 impl LocalWorkingCopy {
@@ -1436,31 +1461,6 @@ impl LocalWorkingCopy {
         });
     }
 
-    pub fn start_mutation(&self) -> Result<LockedLocalWorkingCopy, WorkingCopyStateError> {
-        let lock_path = self.state_path.join("working_copy.lock");
-        let lock = FileLock::lock(lock_path);
-
-        let wc = LocalWorkingCopy {
-            store: self.store.clone(),
-            working_copy_path: self.working_copy_path.clone(),
-            state_path: self.state_path.clone(),
-            // Empty so we re-read the state after taking the lock
-            checkout_state: OnceCell::new(),
-            // TODO: It's expensive to reload the whole tree. We should copy it from `self` if it
-            // hasn't changed.
-            tree_state: OnceCell::new(),
-        };
-        let old_operation_id = wc.operation_id().clone();
-        let old_tree_id = wc.current_tree_id()?.clone();
-        Ok(LockedLocalWorkingCopy {
-            wc,
-            lock,
-            old_operation_id,
-            old_tree_id,
-            tree_state_dirty: false,
-        })
-    }
-
     #[cfg(feature = "watchman")]
     pub fn query_watchman(
         &self,
@@ -1558,23 +1558,9 @@ impl LockedWorkingCopy for LockedLocalWorkingCopy {
         self.tree_state_dirty = true;
         Ok(stats)
     }
-}
-
-impl LockedLocalWorkingCopy {
-    pub fn reset_watchman(&mut self) -> Result<(), SnapshotError> {
-        self.wc
-            .tree_state_mut()
-            .map_err(|err| SnapshotError::Other {
-                message: "Failed to read the working copy state".to_string(),
-                err: err.into(),
-            })?
-            .reset_watchman();
-        self.tree_state_dirty = true;
-        Ok(())
-    }
 
     #[instrument(skip_all)]
-    pub fn finish(
+    fn finish(
         mut self,
         operation_id: OperationId,
     ) -> Result<LocalWorkingCopy, WorkingCopyStateError> {
@@ -1596,3 +1582,17 @@ impl LockedLocalWorkingCopy {
         Ok(self.wc)
     }
 }
+
+impl LockedLocalWorkingCopy {
+    pub fn reset_watchman(&mut self) -> Result<(), SnapshotError> {
+        self.wc
+            .tree_state_mut()
+            .map_err(|err| SnapshotError::Other {
+                message: "Failed to read the working copy state".to_string(),
+                err: err.into(),
+            })?
+            .reset_watchman();
+        self.tree_state_dirty = true;
+        Ok(())
+    }
+}
diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs
index 191abeef3..dbfced477 100644
--- a/lib/src/working_copy.rs
+++ b/lib/src/working_copy.rs
@@ -25,6 +25,7 @@ use thiserror::Error;
 use crate::backend::{BackendError, MergedTreeId};
 use crate::fsmonitor::FsmonitorKind;
 use crate::gitignore::GitIgnoreFile;
+use crate::local_working_copy::{LocalWorkingCopy, LockedLocalWorkingCopy};
 use crate::merged_tree::MergedTree;
 use crate::op_store::{OperationId, WorkspaceId};
 use crate::repo_path::RepoPath;
@@ -53,6 +54,11 @@ pub trait WorkingCopy {
     /// checked out in the working copy. A single `RepoPath::root()` entry means
     /// that all files should be checked out.
     fn sparse_patterns(&self) -> Result<&[RepoPath], WorkingCopyStateError>;
+
+    /// 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>;
 }
 
 /// A working copy that's being modified.
@@ -89,6 +95,11 @@ pub trait LockedWorkingCopy {
         &mut self,
         new_sparse_patterns: Vec<RepoPath>,
     ) -> Result<CheckoutStats, CheckoutError>;
+
+    /// Finish the modifications to the working copy by writing the updated
+    /// states to disk. Returns the new (unlocked) working copy.
+    // TODO: return a `Box<dyn WorkingCopy>` instead
+    fn finish(self, operation_id: OperationId) -> Result<LocalWorkingCopy, WorkingCopyStateError>;
 }
 
 /// An error while snapshotting the working copy.