repo: when abandoning a working copy that a merge, recreate it

I recently needed to test something on top of a two branches at the
same time, so I created a new commit on top of both of them (i.e. a
merge commit). I then ran tests and made some adjustments to the
code. These adjustments belonged in one of the parent branches, so I
used `jj squash --into` to squash it in there. Unfortunately, that
meant that my working copy became a single-parent commit based on one
of the branches only. We already had #2859 for tracking this issue.

This patch changes the behavior so we create a new working-copy commit
with all of the previous parents.

CHANGELOG.md

@@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   key](https://toml.io/en/v1.0.0#keys). Quote meta characters as needed.
   Example: `jj config get "revset-aliases.'trunk()'"`
 
+* If a new working-copy commit is created because the old one was abandoned, and
+  the old commit was merge, then the new commit will now also be.
+  [#2859](https://github.com/martinvonz/jj/issues/2859)
+
 ### Deprecations
 
 - Attempting to alias a built-in command now gives a warning, rather than being silently ignored.

cli/tests/test_abandon_command.rs

@@ -78,39 +78,44 @@ fn test_basics() {
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Abandoned commit znkkpsqq 5557ece3 e | e
-    Working copy now at: nkmrtpmo 6b527513 (empty) (no description set)
+    Working copy now at: nkmrtpmo d4f8ea73 (empty) (no description set)
     Parent commit      : rlvkpnrz 2443ea76 a e?? | a
-    Added 0 files, modified 0 files, removed 3 files
+    Parent commit      : vruxwmqv b7c62f28 d e?? | d
+    Added 0 files, modified 0 files, removed 1 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  [nkm]
-    │ ◉  [zsu] b
-    ├─╯
-    ◉  [rlv] a e??
+    @    [nkm]
+    ├─╮
     │ ◉  [vru] d e??
     │ ◉  [roy] c
+    │ │ ◉  [zsu] b
+    ├───╯
+    ◉ │  [rlv] a e??
     ├─╯
     ◉  [zzz]
     "###);
 
     test_env.jj_cmd_ok(&repo_path, &["undo"]);
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "descendants(c)"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "descendants(d)"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Abandoned the following commits:
       znkkpsqq 5557ece3 e | e
       vruxwmqv b7c62f28 d | d
-      royxmykx fe2e8e8b c | c
-    Working copy now at: xtnwkqum e7bb0612 (empty) (no description set)
+    Working copy now at: xtnwkqum fa4ee8e6 (empty) (no description set)
     Parent commit      : rlvkpnrz 2443ea76 a e?? | a
-    Added 0 files, modified 0 files, removed 3 files
+    Parent commit      : royxmykx fe2e8e8b c d e?? | c
+    Added 0 files, modified 0 files, removed 2 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  [xtn]
-    │ ◉  [zsu] b
+    @    [xtn]
+    ├─╮
+    │ ◉  [roy] c d e??
+    │ │ ◉  [zsu] b
+    ├───╯
+    ◉ │  [rlv] a e??
     ├─╯
-    ◉  [rlv] a e??
-    ◉  [zzz] c d e??
+    ◉  [zzz]
     "###);
 
     // Test abandoning the same commit twice directly
@@ -132,23 +137,26 @@ fn test_basics() {
 
     // Test abandoning the same commit twice indirectly
     test_env.jj_cmd_ok(&repo_path, &["undo"]);
-    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "d::", "a::"]);
+    let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "d::", "e"]);
     insta::assert_snapshot!(stdout, @"");
     insta::assert_snapshot!(stderr, @r###"
     Abandoned the following commits:
       znkkpsqq 5557ece3 e | e
       vruxwmqv b7c62f28 d | d
-      zsuskuln 1394f625 b | b
-      rlvkpnrz 2443ea76 a | a
-    Working copy now at: xlzxqlsl af874bff (empty) (no description set)
-    Parent commit      : zzzzzzzz 00000000 a b e?? | (empty) (no description set)
-    Added 0 files, modified 0 files, removed 4 files
+    Working copy now at: xlzxqlsl 14991aec (empty) (no description set)
+    Parent commit      : rlvkpnrz 2443ea76 a e?? | a
+    Parent commit      : royxmykx fe2e8e8b c d e?? | c
+    Added 0 files, modified 0 files, removed 2 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  [xlz]
+    @    [xlz]
+    ├─╮
     │ ◉  [roy] c d e??
+    │ │ ◉  [zsu] b
+    ├───╯
+    ◉ │  [rlv] a e??
     ├─╯
-    ◉  [zzz] a b e??
+    ◉  [zzz]
     "###);
 
     let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "none()"]);

