EXAMPLE: Rewrite commit to multiple commits

CHANGELOG.md

@@ -44,6 +44,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * Revsets now support `\`-escapes in string literal.
 
+* Fixed a bug with `jj split` introduced in 0.16.0 that caused it to incorrectly
+  rebase the children of the revision being split if they had other parents
+  (i.e. if the child was a merge).
 
 ## [0.16.0] - 2024-04-03
 

cli/src/commands/split.rs

@@ -19,9 +19,10 @@ use jj_lib::object_id::ObjectId;
 use jj_lib::repo::Repo;
 use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
 use jj_lib::rewrite::merge_commit_trees;
+use jj_lib::settings::UserSettings;
 use tracing::instrument;
 
-use crate::cli_util::{CommandHelper, RevisionArg};
+use crate::cli_util::{CommandHelper, RevisionArg, WorkspaceCommandTransaction};
 use crate::command_error::CommandError;
 use crate::commands::rebase::rebase_descendants;
 use crate::description_util::{description_template_for_commit, edit_description};
@@ -174,27 +175,17 @@ the operation will be aborted.
     // second commit after the command finishes. This also means that any
     // branches pointing to the commit being split are moved to the second
     // commit.
-    tx.mut_repo()
-        .set_rewritten_commit(commit.id().clone(), second_commit.id().clone());
-    // Rebase descendants of the commit being split.
-    let new_parents = if args.siblings {
-        vec![first_commit.clone(), second_commit.clone()]
+    if args.siblings {
+        tx.mut_repo().set_rewritten_commit_multiple(
+            commit.id().clone(),
+            vec![first_commit.id().clone(), second_commit.id().clone()],
+        );
     } else {
-        vec![second_commit.clone()]
+        tx.mut_repo()
+            .set_rewritten_commit(commit.id().clone(), second_commit.id().clone());
     };
-    let children: Vec<Commit> = RevsetExpression::commit(commit.id().clone())
-        .children()
-        .evaluate_programmatic(tx.base_repo().as_ref())?
-        .iter()
-        .commits(tx.base_repo().store())
-        .try_collect()?;
-    let num_rebased = rebase_descendants(
-        &mut tx,
-        command.settings(),
-        &new_parents,
-        &children,
-        Default::default(),
-    )?;
+    let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
+
     if let Some(mut formatter) = ui.status_formatter() {
         if num_rebased > 0 {
             writeln!(formatter, "Rebased {num_rebased} descendant commits")?;

cli/tests/test_split_command.rs

@@ -296,9 +296,9 @@ JJ: Lines starting with "JJ: " (like this one) will be removed.
     );
     assert!(!test_env.env_root().join("editor2").exists());
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  zsuskulnrvyr false test_branch
-    │ ◉  qpvuntsmwlqt false TESTED=TODO
-    ├─╯
+    ◉  zsuskulnrvyr false test_branch??
+    │ @  qpvuntsmwlqt false TESTED=TODO
+    ├─╯   test_branch??
     ◉  zzzzzzzzzzzz true
     "###);
 }
@@ -371,8 +371,55 @@ JJ: Lines starting with "JJ: " (like this one) will be removed.
     ◉  kkmpptxzrspx false Add file4
     ◉    rlvkpnrzqnoo false Add file3
     ├─╮
-    │ @  yqosqzytrlsw false Add file2
-    ◉ │  qpvuntsmwlqt false Add file1
+    │ ◉  yqosqzytrlsw false Add file2
+    @ │  qpvuntsmwlqt false Add file1
+    ├─╯
+    ◉  zzzzzzzzzzzz true
+    "###);
+}
+
+// This test makes sure that the children of the commit being split retain any
+// other parents which weren't involved in the split.
+#[test]
+fn test_split_siblings_with_merge_child() {
+    let mut test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
+    let workspace_path = test_env.env_root().join("repo");
+    test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=1"]);
+    test_env.jj_cmd_ok(&workspace_path, &["new", "root()", "-m=a"]);
+    std::fs::write(workspace_path.join("file1"), "foo\n").unwrap();
+    std::fs::write(workspace_path.join("file2"), "bar\n").unwrap();
+    test_env.jj_cmd_ok(
+        &workspace_path,
+        &["new", "description(1)", "description(a)", "-m=2"],
+    );
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @    zsuskulnrvyr true 2
+    ├─╮
+    │ ◉  kkmpptxzrspx false a
+    ◉ │  qpvuntsmwlqt true 1
+    ├─╯
+    ◉  zzzzzzzzzzzz true
+    "###);
+
+    // Set up the editor and do the split.
+    let edit_script = test_env.set_up_fake_editor();
+    std::fs::write(
+        edit_script,
+        ["write\nAdd file1", "next invocation\n", "write\nAdd file2"].join("\0"),
+    )
+    .unwrap();
+    test_env.jj_cmd_ok(
+        &workspace_path,
+        &["split", "-r", "description(a)", "--siblings", "file1"],
+    );
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @      zsuskulnrvyr true 2
+    ├─┬─╮
+    │ │ ◉  royxmykxtrkr false Add file2
+    │ ◉ │  kkmpptxzrspx false Add file1
+    │ ├─╯
+    ◉ │  qpvuntsmwlqt true 1
     ├─╯
     ◉  zzzzzzzzzzzz true
     "###);

lib/src/repo.rs

@@ -831,6 +831,12 @@ impl MutableRepo {
             .insert(old_id, (RewriteType::Rewritten, vec![new_id]));
     }
 
+    pub fn set_rewritten_commit_multiple(&mut self, old_id: CommitId, new_ids: Vec<CommitId>) {
+        assert_ne!(old_id, *self.store().root_commit_id());
+        self.parent_mapping
+            .insert(old_id, (RewriteType::Rewritten, new_ids));
+    }
+
     /// Record a commit as being rewritten into multiple other commits in this
     /// transaction.
     ///