cli: snapshot stale working copy before updating

Since there's no easy API to snapshot the stale working copy without releasing
the lock, we have to compare the tree ids after reacquiring the lock. We could
instead manually snapshot and rebase the working-copy commit, but that would
require more copy-paste codes.

Closes #1310

CHANGELOG.md

@@ -41,6 +41,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   the new name. If a new remote with the old name and containing the same
   branches was added, the remote branches may not be recreated in some cases.
 
+* `jj workspace update-stale` now snapshots the working-copy changes before
+  updating to the new working-copy commit.
+
 ## [0.7.0] - 2023-02-16
 
 ### Breaking changes

src/cli_util.rs

@@ -459,6 +459,22 @@ impl CommandHelper {
             repo,
         )
     }
+
+    /// Loads workspace that will diverge from the last working-copy operation.
+    pub fn for_stale_working_copy(
+        &self,
+        ui: &mut Ui,
+    ) -> Result<WorkspaceCommandHelper, CommandError> {
+        let workspace = self.load_workspace()?;
+        let op_store = workspace.repo_loader().op_store();
+        let op_id = workspace.working_copy().operation_id();
+        let op_data = op_store
+            .read_operation(op_id)
+            .map_err(|e| CommandError::InternalError(format!("Failed to read operation: {e}")))?;
+        let operation = Operation::new(op_store.clone(), op_id.clone(), op_data);
+        let repo = workspace.repo_loader().load_at(&operation);
+        self.for_loaded_repo(ui, workspace, repo)
+    }
 }
 
 // Provides utilities for writing a command that works on a workspace (like most

src/commands/mod.rs

@@ -3227,7 +3227,18 @@ fn cmd_workspace_update_stale(
     command: &CommandHelper,
     _args: &WorkspaceUpdateStaleArgs,
 ) -> Result<(), CommandError> {
+    // Snapshot the current working copy on top of the last known working-copy
+    // operation, then merge the concurrent operations. The wc_commit_id of the
+    // merged repo wouldn't change because the old one wins, but it's probably
+    // fine if we picked the new wc_commit_id.
+    let known_wc_commit = {
+        let mut workspace_command = command.for_stale_working_copy(ui)?;
+        workspace_command.snapshot(ui)?;
+        let wc_commit_id = workspace_command.get_wc_commit_id().unwrap();
+        workspace_command.repo().store().get_commit(wc_commit_id)?
+    };
     let mut workspace_command = command.workspace_helper_no_snapshot(ui)?;
+
     let repo = workspace_command.repo().clone();
     let (mut locked_wc, desired_wc_commit) =
         workspace_command.unsafe_start_working_copy_mutation()?;
@@ -3237,7 +3248,11 @@ fn cmd_workspace_update_stale(
             ui.write("Nothing to do (the working copy is not stale).\n")?;
         }
         Err(_) => {
-            // TODO: First commit the working copy
+            // The same check as start_working_copy_mutation(), but with the stale
+            // working-copy commit.
+            if known_wc_commit.tree_id() != locked_wc.old_tree_id() {
+                return Err(user_error("Concurrent working copy operation. Try again."));
+            }
             let stats = locked_wc
                 .check_out(&desired_wc_commit.tree())
                 .map_err(|err| {

tests/test_workspaces.rs

@@ -122,17 +122,21 @@ fn test_workspaces_conflicting_edits() {
     Hint: Run `jj workspace update-stale` to update it.
     "###);
     let stdout = test_env.jj_cmd_success(&secondary_path, &["workspace", "update-stale"]);
-    // It was detected that the working copy is now stale
-    // TODO: Since there was an uncommitted change in the working copy, it should
+    // It was detected that the working copy is now stale.
+    // Since there was an uncommitted change in the working copy, it should
     // have been committed first (causing divergence)
     insta::assert_snapshot!(stdout, @r###"
+    Concurrent modification detected, resolving automatically.
+    Rebased 1 descendant commits onto commits rewritten by other operation
     Working copy now at: a1896a17282f (no description set)
     Added 0 files, modified 1 files, removed 0 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &secondary_path),
     @r###"
-    o  fe8f41ed01d693b2d4365cd89e42ad9c531a939b default@
-    │ @  a1896a17282f19089a5cec44358d6609910e0513 secondary@
+    o  8d90dc175c874af0dff032d611029dc722d4e108 (divergent)
+    │ o  fe8f41ed01d693b2d4365cd89e42ad9c531a939b default@
+    ├─╯
+    │ @  a1896a17282f19089a5cec44358d6609910e0513 secondary@ (divergent)
     ├─╯
     o  c0d4a99ef98ada7da8dc73a778bbb747c4178385
     o  0000000000000000000000000000000000000000
@@ -141,8 +145,10 @@ fn test_workspaces_conflicting_edits() {
     let stdout = get_log_output(&test_env, &secondary_path);
     assert!(!stdout.starts_with("The working copy is stale"));
     insta::assert_snapshot!(stdout, @r###"
-    o  fe8f41ed01d693b2d4365cd89e42ad9c531a939b default@
-    │ @  a1896a17282f19089a5cec44358d6609910e0513 secondary@
+    o  8d90dc175c874af0dff032d611029dc722d4e108 (divergent)
+    │ o  fe8f41ed01d693b2d4365cd89e42ad9c531a939b default@
+    ├─╯
+    │ @  a1896a17282f19089a5cec44358d6609910e0513 secondary@ (divergent)
     ├─╯
     o  c0d4a99ef98ada7da8dc73a778bbb747c4178385
     o  0000000000000000000000000000000000000000
@@ -235,6 +241,40 @@ fn test_workspaces_update_stale_noop() {
     "###);
 }
 
+/// Test "update-stale" in a dirty, but not stale working copy.
+#[test]
+fn test_workspaces_update_stale_snapshot() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_success(test_env.env_root(), &["init", "--git", "main"]);
+    let main_path = test_env.env_root().join("main");
+    let secondary_path = test_env.env_root().join("secondary");
+
+    std::fs::write(main_path.join("file"), "changed in main\n").unwrap();
+    test_env.jj_cmd_success(&main_path, &["new"]);
+    test_env.jj_cmd_success(&main_path, &["workspace", "add", "../secondary"]);
+
+    // Record new operation in one workspace.
+    test_env.jj_cmd_success(&main_path, &["new"]);
+
+    // Snapshot the other working copy, which unfortunately results in concurrent
+    // operations, but should be resolved cleanly.
+    std::fs::write(secondary_path.join("file"), "changed in second\n").unwrap();
+    let stdout = test_env.jj_cmd_success(&secondary_path, &["workspace", "update-stale"]);
+    insta::assert_snapshot!(stdout, @r###"
+    Concurrent modification detected, resolving automatically.
+    Nothing to do (the working copy is not stale).
+    "###);
+
+    insta::assert_snapshot!(get_log_output(&test_env, &secondary_path), @r###"
+    @  4976dfa88529814c4dd8c06253fbd82d076b79f8 secondary@
+    │ o  8357b22214ba8adb6d2d378fa5b85274f1c7967c default@
+    │ o  1a769966ed69fa7abadbd2d899e2be1025cb04fb
+    ├─╯
+    o  b4a6c25e777817db67fdcbd50f1dd3b74b46b5f1
+    o  0000000000000000000000000000000000000000
+    "###);
+}
+
 /// Test forgetting workspaces
 #[test]
 fn test_workspaces_forget() {