lib/src/repo.rs

@@ -51,7 +51,7 @@ use crate::refs::{
     diff_named_ref_targets, diff_named_remote_refs, merge_ref_targets, merge_remote_refs,
 };
 use crate::revset::{RevsetEvaluationError, RevsetExpression, RevsetIteratorExt};
-use crate::rewrite::{CommitRewriter, DescendantRebaser, RebaseOptions};
+use crate::rewrite::{merge_commit_trees, CommitRewriter, DescendantRebaser, RebaseOptions};
 use crate::settings::{RepoSettings, UserSettings};
 use crate::signing::{SignInitError, Signer};
 use crate::simple_op_heads_store::SimpleOpHeadsStore;
@@ -1027,15 +1027,14 @@ impl MutableRepo {
         old_commit_id: CommitId,
         new_commit_ids: Vec<CommitId>,
     ) -> BackendResult<()> {
-        // We arbitrarily pick a new working-copy commit among the candidates.
         let abandoned_old_commit = matches!(
             self.parent_mapping.get(&old_commit_id),
             Some(Rewrite::Abandoned(_))
         );
         self.update_wc_commits(
             settings,
             &old_commit_id,
-            &new_commit_ids[0],
+            &new_commit_ids,
             abandoned_old_commit,
         )?;
 
@@ -1082,22 +1081,27 @@ impl MutableRepo {
         &mut self,
         settings: &UserSettings,
         old_commit_id: &CommitId,
-        new_commit_id: &CommitId,
+        new_commit_ids: &[CommitId],
         abandoned_old_commit: bool,
     ) -> BackendResult<()> {
         let workspaces_to_update = self.view().workspaces_for_wc_commit_id(old_commit_id);
         if workspaces_to_update.is_empty() {
             return Ok(());
         }
 
-        let new_commit = self.store().get_commit(new_commit_id)?;
         let new_wc_commit = if !abandoned_old_commit {
-            new_commit
+            // We arbitrarily pick a new working-copy commit among the candidates.
+            self.store().get_commit(&new_commit_ids[0])?
         } else {
+            let new_commits: Vec<_> = new_commit_ids
+                .iter()
+                .map(|id| self.store().get_commit(id))
+                .try_collect()?;
+            let merged_parents_tree = merge_commit_trees(self, &new_commits)?;
             self.new_commit(
                 settings,
-                vec![new_commit.id().clone()],
-                new_commit.tree_id().clone(),
+                new_commit_ids.to_vec(),
+                merged_parents_tree.id().clone(),
             )
             .write()?
         };

lib/tests/test_rewrite.rs

@@ -1492,7 +1492,7 @@ fn test_rebase_descendants_update_checkout_abandoned_merge() {
     let test_repo = TestRepo::init();
     let repo = &test_repo.repo;
 
-    // Checked-out merge commit D was abandoned. A parent commit should become
+    // Checked-out merge commit D was abandoned. A new merge commit should become
     // checked out.
     //
     // D
@@ -1527,7 +1527,7 @@ fn test_rebase_descendants_update_checkout_abandoned_merge() {
 
     let new_checkout_id = repo.view().get_wc_commit_id(&workspace_id).unwrap();
     let checkout = repo.store().get_commit(new_checkout_id).unwrap();
-    assert_eq!(checkout.parent_ids(), vec![commit_b.id().clone()]);
+    assert_eq!(checkout.parent_ids(), vec![commit_b.id().clone(), commit_c.id().clone()]);
 }
 
 #[test_case(EmptyBehaviour::Keep; "keep all commits")]