cli: when merging concurrent operations, snapshot afterwards

It seems simpler to do the snapshotting after merging any concurrent
operations instead of snapshotting on top of one of the operations,
especially since the attempt to snapshot may end up noticing that the
working copy is stale.

More importantly, snapshotting before resolving operations resulted in
a crash if the working copy was modified. That happened because we
held a lock on the operation heads (`locked_op_heads`) while we tried
to record the operation committing the working copy. I noticed this
only after adding the test.

src/cli_util.rs

@@ -247,7 +247,6 @@ jj init --git-repo=.";
                 let base_repo = repo_loader.load_at(&op_heads[0]);
                 // TODO: It may be helpful to print each operation we're merging here
                 let mut workspace_command = self.for_loaded_repo(ui, workspace, base_repo)?;
-                workspace_command.snapshot(ui)?;
                 let mut tx = workspace_command.start_transaction("resolve concurrent operations");
                 for other_op_head in op_heads.into_iter().skip(1) {
                     tx.merge_operation(other_op_head);
@@ -264,6 +263,7 @@ jj init --git-repo=.";
                 let merged_repo = tx.write().leave_unpublished();
                 locked_op_heads.finish(merged_repo.operation());
                 workspace_command.repo = merged_repo;
+                workspace_command.snapshot(ui)?;
                 return Ok(workspace_command);
             }
         };

tests/test_concurrent_operations.rs

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use regex::Regex;
+
 use crate::common::TestEnvironment;
 
 pub mod common;
@@ -66,3 +68,81 @@ fn test_concurrent_operations_auto_rebase() {
     o 0000000000000000000000000000000000000000 (no description set)
     "###);
 }
+
+#[test]
+fn test_concurrent_operations_wc_modified() {
+    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");
+
+    std::fs::write(repo_path.join("file"), "contents\n").unwrap();
+    test_env.jj_cmd_success(&repo_path, &["describe", "-m", "initial"]);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]);
+    let op_id_hex = stdout[2..14].to_string();
+
+    test_env.jj_cmd_success(
+        &repo_path,
+        &["new", "--at-op", &op_id_hex, "-m", "new child1"],
+    );
+    test_env.jj_cmd_success(
+        &repo_path,
+        &["new", "--at-op", &op_id_hex, "-m", "new child2"],
+    );
+    std::fs::write(repo_path.join("file"), "modified\n").unwrap();
+
+    // We should be informed about the concurrent modification
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", "commit_id \" \" description"]);
+    insta::assert_snapshot!(stdout, @r###"
+    Concurrent modification detected, resolving automatically.
+    @ 304d2b4b70536e0bfd7a38394db584ee069a3b1a new child1
+    | o ac08e56f9b802269864c5061f2a7305b9258a671 new child2
+    |/  
+    o 5af56dcc2cc27bb234e5574b5a3ebc5f22081462 initial
+    o 0000000000000000000000000000000000000000 (no description set)
+    "###);
+    let stdout = test_env.jj_cmd_success(&repo_path, &["diff", "--git"]);
+    insta::assert_snapshot!(stdout, @r###"
+    diff --git a/file b/file
+    index 12f00e90b6...2e0996000b 100644
+    --- a/file
+    +++ b/file
+    @@ -1,1 +1,1 @@
+    -contents
+    +modified
+    "###);
+
+    // The working copy should be committed after merging the operations
+    let stdout = test_env.jj_cmd_success(&repo_path, &["op", "log"]);
+    insta::assert_snapshot!(redact_op_log(&stdout), @r###"
+    @ 
+    | commit working copy
+    o   
+    |\  resolve concurrent operations
+    | | 
+    o | 
+    | | new empty commit
+    | | 
+    | o 
+    |/  new empty commit
+    |   
+    o 
+    | describe commit cf911c223d3e24e001fc8264d6dbf0610804fc40
+    | 
+    o 
+    | commit working copy
+    o 
+    | 
+    o 
+      initialize repo
+    "###);
+}
+
+fn redact_op_log(stdout: &str) -> String {
+    let mut lines = vec![];
+    // Filter out the operation id etc, and the CLI arguments
+    let unwanted = Regex::new(r" ([0-9a-f]+|args:) .*").unwrap();
+    for line in stdout.lines() {
+        lines.push(unwanted.replace(line, " ").to_string());
+    }
+    lines.join("\n")
+}