diff --git a/CHANGELOG.md b/CHANGELOG.md index 03aa854f4..bf80b4f64 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/cli_util.rs b/src/cli_util.rs index 5ab59faf9..3ee6c50b1 100644 --- a/src/cli_util.rs +++ b/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 { + 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 diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 8bfedae19..7c23af544 100644 --- a/src/commands/mod.rs +++ b/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| { diff --git a/tests/test_workspaces.rs b/tests/test_workspaces.rs index 795ffcce7..6446e8dc5 100644 --- a/tests/test_workspaces.rs +++ b/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() {