working_copy: move check of old commit ID on checkout to higher level

There are only two callers of `LockedWorkingCopy::check_out()`. One is
in `commands.rs`. That caller already checks after taking the lock
that the old commit ID is as expected. The other caller is
`WorkingCopy::check_out()`. We can simply move the check to that level
since it's the only caller that cares now.

lib/src/working_copy.rs

@@ -883,14 +883,17 @@ impl WorkingCopy {
         new_commit: Commit,
     ) -> Result<CheckoutStats, CheckoutError> {
         let mut locked_wc = self.start_mutation();
-        let new_commit_id = new_commit.id().clone();
-        let stats = match locked_wc.check_out(old_commit_id, new_commit.tree_id().clone()) {
-            Err(CheckoutError::ConcurrentCheckout) => {
+        // Check if the current checkout has changed on disk compared to what the caller
+        // expected. It's safe to check out another commit regardless, but it's
+        // probably not what  the caller wanted, so we let them know.
+        if let Some(old_commit_id) = old_commit_id {
+            if *old_commit_id != locked_wc.old_commit_id {
                 locked_wc.discard();
                 return Err(CheckoutError::ConcurrentCheckout);
             }
-            other => other,
-        }?;
+        }
+        let new_commit_id = new_commit.id().clone();
+        let stats = locked_wc.check_out(new_commit.tree_id().clone())?;
         locked_wc.finish(operation_id, new_commit_id);
         Ok(stats)
     }
@@ -922,23 +925,9 @@ impl LockedWorkingCopy<'_> {
         self.wc.tree_state().as_mut().unwrap().write_tree()
     }
 
-    pub fn check_out(
-        &mut self,
-        old_commit_id: Option<&CommitId>,
-        new_tree_id: TreeId,
-    ) -> Result<CheckoutStats, CheckoutError> {
+    pub fn check_out(&mut self, new_tree_id: TreeId) -> Result<CheckoutStats, CheckoutError> {
         // TODO: Write a "pending_checkout" file with the old and new TreeIds so we can
         // continue an interrupted checkout if we find such a file.
-
-        // Check if the current checkout has changed on disk compared to what the caller
-        // expected. It's safe to check out another commit regardless, but it's
-        // probably not what  the caller wanted, so we let them know.
-        if let Some(old_commit_id) = old_commit_id {
-            if *old_commit_id != self.old_commit_id {
-                return Err(CheckoutError::ConcurrentCheckout);
-            }
-        }
-
         let stats = self
             .wc
             .tree_state()

lib/tests/test_working_copy.rs

@@ -400,8 +400,7 @@ fn test_checkout_discard() {
 
     let wc = test_workspace.workspace.working_copy_mut();
     let state_path = wc.state_path().to_path_buf();
-    wc.check_out(repo.op_id().clone(), None, commit1.clone())
-        .unwrap();
+    wc.check_out(repo.op_id().clone(), None, commit1).unwrap();
 
     // Test the setup: the file should exist on disk and in the tree state.
     assert!(file1_path.to_fs_path(&workspace_root).is_file());
@@ -409,9 +408,7 @@ fn test_checkout_discard() {
 
     // Start a checkout
     let mut locked_wc = wc.start_mutation();
-    locked_wc
-        .check_out(Some(commit1.id()), tree2.id().clone())
-        .unwrap();
+    locked_wc.check_out(tree2.id().clone()).unwrap();
     // The change should be reflected in the working copy but not saved
     assert!(!file1_path.to_fs_path(&workspace_root).is_file());
     assert!(file2_path.to_fs_path(&workspace_root).is_file());

src/commands.rs

@@ -537,7 +537,7 @@ impl WorkspaceCommandHelper {
                         repo_operation.id().hex()
                     )?;
                     locked_wc
-                        .check_out(Some(&wc_commit_id), checkout_commit.tree_id().clone())
+                        .check_out(checkout_commit.tree_id().clone())
                         .unwrap();
                 } else {
                     return Err(CommandError::InternalError(format!(