diff --git a/src/cli_util.rs b/src/cli_util.rs index d64a293b5..7a62f937c 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -1023,23 +1023,37 @@ impl WorkspaceCommandHelper { } pub fn snapshot_working_copy(&mut self, ui: &mut Ui) -> Result<(), CommandError> { - let repo = self.repo().clone(); let workspace_id = self.workspace_id().to_owned(); - let wc_commit_id = match repo.view().get_wc_commit_id(&workspace_id) { - Some(wc_commit_id) => wc_commit_id.clone(), - None => { - // If the workspace has been deleted, it's unclear what to do, so we just skip - // committing the working copy. - return Ok(()); - } + let get_wc_commit = |repo: &ReadonlyRepo| -> Result, _> { + repo.view() + .get_wc_commit_id(&workspace_id) + .map(|id| repo.store().get_commit(id)) + .transpose() + }; + let repo = self.repo().clone(); + let wc_commit = if let Some(wc_commit) = get_wc_commit(&repo)? { + wc_commit + } else { + // If the workspace has been deleted, it's unclear what to do, so we just skip + // committing the working copy. + return Ok(()); }; let base_ignores = self.base_ignores(); + + // Compare working-copy tree and operation with repo's, and reload as needed. let mut locked_wc = self.workspace.working_copy_mut().start_mutation(); let old_op_id = locked_wc.old_operation_id().clone(); - let wc_commit = repo.store().get_commit(&wc_commit_id)?; - let repo = match check_stale_working_copy(&locked_wc, &wc_commit, &repo) { - Ok(None) => repo, - Ok(Some(wc_operation)) => repo.reload_at(&wc_operation), + let (repo, wc_commit) = match check_stale_working_copy(&locked_wc, &wc_commit, &repo) { + Ok(None) => (repo, wc_commit), + Ok(Some(wc_operation)) => { + let repo = repo.reload_at(&wc_operation); + let wc_commit = if let Some(wc_commit) = get_wc_commit(&repo)? { + wc_commit + } else { + return Ok(()); // The workspace has been deleted (see above) + }; + (repo, wc_commit) + } Err(StaleWorkingCopyError::WorkingCopyStale) => { locked_wc.discard(); return Err(user_error_with_hint( diff --git a/tests/test_concurrent_operations.rs b/tests/test_concurrent_operations.rs index e0f08b066..2a247f8b0 100644 --- a/tests/test_concurrent_operations.rs +++ b/tests/test_concurrent_operations.rs @@ -14,6 +14,8 @@ use std::path::Path; +use itertools::Itertools as _; + use crate::common::TestEnvironment; pub mod common; @@ -137,6 +139,77 @@ fn test_concurrent_operations_wc_modified() { "###); } +#[test] +fn test_concurrent_snapshot_wc_reloadable() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + let op_heads_dir = repo_path + .join(".jj") + .join("repo") + .join("op_heads") + .join("heads"); + + std::fs::write(repo_path.join("base"), "").unwrap(); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "initial"]); + + // Create new commit and checkout it. + std::fs::write(repo_path.join("child1"), "").unwrap(); + test_env.jj_cmd_success(&repo_path, &["commit", "-m", "new child1"]); + + let template = r#"id ++ "\n" ++ description ++ "\n" ++ tags"#; + let op_log_stdout = test_env.jj_cmd_success(&repo_path, &["op", "log", "-T", template]); + insta::assert_snapshot!(op_log_stdout, @r###" + @ bacc8f507ccede29c282bb1459b71ffd233a9e29f4ec11b027422923592c4ef949e4465fd101c99ee6cfd39af26f29cd5910ef3b16985538e50fd21523bcf3e1 + │ commit 323b414dd255b51375d7f4392b7b2641ffe4289f + │ args: jj commit -m 'new child1' + ◉ 6aa8b9099660e021813be72b4230692f782699448947cc76ffe5245d23108184dd60bb795fba350eb39abc2b0643169623cf59bb0c04130102388f8d87791070 + │ snapshot working copy + │ args: jj commit -m 'new child1' + ◉ 7a31786821de03db03d5b9b7ec97454d10b90eee33844a86ebd282124c8ab43babcf79091cd1c47796ed8cc6ff23febabb35a10e0aad1f96428f958eb834b30d + │ commit 3d918700494a9895696e955b85fa05eb0d314cc6 + │ args: jj commit -m initial + ◉ 36440b76f049c4999505fa1dd3b21bfdb1a1f93c64e04f0a3797e704145780df959afcd3babd59c536544c6e7a6b883594abe7b38e8c4be5fe6d66a3cd8f4f9d + │ snapshot working copy + │ args: jj commit -m initial + ◉ a99a3fd5c51e8f7ccb9ae2f9fb749612a23f0a7cf25d8c644f36c35c077449ce3c66f49d098a5a704ca5e47089a7f019563a5b8cbc7d451619e0f90c82241ceb + │ add workspace 'default' + ◉ 56b94dfc38e7d54340377f566e96ab97dc6163ea7841daf49fb2e1d1ceb27e26274db1245835a1a421fb9d06e6e0fe1e4f4aa1b0258c6e86df676ad9111d0dab + initialize repo + "###); + let op_log_lines = op_log_stdout.lines().collect_vec(); + let current_op_id = op_log_lines[0].split_once(" ").unwrap().1; + let previous_op_id = op_log_lines[6].split_once(" ").unwrap().1; + + // Another process started from the "initial" operation, but snapshots after + // the "child1" checkout has been completed. + std::fs::rename( + op_heads_dir.join(current_op_id), + op_heads_dir.join(previous_op_id), + ) + .unwrap(); + std::fs::write(repo_path.join("child2"), "").unwrap(); + let stdout = test_env.jj_cmd_success(&repo_path, &["describe", "-m", "new child2"]); + insta::assert_snapshot!(stdout, @r###" + Working copy now at: 4011424ea0a2 new child2 + Parent commit : e08863ee7a0d new child1 + "###); + + // Since the repo can be reloaded before snapshotting, "child2" should be + // a child of "child1", not of "initial". + let template = r#"commit_id ++ " " ++ description"#; + let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", template, "-s"]); + insta::assert_snapshot!(stdout, @r###" + @ 4011424ea0a210a914f869ea3c47d76931598d1d new child2 + │ A child2 + ◉ e08863ee7a0df688755d3d3126498afdf4f580ad new child1 + │ A child1 + ◉ 79989e62f8331e69a803058b57bacc264405cb65 initial + │ A base + ◉ 0000000000000000000000000000000000000000 + "###); +} + fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"commit_id ++ " " ++ description"#; test_env.jj_cmd_success(cwd, &["log", "-T", template])