WIP: Handle complex descendants

cli/src/commands/parallelize.rs

@@ -76,8 +76,12 @@ pub(crate) fn cmd_parallelize(
     let mut tx = workspace_command.start_transaction();
     let target_revset = tx
         .base_workspace_helper()
-        .parse_union_revsets(&args.revisions)?;
-    let target_commits: Vec<Commit> = target_revset
+        .parse_union_revsets(&args.revisions)?
+        .expression()
+        .clone();
+    let target_commits: Vec<Commit> = tx
+        .base_workspace_helper()
+        .attach_revset_evaluator(target_revset.clone())?
         .evaluate()?
         .iter()
         // We want parents before children, so we need to reverse the order.
@@ -89,7 +93,7 @@ pub(crate) fn cmd_parallelize(
     }
     let target_heads: Vec<CommitId> = tx
         .base_workspace_helper()
-        .attach_revset_evaluator(target_revset.expression().clone().heads())?
+        .attach_revset_evaluator(target_revset.heads())?
         .evaluate()?
         .iter()
         .collect();
@@ -101,7 +105,7 @@ pub(crate) fn cmd_parallelize(
     }
     let target_roots: Vec<CommitId> = tx
         .base_workspace_helper()
-        .attach_revset_evaluator(target_revset.expression().clone().roots())?
+        .attach_revset_evaluator(target_revset.roots())?
         .evaluate()?
         .iter()
         .collect();
@@ -111,34 +115,61 @@ pub(crate) fn cmd_parallelize(
             "Cannot parallelize a set of revisions with multiple roots. Roots: {roots}"
         )));
     }
+    let connected_length =
+        RevsetExpression::commits(vec![target_heads[0].clone(), target_roots[0].clone()])
+            .connected()
+            .evaluate_programmatic(tx.base_repo().as_ref())?
+            .iter()
+            .count();
+    if connected_length != target_commits.len() {
+        return Err(user_error(
+            "Cannot parallelize since the target revisions are not connected.",
+        ));
+    }
 
-    // Rebase the children of the head commit onto the target commits.
-    let new_children: Vec<Commit> = RevsetExpression::commit(target_heads[0].clone())
-        .children()
-        .evaluate_programmatic(tx.base_repo().as_ref())?
-        .iter()
-        .commits(tx.base_repo().store())
-        .try_collect()?;
-    for child in new_children {
-        // The ordering here is intentional. We iterate over the target commits first so
-        // that when `jj log` prints the graph they will be printed in their original
-        // order. After that, we add any existing parents which aren't being
+    // Rebase the children of each target commit onto its new parents. A child
+    // which had a target commit as an ancestor before parallelize ran will have
+    // the target commit as a parent afterward.
+    for target_commit in target_commits.iter() {
+        // These parents are shared by all children of the target commit. This
+        // includes the target commit itself plus any ancestors which are being
         // parallelized.
-        let new_parents_for_child: Vec<Commit> = target_commits
-            .iter()
-            .map(|c| c.id().clone())
-            .chain(child.parent_ids().iter().cloned())
-            .collect::<IndexSet<CommitId>>()
+        let common_parents: IndexSet<CommitId> =
+            tx.base_workspace_helper()
+                .attach_revset_evaluator(target_revset.intersection(
+                    &RevsetExpression::commit(target_commit.id().clone()).ancestors(),
+                ))?
+                .evaluate()?
+                .iter()
+                // We want parents before children, so we need to reverse the order.
+                .reversed()
+                .collect();
+        let children: Vec<Commit> = tx
+            .base_workspace_helper()
+            .attach_revset_evaluator(
+                RevsetExpression::commit(target_commit.id().clone())
+                    .children()
+                    .minus(&target_revset),
+            )?
+            .evaluate()?
             .iter()
-            .map(|c| tx.mut_repo().store().get_commit(c))
-            .collect::<Result<Vec<Commit>, _>>()?;
-        rebase_descendants(
-            &mut tx,
-            command.settings(),
-            &new_parents_for_child,
-            &[child],
-            Default::default(),
-        )?;
+            .commits(tx.base_repo().store())
+            .try_collect()?;
+        for child in children {
+            let mut new_parents = common_parents.clone();
+            new_parents.extend(child.parent_ids().iter().cloned());
+            let parents = new_parents
+                .iter()
+                .map(|c| tx.mut_repo().store().get_commit(c))
+                .try_collect::<_, Vec<Commit>, _>()?;
+            rebase_descendants(
+                &mut tx,
+                command.settings(),
+                &parents,
+                &[child],
+                Default::default(),
+            )?;
+        }
     }
 
     // Rebase the target commits onto the parents of the root commit.

cli/tests/test_parallelize_command.rs

@@ -123,17 +123,14 @@ fn test_parallelize_with_descendants_complex() {
     │ │ │ ◉  b8f977c12383 4
     │ │ ◉ │  7be8374575b9 3
     │ │ ├─╯
-    ◉ │ │  2bfe3fe3e472 1
-    ├───╯
-    │ │ ◉  bf7eef1f1dc6 child-of-2
+    │ │ │ ◉  3b27a65c2742 child-of-2
+    ╭─┬───╯
+    │ ◉ │  96ce11389312 2
     │ ├─╯
-    │ ◉  96ce11389312 2
+    ◉ │  2bfe3fe3e472 1
     ├─╯
     ◉  000000000000
     "###);
-    // TODO: This is wrong. the "child-of-2" branch should have 1 as a parent as
-    // well.
-    panic!();
 }
 
 #[test]
@@ -156,22 +153,9 @@ fn test_parallelize_failure_disjoint_target_commits() {
     ◉  000000000000
     "###);
 
-    test_env.jj_cmd_ok(&workspace_path, &["parallelize", "dc0", "9f5"]);
-    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  3d657803ddcd 6
-    ◉  b82bb741509e 5
-    ◉    aa0a032f2d92 4
-    ├─╮
-    │ ◉  a9334ecaa379 3
-    │ │ ◉  f382cea5a7a8 2
-    ├───╯
-    ◉ │  761e67df44b7 1
-    ├─╯
-    ◉  000000000000
+    insta::assert_snapshot!(test_env.jj_cmd_failure(&workspace_path, &["parallelize", "dc0", "9f5"]),@r###"
+    Error: Cannot parallelize since the target revisions are not connected.
     "###);
-    // TODO: this isn't right. Either we should return an error, or 4 should
-    // have 2 as a parent instead of 1.
-    panic!();
 }
 
 #[test]