From c5f73d4619c1b0260b5c8635bb6421433079b318 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Mon, 15 Apr 2024 22:49:59 -0700
Subject: [PATCH 1/9] repo: add method for tranforming descendants, use in
 `rebase_descendants()`

There are several existing commands that would benefit from an API
that makes it easier to rewrite a whole graph of commits while
transforming them in some way.

`jj squash` is one example. When squashing into an ancestor, that
command currently rewrites the ancestor, then rebases descendants, and
then rewrites the rewritten source commit. It would be better to
rewrite the source commit (and any descendants) only once.

Another example is the future `jj fix`. That command will want to
rewrite a graph while updating the trees. There's currently no good
API for that; you have to manually iterate over descendants and
rewrite them.

This patch adds a new `MutableRepo::transform_descendants()` method
that takes a callback which gets a `CommitRewriter` passed to it. The
callback can then decide to change the parents, the tree, etc. The
callback is also free to leave the commit in place or to abandon it.

I updated the regular `rebase_descendants()` to use the new function
in order to exercise it. I hope we can replace all of the
`rebase_descendant_*()` flavors later.

I added a `replace_parent()` method that was a bit useful for the test
case. It could easily be hard-coded in the test case instead, but I
think the method will be useful for `jj git sync` and similar in the
future.
---
 lib/src/repo.rs                     | 52 +++++++++++++++++-
 lib/src/rewrite.rs                  |  9 ++++
 lib/tests/runner.rs                 |  1 +
 lib/tests/test_rewrite_transform.rs | 84 +++++++++++++++++++++++++++++
 4 files changed, 144 insertions(+), 2 deletions(-)
 create mode 100644 lib/tests/test_rewrite_transform.rs

diff --git a/lib/src/repo.rs b/lib/src/repo.rs
index d136babbfe..24283ad3ea 100644
--- a/lib/src/repo.rs
+++ b/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::{DescendantRebaser, RebaseOptions};
+use crate::rewrite::{CommitRewriter, DescendantRebaser, RebaseOptions};
 use crate::settings::{RepoSettings, UserSettings};
 use crate::signing::{SignInitError, Signer};
 use crate::simple_op_heads_store::SimpleOpHeadsStore;
@@ -1154,6 +1154,42 @@ impl MutableRepo {
         )
     }
 
+    /// Rewrite descendants of the given roots.
+    ///
+    /// The callback will be called for each commit with the new parents
+    /// prepopulated. The callback may change the parents and write the new
+    /// commit, or it may abandon the commit, or it may leave the old commit
+    /// unchanged.
+    ///
+    /// The set of commits to visit is determined at the start. If the callback
+    /// adds new descendants, then the callback will not be called for those.
+    /// Similarly, if the callback rewrites unrelated commits, then the callback
+    /// will not be called for descendants of those commits.
+    pub fn transform_descendants(
+        &mut self,
+        settings: &UserSettings,
+        roots: Vec<CommitId>,
+        mut callback: impl FnMut(CommitRewriter) -> BackendResult<()>,
+    ) -> BackendResult<()> {
+        let mut to_visit = self.find_descendants_to_rebase(roots)?;
+        while let Some(old_commit) = to_visit.pop() {
+            let new_parent_ids = self.new_parents(old_commit.parent_ids());
+            let rewriter = CommitRewriter::new(self, old_commit, new_parent_ids);
+            callback(rewriter)?;
+        }
+        self.update_rewritten_references(settings)?;
+        // Since we didn't necessarily visit all descendants of rewritten commits (e.g.
+        // if they were rewritten in the callback), there can still be commits left to
+        // rebase, so we don't clear `parent_mapping` here.
+        // TODO: Should we make this stricter? We could check that there were no
+        // rewrites before this function was called, and we can check that only
+        // commits in the `to_visit` set were added by the callback. Then we
+        // could clear `parent_mapping` here and not have to scan it again at
+        // the end of the transaction when we call `rebase_descendants()`.
+
+        Ok(())
+    }
+
     /// After the rebaser returned by this function is dropped,
     /// self.parent_mapping needs to be cleared.
     fn rebase_descendants_return_rebaser<'settings, 'repo>(
@@ -1202,6 +1238,7 @@ impl MutableRepo {
     /// **its parent**. One can tell this case apart since the change ids of the
     /// key and the value will not match. The parent will inherit the
     /// descendants and the branches of the abandoned commit.
+    // TODO: Rewrite this using `transform_descendants()`
     pub fn rebase_descendants_with_options_return_map(
         &mut self,
         settings: &UserSettings,
@@ -1218,7 +1255,18 @@ impl MutableRepo {
     }
 
     pub fn rebase_descendants(&mut self, settings: &UserSettings) -> BackendResult<usize> {
-        self.rebase_descendants_with_options(settings, Default::default())
+        let roots = self.parent_mapping.keys().cloned().collect_vec();
+        let mut num_rebased = 0;
+        self.transform_descendants(settings, roots, |rewriter| {
+            if rewriter.parents_changed() {
+                let builder = rewriter.rebase(settings)?;
+                builder.write()?;
+                num_rebased += 1;
+            }
+            Ok(())
+        })?;
+        self.parent_mapping.clear();
+        Ok(num_rebased)
     }
 
     pub fn rebase_descendants_return_map(
diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index c9249459bd..d67f050ae5 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -145,6 +145,15 @@ impl<'repo> CommitRewriter<'repo> {
         self.new_parents = new_parents;
     }
 
+    /// Update the intended new parents by replacing any occurrence of
+    /// `old_parent` by `new_parents`
+    pub fn replace_parent(&mut self, old_parent: &CommitId, new_parents: &[CommitId]) {
+        if let Some(i) = self.new_parents.iter().position(|p| p == old_parent) {
+            self.new_parents
+                .splice(i..i + 1, new_parents.iter().cloned());
+        }
+    }
+
     /// Checks if the intended new parents are different from the old commit's
     /// parents.
     pub fn parents_changed(&self) -> bool {
diff --git a/lib/tests/runner.rs b/lib/tests/runner.rs
index 44f939ef3d..03f134bbde 100644
--- a/lib/tests/runner.rs
+++ b/lib/tests/runner.rs
@@ -29,6 +29,7 @@ mod test_operations;
 mod test_refs;
 mod test_revset;
 mod test_rewrite;
+mod test_rewrite_transform;
 mod test_signing;
 mod test_ssh_signing;
 mod test_view;
diff --git a/lib/tests/test_rewrite_transform.rs b/lib/tests/test_rewrite_transform.rs
new file mode 100644
index 0000000000..014f521197
--- /dev/null
+++ b/lib/tests/test_rewrite_transform.rs
@@ -0,0 +1,84 @@
+// Copyright 2024 The Jujutsu Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use std::collections::HashMap;
+
+use jj_lib::repo::Repo;
+use maplit::hashset;
+use testutils::{CommitGraphBuilder, TestRepo};
+
+// Simulate some `jj sync` command that rebases B:: onto G while abandoning C
+// (because it's presumably already in G).
+//
+// G
+// | E
+// | D F
+// | |/
+// | C
+// | B
+// |/
+// A
+#[test]
+fn test_transform_descendants_sync() {
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init();
+    let repo = &test_repo.repo;
+
+    let mut tx = repo.start_transaction(&settings);
+    let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
+    let commit_a = graph_builder.initial_commit();
+    let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
+    let commit_c = graph_builder.commit_with_parents(&[&commit_b]);
+    let commit_d = graph_builder.commit_with_parents(&[&commit_c]);
+    let commit_e = graph_builder.commit_with_parents(&[&commit_d]);
+    let commit_f = graph_builder.commit_with_parents(&[&commit_c]);
+    let commit_g = graph_builder.commit_with_parents(&[&commit_a]);
+
+    let mut rebased = HashMap::new();
+    tx.mut_repo()
+        .transform_descendants(&settings, vec![commit_b.id().clone()], |mut rewriter| {
+            rewriter.replace_parent(commit_a.id(), &[commit_g.id().clone()]);
+            if *rewriter.old_commit() == commit_c {
+                let old_id = rewriter.old_commit().id().clone();
+                let new_parent_ids = rewriter.new_parents().to_vec();
+                rewriter
+                    .mut_repo()
+                    .record_abandoned_commit_with_parents(old_id, new_parent_ids);
+            } else {
+                let old_commit_id = rewriter.old_commit().id().clone();
+                let new_commit = rewriter.rebase(&settings)?.write()?;
+                rebased.insert(old_commit_id, new_commit);
+            }
+            Ok(())
+        })
+        .unwrap();
+    assert_eq!(rebased.len(), 4);
+    let new_commit_b = rebased.get(commit_b.id()).unwrap();
+    let new_commit_d = rebased.get(commit_d.id()).unwrap();
+    let new_commit_e = rebased.get(commit_e.id()).unwrap();
+    let new_commit_f = rebased.get(commit_f.id()).unwrap();
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {
+            new_commit_e.id().clone(),
+            new_commit_f.id().clone(),
+        }
+    );
+
+    assert_eq!(new_commit_b.parent_ids(), vec![commit_g.id().clone()]);
+    assert_eq!(new_commit_d.parent_ids(), vec![new_commit_b.id().clone()]);
+    assert_eq!(new_commit_e.parent_ids(), vec![new_commit_d.id().clone()]);
+    assert_eq!(new_commit_f.parent_ids(), vec![new_commit_b.id().clone()]);
+}

From b786a10668f87e76d3b5030c38c8097801b837d6 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 18 Apr 2024 08:43:38 -0700
Subject: [PATCH 2/9] rewrite: make `CommitRewriter::replace_parents()` remove
 repeats

---
 lib/src/rewrite.rs                  |  4 ++-
 lib/tests/test_rewrite_transform.rs | 43 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index d67f050ae5..8416859f5e 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -146,11 +146,13 @@ impl<'repo> CommitRewriter<'repo> {
     }
 
     /// Update the intended new parents by replacing any occurrence of
-    /// `old_parent` by `new_parents`
+    /// `old_parent` by `new_parents`.
     pub fn replace_parent(&mut self, old_parent: &CommitId, new_parents: &[CommitId]) {
         if let Some(i) = self.new_parents.iter().position(|p| p == old_parent) {
             self.new_parents
                 .splice(i..i + 1, new_parents.iter().cloned());
+            let mut unique = HashSet::new();
+            self.new_parents.retain(|p| unique.insert(p.clone()));
         }
     }
 
diff --git a/lib/tests/test_rewrite_transform.rs b/lib/tests/test_rewrite_transform.rs
index 014f521197..2a7c804b8d 100644
--- a/lib/tests/test_rewrite_transform.rs
+++ b/lib/tests/test_rewrite_transform.rs
@@ -82,3 +82,46 @@ fn test_transform_descendants_sync() {
     assert_eq!(new_commit_e.parent_ids(), vec![new_commit_d.id().clone()]);
     assert_eq!(new_commit_f.parent_ids(), vec![new_commit_b.id().clone()]);
 }
+
+// Transform just commit C replacing parent A by parent B. The parents should be
+// deduplicated.
+//
+//   C
+//  /|
+// B |
+// |/
+// A
+#[test]
+fn test_transform_descendants_sync_linearize_merge() {
+    let settings = testutils::user_settings();
+    let test_repo = TestRepo::init();
+    let repo = &test_repo.repo;
+
+    let mut tx = repo.start_transaction(&settings);
+    let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo());
+    let commit_a = graph_builder.initial_commit();
+    let commit_b = graph_builder.commit_with_parents(&[&commit_a]);
+    let commit_c = graph_builder.commit_with_parents(&[&commit_a, &commit_b]);
+
+    let mut rebased = HashMap::new();
+    tx.mut_repo()
+        .transform_descendants(&settings, vec![commit_c.id().clone()], |mut rewriter| {
+            rewriter.replace_parent(commit_a.id(), &[commit_b.id().clone()]);
+            let old_commit_id = rewriter.old_commit().id().clone();
+            let new_commit = rewriter.rebase(&settings)?.write()?;
+            rebased.insert(old_commit_id, new_commit);
+            Ok(())
+        })
+        .unwrap();
+    assert_eq!(rebased.len(), 1);
+    let new_commit_c = rebased.get(commit_c.id()).unwrap();
+
+    assert_eq!(
+        *tx.mut_repo().view().heads(),
+        hashset! {
+            new_commit_c.id().clone(),
+        }
+    );
+
+    assert_eq!(new_commit_c.parent_ids(), vec![commit_b.id().clone()]);
+}

From f0e8769c59ce7992d5d2b0a571b1e078afd86916 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 18 Apr 2024 09:22:34 -0700
Subject: [PATCH 3/9] repo: take owned commit IDs to
 `MutableRepo::new_parents()`

We always call `.to_vec()` on the slice, so let's just have the caller
pass in an owned vector instead.
---
 lib/src/repo.rs    | 8 ++++----
 lib/src/rewrite.rs | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/lib/src/repo.rs b/lib/src/repo.rs
index 24283ad3ea..6e01b528b8 100644
--- a/lib/src/repo.rs
+++ b/lib/src/repo.rs
@@ -926,7 +926,7 @@ impl MutableRepo {
     /// rewritten and abandoned.
     ///
     /// Panics if `parent_mapping` contains cycles
-    pub fn new_parents(&self, old_ids: &[CommitId]) -> Vec<CommitId> {
+    pub fn new_parents(&self, old_ids: Vec<CommitId>) -> Vec<CommitId> {
         fn single_substitution_round(
             parent_mapping: &HashMap<CommitId, (RewriteType, Vec<CommitId>)>,
             ids: Vec<CommitId>,
@@ -958,7 +958,7 @@ impl MutableRepo {
             (new_ids, made_replacements)
         }
 
-        let mut new_ids: Vec<CommitId> = old_ids.to_vec();
+        let mut new_ids = old_ids;
         // The longest possible non-cycle substitution sequence goes through each key of
         // parent_mapping once.
         let mut allowed_iterations = 0..self.parent_mapping.len();
@@ -995,7 +995,7 @@ impl MutableRepo {
             // mappings, not transitive ones.
             // TODO: keep parent_mapping updated with transitive mappings so we don't need
             // to call `new_parents()` here.
-            let new_parent_ids = self.new_parents(&new_parent_ids);
+            let new_parent_ids = self.new_parents(new_parent_ids);
             self.update_references(settings, old_parent_id, new_parent_ids)?;
         }
         Ok(())
@@ -1173,7 +1173,7 @@ impl MutableRepo {
     ) -> BackendResult<()> {
         let mut to_visit = self.find_descendants_to_rebase(roots)?;
         while let Some(old_commit) = to_visit.pop() {
-            let new_parent_ids = self.new_parents(old_commit.parent_ids());
+            let new_parent_ids = self.new_parents(old_commit.parent_ids().to_vec());
             let rewriter = CommitRewriter::new(self, old_commit, new_parent_ids);
             callback(rewriter)?;
         }
diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index 8416859f5e..220b851ea2 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -395,7 +395,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
     fn rebase_one(&mut self, old_commit: Commit) -> BackendResult<()> {
         let old_commit_id = old_commit.id().clone();
         let old_parent_ids = old_commit.parent_ids();
-        let new_parent_ids = self.mut_repo.new_parents(old_parent_ids);
+        let new_parent_ids = self.mut_repo.new_parents(old_parent_ids.to_vec());
         let rewriter = CommitRewriter::new(self.mut_repo, old_commit, new_parent_ids);
         if !rewriter.parents_changed() {
             // The commit is already in place.

From 22fc7051f1d46dd8b9b4824cddaada7214173147 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Mon, 15 Apr 2024 22:49:59 -0700
Subject: [PATCH 4/9] parallelize: rewrite using `transform_descendants()`

`jj parallelize` was a good example of a command that can be
simplified by the new API, so I decided to rewrite it as an example.

The rewritten version is more flexible and doesn't actually need the
restrictions from the old version (such as checking that the commits
are connected). I still left the check for now to keep this patch
somewhat small. A subsequent commit will remove the restrictions.
---
 cli/src/commands/parallelize.rs       | 111 ++++++++++++++------------
 cli/tests/test_parallelize_command.rs |   7 +-
 lib/src/rewrite.rs                    |   6 ++
 3 files changed, 69 insertions(+), 55 deletions(-)

diff --git a/cli/src/commands/parallelize.rs b/cli/src/commands/parallelize.rs
index 9e311d327c..7ebacad184 100644
--- a/cli/src/commands/parallelize.rs
+++ b/cli/src/commands/parallelize.rs
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::collections::HashSet;
+use std::collections::{HashMap, HashSet};
 use std::io::Write;
 use std::rc::Rc;
 
@@ -26,7 +26,6 @@ use tracing::instrument;
 
 use crate::cli_util::{CommandHelper, RevisionArg};
 use crate::command_error::{user_error, CommandError};
-use crate::commands::rebase::rebase_descendants;
 use crate::ui::Ui;
 
 /// Parallelize revisions by making them siblings
@@ -69,7 +68,7 @@ pub(crate) fn cmd_parallelize(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     // The target commits are the commits being parallelized. They are ordered
-    // here with parents before children.
+    // here with children before parents.
     let target_commits: Vec<Commit> = workspace_command
         .parse_union_revsets(&args.revisions)?
         .evaluate_to_commits()?
@@ -83,61 +82,69 @@ pub(crate) fn cmd_parallelize(
     let mut tx = workspace_command.start_transaction();
     let target_revset =
         RevsetExpression::commits(target_commits.iter().ids().cloned().collect_vec());
-    let new_parents =
+    // TODO: The checks are now unnecessary, so drop them
+    let _new_parents =
         check_preconditions_and_get_new_parents(&target_revset, &target_commits, tx.repo())?;
 
-    // Rebase the non-target 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() {
-        // Children of the target commit, excluding other target commits.
-        let children: Vec<Commit> = RevsetExpression::commit(target_commit.id().clone())
-            .children()
-            .minus(&target_revset)
-            .evaluate_programmatic(tx.repo())?
-            .iter()
-            .commits(tx.repo().store())
-            .try_collect()?;
-        // These parents are shared by all children of the target commit and
-        // include the target commit itself plus any of its ancestors which are
-        // being parallelized.
-        let common_parents: IndexSet<Commit> = RevsetExpression::commit(target_commit.id().clone())
-            .ancestors()
-            .intersection(&target_revset)
-            .evaluate_programmatic(tx.repo())?
-            .iter()
-            .commits(tx.repo().store())
-            .try_collect()?;
-        for child in children {
-            let mut new_parents = common_parents.clone();
-            new_parents.extend(child.parents().into_iter());
-            rebase_descendants(
-                &mut tx,
-                command.settings(),
-                new_parents.into_iter().collect_vec(),
-                &[child],
-                Default::default(),
-            )?;
+    // New parents for commits in the target set. Since commits in the set are now
+    // supposed to be independent, they inherit the parent's non-target parents,
+    // recursively.
+    let mut new_target_parents: HashMap<CommitId, Vec<CommitId>> = HashMap::new();
+    for commit in target_commits.iter().rev() {
+        let mut new_parents = vec![];
+        for old_parent in commit.parent_ids() {
+            if let Some(grand_parents) = new_target_parents.get(old_parent) {
+                new_parents.extend_from_slice(grand_parents);
+            } else {
+                new_parents.push(old_parent.clone());
+            }
         }
+        new_target_parents.insert(commit.id().clone(), new_parents);
     }
 
-    // Rebase the target commits onto the parents of the root commit.
-    // We already checked that the roots have the same parents, so we can just
-    // use the first one.
-    let target_commits = target_commits
-        .into_iter()
-        // We need to reverse the iterator so that when we rebase the target
-        // commits they will appear in the same relative order in `jj log` that
-        // they were in before being parallelized. After reversing, the commits
-        // are ordered with children before parents.
-        .rev()
-        .collect_vec();
-    rebase_descendants(
-        &mut tx,
+    // If a commit outside the target set has a commit in the target set as parent,
+    // then - after the transformation - it should also have that commit's
+    // parents as direct parents, if those commits are also in the target set.
+    let mut new_child_parents: HashMap<CommitId, IndexSet<CommitId>> = HashMap::new();
+    for commit in target_commits.iter().rev() {
+        let mut new_parents = IndexSet::new();
+        for old_parent in commit.parent_ids() {
+            if let Some(parents) = new_child_parents.get(old_parent) {
+                new_parents.extend(parents.iter().cloned());
+            }
+        }
+        new_parents.insert(commit.id().clone());
+        new_child_parents.insert(commit.id().clone(), new_parents);
+    }
+
+    tx.mut_repo().transform_descendants(
         command.settings(),
-        new_parents,
-        &target_commits,
-        Default::default(),
+        target_commits.iter().ids().cloned().collect_vec(),
+        |mut rewriter| {
+            // Commits in the target set do not depend on each other but they still depend
+            // on other parents
+            if let Some(new_parents) = new_target_parents.get(rewriter.old_commit().id()) {
+                rewriter.set_new_rewritten_parents(new_parents.clone());
+            } else if rewriter
+                .old_commit()
+                .parent_ids()
+                .iter()
+                .any(|id| new_child_parents.contains_key(id))
+            {
+                let mut new_parents = vec![];
+                for parent in rewriter.old_commit().parent_ids() {
+                    if let Some(parents) = new_child_parents.get(parent) {
+                        new_parents.extend(parents.iter().cloned());
+                    } else {
+                        new_parents.push(parent.clone());
+                    }
+                }
+                rewriter.set_new_rewritten_parents(new_parents);
+            }
+            let builder = rewriter.rebase(command.settings())?;
+            builder.write()?;
+            Ok(())
+        },
     )?;
 
     tx.finish(ui, format!("parallelize {} commits", target_commits.len()))
diff --git a/cli/tests/test_parallelize_command.rs b/cli/tests/test_parallelize_command.rs
index 1f941f761e..bab82d20c9 100644
--- a/cli/tests/test_parallelize_command.rs
+++ b/cli/tests/test_parallelize_command.rs
@@ -162,12 +162,13 @@ fn test_parallelize_where_root_has_non_target_children() {
         &["parallelize", "description(1)::description(3)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    ◉  ad35c9caf4fb 1c
-    │ @    6ee674074e23 4
-    ╭─┼─╮
+    @      6ee674074e23 4
+    ├─┬─╮
     │ │ ◉  5bd049136a7c 3
     │ ◉ │  60f737a5a4a7 2
     │ ├─╯
+    │ │ ◉  ad35c9caf4fb 1c
+    ├───╯
     ◉ │  79ebcd81a1ee 1
     ├─╯
     ◉  000000000000
diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs
index 220b851ea2..c66c8dc015 100644
--- a/lib/src/rewrite.rs
+++ b/lib/src/rewrite.rs
@@ -145,6 +145,12 @@ impl<'repo> CommitRewriter<'repo> {
         self.new_parents = new_parents;
     }
 
+    /// Set the old commit's intended new parents to be the rewritten versions
+    /// of the given parents.
+    pub fn set_new_rewritten_parents(&mut self, unrewritten_parents: Vec<CommitId>) {
+        self.new_parents = self.mut_repo.new_parents(unrewritten_parents);
+    }
+
     /// Update the intended new parents by replacing any occurrence of
     /// `old_parent` by `new_parents`.
     pub fn replace_parent(&mut self, old_parent: &CommitId, new_parents: &[CommitId]) {

From 248ab6f32c0f13e7c0c462cda94a5d9608ddcda9 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Tue, 16 Apr 2024 00:16:55 -0700
Subject: [PATCH 5/9] parallelize: don't rewrite commits that keep their
 parents

The new API makes it easy to leave commits in place if their parents
didn't change, so let's do that.
---
 cli/src/commands/parallelize.rs       |  6 ++-
 cli/tests/test_parallelize_command.rs | 56 +++++++++++++--------------
 2 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/cli/src/commands/parallelize.rs b/cli/src/commands/parallelize.rs
index 7ebacad184..0de3615d04 100644
--- a/cli/src/commands/parallelize.rs
+++ b/cli/src/commands/parallelize.rs
@@ -141,8 +141,10 @@ pub(crate) fn cmd_parallelize(
                 }
                 rewriter.set_new_rewritten_parents(new_parents);
             }
-            let builder = rewriter.rebase(command.settings())?;
-            builder.write()?;
+            if rewriter.parents_changed() {
+                let builder = rewriter.rebase(command.settings())?;
+                builder.write()?;
+            }
             Ok(())
         },
     )?;
diff --git a/cli/tests/test_parallelize_command.rs b/cli/tests/test_parallelize_command.rs
index bab82d20c9..e3a09bb641 100644
--- a/cli/tests/test_parallelize_command.rs
+++ b/cli/tests/test_parallelize_command.rs
@@ -47,7 +47,7 @@ fn test_parallelize_no_descendants() {
     ├─╯
     │ ◉  3a7b37ebe843 2
     ├─╯
-    │ ◉  761e67df44b7 1
+    │ ◉  dc0e5d6135ce 1
     ├─╯
     ◉  000000000000
     "###);
@@ -79,15 +79,15 @@ fn test_parallelize_with_descendants_simple() {
         &["parallelize", "description(1)::description(4)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  f28f986c7134 6
-    ◉        21e9963ac5ff 5
+    @  259d624373d7 6
+    ◉        60d419591c77 5
     ├─┬─┬─╮
     │ │ │ ◉  524062469789 4
     │ │ ◉ │  a9334ecaa379 3
     │ │ ├─╯
     │ ◉ │  3a7b37ebe843 2
     │ ├─╯
-    ◉ │  761e67df44b7 1
+    ◉ │  dc0e5d6135ce 1
     ├─╯
     ◉  000000000000
     "###);
@@ -121,17 +121,17 @@ fn test_parallelize_where_interior_has_non_target_children() {
 
     test_env.jj_cmd_ok(&workspace_path, &["parallelize", "dc0::9df"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  9f1bec0d6c46 6
-    ◉        7dd2f5648395 5
+    @  a42de3959cae 6
+    ◉        d907c901bad0 5
     ├─┬─┬─╮
     │ │ │ ◉  b8f977c12383 4
     │ │ ◉ │  7be8374575b9 3
     │ │ ├─╯
-    │ │ │ ◉  679fc870858c 2c
+    │ │ │ ◉  2a4c3dab2a50 2c
     ╭─┬───╯
     │ ◉ │  96ce11389312 2
     │ ├─╯
-    ◉ │  2bfe3fe3e472 1
+    ◉ │  dc0e5d6135ce 1
     ├─╯
     ◉  000000000000
     "###);
@@ -162,14 +162,14 @@ fn test_parallelize_where_root_has_non_target_children() {
         &["parallelize", "description(1)::description(3)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @      6ee674074e23 4
+    @      d024344469c3 4
     ├─┬─╮
     │ │ ◉  5bd049136a7c 3
     │ ◉ │  60f737a5a4a7 2
     │ ├─╯
-    │ │ ◉  ad35c9caf4fb 1c
+    │ │ ◉  50e2ced81124 1c
     ├───╯
-    ◉ │  79ebcd81a1ee 1
+    ◉ │  dc0e5d6135ce 1
     ├─╯
     ◉  000000000000
     "###);
@@ -210,16 +210,16 @@ fn test_parallelize_with_merge_commit_child() {
         &["parallelize", "description(1)::description(3)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @      5a0dd49510d1 4
+    @      6107429ab54b 4
     ├─┬─╮
     │ │ ◉  a9334ecaa379 3
-    │ │ │ ◉  605371712469 2a-c
+    │ │ │ ◉  a386386b94bc 2a-c
     ╭─┬───┤
     │ │ │ ◉  1eb902150bb9 a
     │ │ ├─╯
     │ ◉ │  3a7b37ebe843 2
     │ ├─╯
-    ◉ │  761e67df44b7 1
+    ◉ │  dc0e5d6135ce 1
     ├─╯
     ◉  000000000000
     "###);
@@ -336,11 +336,11 @@ fn test_parallelize_root_is_a_merge() {
         &["parallelize", "description(1)::description(2)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @    4e81469adb0d 3
+    @    d6df04b236b0 3
     ├─╮
     │ ◉    38945baf55f4 2
     │ ├─╮
-    ◉ │ │  9b1a1927720c 1
+    ◉ │ │  4b4941342e06 1
     ╰─┬─╮
       │ ◉  4035b23c8f72 x
       ◉ │  f3ec359cf9ff y
@@ -370,7 +370,7 @@ fn test_parallelize_multiple_heads() {
     @  e84481c26195 2
     │ ◉  2047527ade93 1
     ├─╯
-    │ ◉  9d0c0750973c 0
+    │ ◉  a56846756248 0
     ├─╯
     ◉  000000000000
     "###);
@@ -400,9 +400,9 @@ fn test_parallelize_multiple_heads_with_and_without_children() {
         &["parallelize", "description(0)", "description(1)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  49fe9e130d15 2
-    ◉  9d0c0750973c 0
-    │ ◉  2047527ade93 1
+    ◉  2047527ade93 1
+    │ @  8314addde180 2
+    │ ◉  a56846756248 0
     ├─╯
     ◉  000000000000
     "###);
@@ -436,9 +436,9 @@ fn test_parallelize_multiple_roots() {
     @  3c90598481cd 3
     │ ◉  b96aa55582e5 2
     ├─╯
-    │ ◉  3178394e33e7 a
+    │ ◉  6d37472c632c a
     ├─╯
-    │ ◉  1d9a0895e7d6 1
+    │ ◉  dc0e5d6135ce 1
     ├─╯
     ◉  000000000000
     "###);
@@ -550,23 +550,23 @@ fn test_parallelize_complex_nonlinear_target() {
         &["parallelize", "description(0)::description(4)"],
     );
     insta::assert_snapshot!(stderr, @r###"
-    Working copy now at: yostqsxw d193f3b7 (empty) 3c
-    Parent commit      : rlvkpnrz cbb4e169 (empty) 0
+    Working copy now at: yostqsxw 59a216e5 (empty) 3c
+    Parent commit      : rlvkpnrz 745bea80 (empty) 0
     Parent commit      : mzvwutvl cb944786 (empty) 3
     "###);
     insta::assert_snapshot!(get_log_output_with_parents(&test_env, &workspace_path), @r###"
-    @    d193f3b72495 3c parents: 0 3
+    @    59a216e537c4 3c parents: 0 3
     ├─╮
     │ ◉  cb9447869bf0 3 parents:
-    │ │ ◉  80fbafb56917 2c parents: 0 2
+    │ │ ◉  248ce1ffd76b 2c parents: 0 2
     ╭───┤
     │ │ ◉  8f4b8ef68676 2 parents:
     │ ├─╯
-    │ │ ◉  1985e0427139 1c parents: 0 1
+    │ │ ◉  55c626d090e2 1c parents: 0 1
     ╭───┤
     │ │ ◉  82918d78c984 1 parents:
     │ ├─╯
-    ◉ │  cbb4e1692ef4 0 parents:
+    ◉ │  745bea8029c1 0 parents:
     ├─╯
     │ ◉  14ca4df576b3 4 parents:
     ├─╯

From 601a1c9b7728e49fffa4e778ff3624d23cd4a57b Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Tue, 16 Apr 2024 16:45:33 -0700
Subject: [PATCH 6/9] tests: avoid a future ancestor merge with root commit

I'm going to make some `jj parallelize` cases that currently error out
instead be successful. Some of the will result in ancestor merges with
the root commit. This patch updates those tests to avoid that.
---
 cli/tests/test_parallelize_command.rs | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/cli/tests/test_parallelize_command.rs b/cli/tests/test_parallelize_command.rs
index e3a09bb641..e16d1c7872 100644
--- a/cli/tests/test_parallelize_command.rs
+++ b/cli/tests/test_parallelize_command.rs
@@ -253,6 +253,7 @@ fn test_parallelize_head_is_a_merge() {
     let 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, &["commit", "-m=0"]);
     test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=1"]);
     test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=2"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "root()"]);
@@ -263,12 +264,13 @@ fn test_parallelize_head_is_a_merge() {
         &["new", "description(2)", "description(b)", "-m=merged-head"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @    1a8db14a8cf0 merged-head
+    @    f2087b66e475 merged-head
     ├─╮
-    │ ◉  401e43e9461f b
-    │ ◉  66ea2ab19a70 a
-    ◉ │  d826910d21fb 2
-    ◉ │  dc0e5d6135ce 1
+    │ ◉  5164ab888473 b
+    │ ◉  f16fe8ac5ce9 a
+    ◉ │  fe79412860e8 2
+    ◉ │  a915696cf0ad 1
+    ◉ │  a56846756248 0
     ├─╯
     ◉  000000000000
     "###);
@@ -284,6 +286,7 @@ fn test_parallelize_interior_target_is_a_merge() {
     let 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, &["commit", "-m=0"]);
     test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=1"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "root()", "-m=a"]);
     test_env.jj_cmd_ok(
@@ -292,11 +295,12 @@ fn test_parallelize_interior_target_is_a_merge() {
     );
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=3"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  299099c22761 3
-    ◉    0c4da981fc0a 2
+    @  a6321093e3d3 3
+    ◉    705c32f67ce1 2
     ├─╮
-    │ ◉  6d37472c632c a
-    ◉ │  dc0e5d6135ce 1
+    │ ◉  427890ea3f2b a
+    ◉ │  a915696cf0ad 1
+    ◉ │  a56846756248 0
     ├─╯
     ◉  000000000000
     "###);

From 57c14a6c6fe5f3b3fb116a3bda9f783abca1af77 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Thu, 18 Apr 2024 10:04:34 -0700
Subject: [PATCH 7/9] parallelize: include parents in template for all tests

Should make it easier to understand the graph shape when there are
lots of crossing lines.
---
 cli/tests/test_parallelize_command.rs | 289 +++++++++++++-------------
 1 file changed, 142 insertions(+), 147 deletions(-)

diff --git a/cli/tests/test_parallelize_command.rs b/cli/tests/test_parallelize_command.rs
index e16d1c7872..eb58700813 100644
--- a/cli/tests/test_parallelize_command.rs
+++ b/cli/tests/test_parallelize_command.rs
@@ -27,29 +27,29 @@ fn test_parallelize_no_descendants() {
     }
     test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=6"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  b911505e443e 6
-    ◉  2e00cb15c7b6 5
-    ◉  9df3c87db1a2 4
-    ◉  9f5b59fa4622 3
-    ◉  d826910d21fb 2
-    ◉  dc0e5d6135ce 1
-    ◉  000000000000
+    @  b911505e443e 6 parents: 5
+    ◉  2e00cb15c7b6 5 parents: 4
+    ◉  9df3c87db1a2 4 parents: 3
+    ◉  9f5b59fa4622 3 parents: 2
+    ◉  d826910d21fb 2 parents: 1
+    ◉  dc0e5d6135ce 1 parents:
+    ◉  000000000000 parents:
     "###);
 
     test_env.jj_cmd_ok(&workspace_path, &["parallelize", "description(1)::"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  6c7b60a45eb6 6
-    │ ◉  296f48966777 5
+    @  6c7b60a45eb6 6 parents:
+    │ ◉  296f48966777 5 parents:
     ├─╯
-    │ ◉  524062469789 4
+    │ ◉  524062469789 4 parents:
     ├─╯
-    │ ◉  a9334ecaa379 3
+    │ ◉  a9334ecaa379 3 parents:
     ├─╯
-    │ ◉  3a7b37ebe843 2
+    │ ◉  3a7b37ebe843 2 parents:
     ├─╯
-    │ ◉  dc0e5d6135ce 1
+    │ ◉  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -65,13 +65,13 @@ fn test_parallelize_with_descendants_simple() {
     }
     test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=6"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  b911505e443e 6
-    ◉  2e00cb15c7b6 5
-    ◉  9df3c87db1a2 4
-    ◉  9f5b59fa4622 3
-    ◉  d826910d21fb 2
-    ◉  dc0e5d6135ce 1
-    ◉  000000000000
+    @  b911505e443e 6 parents: 5
+    ◉  2e00cb15c7b6 5 parents: 4
+    ◉  9df3c87db1a2 4 parents: 3
+    ◉  9f5b59fa4622 3 parents: 2
+    ◉  d826910d21fb 2 parents: 1
+    ◉  dc0e5d6135ce 1 parents:
+    ◉  000000000000 parents:
     "###);
 
     test_env.jj_cmd_ok(
@@ -79,17 +79,17 @@ fn test_parallelize_with_descendants_simple() {
         &["parallelize", "description(1)::description(4)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  259d624373d7 6
-    ◉        60d419591c77 5
+    @  259d624373d7 6 parents: 5
+    ◉        60d419591c77 5 parents: 1 2 3 4
     ├─┬─┬─╮
-    │ │ │ ◉  524062469789 4
-    │ │ ◉ │  a9334ecaa379 3
+    │ │ │ ◉  524062469789 4 parents:
+    │ │ ◉ │  a9334ecaa379 3 parents:
     │ │ ├─╯
-    │ ◉ │  3a7b37ebe843 2
+    │ ◉ │  3a7b37ebe843 2 parents:
     │ ├─╯
-    ◉ │  dc0e5d6135ce 1
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -108,32 +108,32 @@ fn test_parallelize_where_interior_has_non_target_children() {
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(2)", "-m=2c"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(5)", "-m=6"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  d27ee705f7a9 6
-    ◉  2e00cb15c7b6 5
-    ◉  9df3c87db1a2 4
-    ◉  9f5b59fa4622 3
-    │ ◉  9c8865930f3c 2c
+    @  d27ee705f7a9 6 parents: 5
+    ◉  2e00cb15c7b6 5 parents: 4
+    ◉  9df3c87db1a2 4 parents: 3
+    ◉  9f5b59fa4622 3 parents: 2
+    │ ◉  9c8865930f3c 2c parents: 2
     ├─╯
-    ◉  d826910d21fb 2
-    ◉  dc0e5d6135ce 1
-    ◉  000000000000
+    ◉  d826910d21fb 2 parents: 1
+    ◉  dc0e5d6135ce 1 parents:
+    ◉  000000000000 parents:
     "###);
 
     test_env.jj_cmd_ok(&workspace_path, &["parallelize", "dc0::9df"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  a42de3959cae 6
-    ◉        d907c901bad0 5
+    @  a42de3959cae 6 parents: 5
+    ◉        d907c901bad0 5 parents: 1 2 3 4
     ├─┬─┬─╮
-    │ │ │ ◉  b8f977c12383 4
-    │ │ ◉ │  7be8374575b9 3
+    │ │ │ ◉  b8f977c12383 4 parents:
+    │ │ ◉ │  7be8374575b9 3 parents:
     │ │ ├─╯
-    │ │ │ ◉  2a4c3dab2a50 2c
+    │ │ │ ◉  2a4c3dab2a50 2c parents: 1 2
     ╭─┬───╯
-    │ ◉ │  96ce11389312 2
+    │ ◉ │  96ce11389312 2 parents:
     │ ├─╯
-    ◉ │  dc0e5d6135ce 1
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -149,29 +149,29 @@ fn test_parallelize_where_root_has_non_target_children() {
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(1)", "-m=1c"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(3)", "-m=4"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  7636b3f489f4 4
-    ◉  9f5b59fa4622 3
-    ◉  d826910d21fb 2
-    │ ◉  50e2ced81124 1c
+    @  7636b3f489f4 4 parents: 3
+    ◉  9f5b59fa4622 3 parents: 2
+    ◉  d826910d21fb 2 parents: 1
+    │ ◉  50e2ced81124 1c parents: 1
     ├─╯
-    ◉  dc0e5d6135ce 1
-    ◉  000000000000
+    ◉  dc0e5d6135ce 1 parents:
+    ◉  000000000000 parents:
     "###);
     test_env.jj_cmd_ok(
         &workspace_path,
         &["parallelize", "description(1)::description(3)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @      d024344469c3 4
+    @      d024344469c3 4 parents: 1 2 3
     ├─┬─╮
-    │ │ ◉  5bd049136a7c 3
-    │ ◉ │  60f737a5a4a7 2
+    │ │ ◉  5bd049136a7c 3 parents:
+    │ ◉ │  60f737a5a4a7 2 parents:
     │ ├─╯
-    │ │ ◉  50e2ced81124 1c
+    │ │ ◉  50e2ced81124 1c parents: 1
     ├───╯
-    ◉ │  dc0e5d6135ce 1
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -193,15 +193,15 @@ fn test_parallelize_with_merge_commit_child() {
     );
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(3)", "-m", "4"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  90a65779e2ec 4
-    ◉  9f5b59fa4622 3
-    │ ◉  a01c1fad8506 2a-c
+    @  90a65779e2ec 4 parents: 3
+    ◉  9f5b59fa4622 3 parents: 2
+    │ ◉  a01c1fad8506 2a-c parents: 2 a
     ╭─┤
-    │ ◉  1eb902150bb9 a
-    ◉ │  d826910d21fb 2
-    ◉ │  dc0e5d6135ce 1
+    │ ◉  1eb902150bb9 a parents:
+    ◉ │  d826910d21fb 2 parents: 1
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     // After this finishes, child-2a will have three parents: "1", "2", and "a".
@@ -210,18 +210,18 @@ fn test_parallelize_with_merge_commit_child() {
         &["parallelize", "description(1)::description(3)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @      6107429ab54b 4
+    @      6107429ab54b 4 parents: 1 2 3
     ├─┬─╮
-    │ │ ◉  a9334ecaa379 3
-    │ │ │ ◉  a386386b94bc 2a-c
+    │ │ ◉  a9334ecaa379 3 parents:
+    │ │ │ ◉  a386386b94bc 2a-c parents: 1 2 a
     ╭─┬───┤
-    │ │ │ ◉  1eb902150bb9 a
+    │ │ │ ◉  1eb902150bb9 a parents:
     │ │ ├─╯
-    │ ◉ │  3a7b37ebe843 2
+    │ ◉ │  3a7b37ebe843 2 parents:
     │ ├─╯
-    ◉ │  dc0e5d6135ce 1
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -236,10 +236,10 @@ fn test_parallelize_failure_disconnected_target_commits() {
     }
     test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=3"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  9f5b59fa4622 3
-    ◉  d826910d21fb 2
-    ◉  dc0e5d6135ce 1
-    ◉  000000000000
+    @  9f5b59fa4622 3 parents: 2
+    ◉  d826910d21fb 2 parents: 1
+    ◉  dc0e5d6135ce 1 parents:
+    ◉  000000000000 parents:
     "###);
 
     insta::assert_snapshot!(test_env.jj_cmd_failure(
@@ -264,15 +264,15 @@ fn test_parallelize_head_is_a_merge() {
         &["new", "description(2)", "description(b)", "-m=merged-head"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @    f2087b66e475 merged-head
+    @    f2087b66e475 merged-head parents: 2 b
     ├─╮
-    │ ◉  5164ab888473 b
-    │ ◉  f16fe8ac5ce9 a
-    ◉ │  fe79412860e8 2
-    ◉ │  a915696cf0ad 1
-    ◉ │  a56846756248 0
+    │ ◉  5164ab888473 b parents: a
+    │ ◉  f16fe8ac5ce9 a parents:
+    ◉ │  fe79412860e8 2 parents: 1
+    ◉ │  a915696cf0ad 1 parents: 0
+    ◉ │  a56846756248 0 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     insta::assert_snapshot!(test_env.jj_cmd_failure(&workspace_path,&["parallelize", "description(1)::"]),
@@ -295,14 +295,14 @@ fn test_parallelize_interior_target_is_a_merge() {
     );
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=3"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  a6321093e3d3 3
-    ◉    705c32f67ce1 2
+    @  a6321093e3d3 3 parents: 2
+    ◉    705c32f67ce1 2 parents: 1 a
     ├─╮
-    │ ◉  427890ea3f2b a
-    ◉ │  a915696cf0ad 1
-    ◉ │  a56846756248 0
+    │ ◉  427890ea3f2b a parents:
+    ◉ │  a915696cf0ad 1 parents: 0
+    ◉ │  a56846756248 0 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     insta::assert_snapshot!(test_env.jj_cmd_failure(&workspace_path,&["parallelize", "description(1)::"]),
@@ -325,14 +325,14 @@ fn test_parallelize_root_is_a_merge() {
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=2"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=3"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  9f66b50aa1f2 3
-    ◉  dd995ce87f21 2
-    ◉    4b4941342e06 1
+    @  9f66b50aa1f2 3 parents: 2
+    ◉  dd995ce87f21 2 parents: 1
+    ◉    4b4941342e06 1 parents: y x
     ├─╮
-    │ ◉  4035b23c8f72 x
-    ◉ │  f3ec359cf9ff y
+    │ ◉  4035b23c8f72 x parents:
+    ◉ │  f3ec359cf9ff y parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     test_env.jj_cmd_ok(
@@ -340,16 +340,16 @@ fn test_parallelize_root_is_a_merge() {
         &["parallelize", "description(1)::description(2)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @    d6df04b236b0 3
+    @    d6df04b236b0 3 parents: 1 2
     ├─╮
-    │ ◉    38945baf55f4 2
+    │ ◉    38945baf55f4 2 parents: y x
     │ ├─╮
-    ◉ │ │  4b4941342e06 1
+    ◉ │ │  4b4941342e06 1 parents: y x
     ╰─┬─╮
-      │ ◉  4035b23c8f72 x
-      ◉ │  f3ec359cf9ff y
+      │ ◉  4035b23c8f72 x parents:
+      ◉ │  f3ec359cf9ff y parents:
       ├─╯
-      ◉  000000000000
+      ◉  000000000000 parents:
     "###);
 }
 
@@ -362,21 +362,21 @@ fn test_parallelize_multiple_heads() {
     test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=1"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(0)", "-m=2"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  8314addde180 2
-    │ ◉  a915696cf0ad 1
+    @  8314addde180 2 parents: 0
+    │ ◉  a915696cf0ad 1 parents: 0
     ├─╯
-    ◉  a56846756248 0
-    ◉  000000000000
+    ◉  a56846756248 0 parents:
+    ◉  000000000000 parents:
     "###);
 
     test_env.jj_cmd_ok(&workspace_path, &["parallelize", "description(0)::"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  e84481c26195 2
-    │ ◉  2047527ade93 1
+    @  e84481c26195 2 parents:
+    │ ◉  2047527ade93 1 parents:
     ├─╯
-    │ ◉  a56846756248 0
+    │ ◉  a56846756248 0 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -392,11 +392,11 @@ fn test_parallelize_multiple_heads_with_and_without_children() {
     test_env.jj_cmd_ok(&workspace_path, &["describe", "-m=1"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "description(0)", "-m=2"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  8314addde180 2
-    │ ◉  a915696cf0ad 1
+    @  8314addde180 2 parents: 0
+    │ ◉  a915696cf0ad 1 parents: 0
     ├─╯
-    ◉  a56846756248 0
-    ◉  000000000000
+    ◉  a56846756248 0 parents:
+    ◉  000000000000 parents:
     "###);
 
     test_env.jj_cmd_ok(
@@ -404,11 +404,11 @@ fn test_parallelize_multiple_heads_with_and_without_children() {
         &["parallelize", "description(0)", "description(1)"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    ◉  2047527ade93 1
-    │ @  8314addde180 2
-    │ ◉  a56846756248 0
+    ◉  2047527ade93 1 parents:
+    │ @  8314addde180 2 parents: 0
+    │ ◉  a56846756248 0 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -425,26 +425,26 @@ fn test_parallelize_multiple_roots() {
     );
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=3"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  299099c22761 3
-    ◉    0c4da981fc0a 2
+    @  299099c22761 3 parents: 2
+    ◉    0c4da981fc0a 2 parents: 1 a
     ├─╮
-    │ ◉  6d37472c632c a
-    ◉ │  dc0e5d6135ce 1
+    │ ◉  6d37472c632c a parents:
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     // Succeeds because the roots have the same parents.
     test_env.jj_cmd_ok(&workspace_path, &["parallelize", "root().."]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  3c90598481cd 3
-    │ ◉  b96aa55582e5 2
+    @  3c90598481cd 3 parents:
+    │ ◉  b96aa55582e5 2 parents:
     ├─╯
-    │ ◉  6d37472c632c a
+    │ ◉  6d37472c632c a parents:
     ├─╯
-    │ ◉  dc0e5d6135ce 1
+    │ ◉  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -461,15 +461,15 @@ fn test_parallelize_failure_multiple_heads_with_different_children() {
     test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=b"]);
     test_env.jj_cmd_ok(&workspace_path, &["commit", "-m=c"]);
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @  9b5fa4b364d4
-    ◉  7b095ae9b21f c
-    ◉  5164ab888473 b
-    ◉  f16fe8ac5ce9 a
-    │ ◉  9f5b59fa4622 3
-    │ ◉  d826910d21fb 2
-    │ ◉  dc0e5d6135ce 1
+    @  9b5fa4b364d4 parents: c
+    ◉  7b095ae9b21f c parents: b
+    ◉  5164ab888473 b parents: a
+    ◉  f16fe8ac5ce9 a parents:
+    │ ◉  9f5b59fa4622 3 parents: 2
+    │ ◉  d826910d21fb 2 parents: 1
+    │ ◉  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     insta::assert_snapshot!(
@@ -500,14 +500,14 @@ fn test_parallelize_failure_multiple_roots_with_different_parents() {
         &["new", "description(2)", "description(b)", "-m=merged-head"],
     );
     insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
-    @    1a8db14a8cf0 merged-head
+    @    1a8db14a8cf0 merged-head parents: 2 b
     ├─╮
-    │ ◉  401e43e9461f b
-    │ ◉  66ea2ab19a70 a
-    ◉ │  d826910d21fb 2
-    ◉ │  dc0e5d6135ce 1
+    │ ◉  401e43e9461f b parents: a
+    │ ◉  66ea2ab19a70 a parents:
+    ◉ │  d826910d21fb 2 parents: 1
+    ◉ │  dc0e5d6135ce 1 parents:
     ├─╯
-    ◉  000000000000
+    ◉  000000000000 parents:
     "###);
 
     insta::assert_snapshot!(
@@ -532,7 +532,7 @@ fn test_parallelize_complex_nonlinear_target() {
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=1c", "description(1)"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=2c", "description(2)"]);
     test_env.jj_cmd_ok(&workspace_path, &["new", "-m=3c", "description(3)"]);
-    insta::assert_snapshot!(get_log_output_with_parents(&test_env, &workspace_path), @r###"
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
     @  b043eb81416c 3c parents: 3
     │ ◉    48277ee9afe0 4 parents: 3 2 1
     ╭─┼─╮
@@ -558,7 +558,7 @@ fn test_parallelize_complex_nonlinear_target() {
     Parent commit      : rlvkpnrz 745bea80 (empty) 0
     Parent commit      : mzvwutvl cb944786 (empty) 3
     "###);
-    insta::assert_snapshot!(get_log_output_with_parents(&test_env, &workspace_path), @r###"
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
     @    59a216e537c4 3c parents: 0 3
     ├─╮
     │ ◉  cb9447869bf0 3 parents:
@@ -578,7 +578,7 @@ fn test_parallelize_complex_nonlinear_target() {
     "###)
 }
 
-fn get_log_output_with_parents(test_env: &TestEnvironment, cwd: &Path) -> String {
+fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
     let template = r#"
     separate(" ",
         commit_id.short(),
@@ -588,8 +588,3 @@ fn get_log_output_with_parents(test_env: &TestEnvironment, cwd: &Path) -> String
     )"#;
     test_env.jj_cmd_success(cwd, &["log", "-T", template])
 }
-
-fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {
-    let template = r#"separate(" ", commit_id.short(), local_branches, description)"#;
-    test_env.jj_cmd_success(cwd, &["log", "-T", template])
-}

From 02bcb50224c27f7c87b6ec5059800c084152f54a Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Tue, 16 Apr 2024 21:32:56 -0700
Subject: [PATCH 8/9] parallelize: make the command pass in more cases

The checks are not needed by the new implementation, so just drop
them.
---
 cli/src/commands/parallelize.rs       | 147 +++-----------------------
 cli/tests/cli-reference@.md.snap      |  24 ++---
 cli/tests/test_parallelize_command.rs |  90 ++++++++++++----
 3 files changed, 95 insertions(+), 166 deletions(-)

diff --git a/cli/src/commands/parallelize.rs b/cli/src/commands/parallelize.rs
index 0de3615d04..3a2b4c5953 100644
--- a/cli/src/commands/parallelize.rs
+++ b/cli/src/commands/parallelize.rs
@@ -12,20 +12,17 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashMap;
 use std::io::Write;
-use std::rc::Rc;
 
 use indexmap::IndexSet;
 use itertools::Itertools;
 use jj_lib::backend::CommitId;
 use jj_lib::commit::{Commit, CommitIteratorExt};
-use jj_lib::repo::Repo;
-use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
 use tracing::instrument;
 
 use crate::cli_util::{CommandHelper, RevisionArg};
-use crate::command_error::{user_error, CommandError};
+use crate::command_error::CommandError;
 use crate::ui::Ui;
 
 /// Parallelize revisions by making them siblings
@@ -41,18 +38,18 @@ use crate::ui::Ui;
 /// 0
 /// ```
 ///
-/// Each of the target revisions is rebased onto the parents of the root(s) of
-/// the target revset (not to be confused with the repo root). The children of
-/// the head(s) of the target revset are rebased onto the target revisions.
+/// The command effectively says "these revisions are actually independent",
+/// meaning that they should no longer be ancestors/descendants of each other.
+/// However, revisions outside the set that were previously ancestors of a
+/// revision in the set will remain ancestors of it. For example, revision 0
+/// above remains an ancestor of both 1 and 2. Similarly,
+/// revisions outside the set that were previously descendants of a revision
+/// in the set will remain descendants of it. For example, revision 3 above
+/// remains a descendant of both 1 and 2.
 ///
-/// The target revset is the union of the `revisions` arguments and must satisfy
-/// several conditions, otherwise the command will fail.
-///
-/// 1. The heads of the target revset must have either the same children as the
-///    other heads or none.
-/// 2. The roots of the target revset have the same parents.
-/// 3. The parents of all target revisions except the roots must also be
-///    parallelized. This means that the target revisions must be connected.
+/// Therefore, `jj parallelize '1 | 3'` is a no-op. That's because 2, which is
+/// not in the target set, was a descendant of 1 before, so it remains a
+/// descendant, and it was an ancestor of 3 before, so it remains an ancestor.
 #[derive(clap::Args, Clone, Debug)]
 #[command(verbatim_doc_comment)]
 pub(crate) struct ParallelizeArgs {
@@ -80,11 +77,6 @@ pub(crate) fn cmd_parallelize(
     workspace_command.check_rewritable(target_commits.iter().ids())?;
 
     let mut tx = workspace_command.start_transaction();
-    let target_revset =
-        RevsetExpression::commits(target_commits.iter().ids().cloned().collect_vec());
-    // TODO: The checks are now unnecessary, so drop them
-    let _new_parents =
-        check_preconditions_and_get_new_parents(&target_revset, &target_commits, tx.repo())?;
 
     // New parents for commits in the target set. Since commits in the set are now
     // supposed to be independent, they inherit the parent's non-target parents,
@@ -151,116 +143,3 @@ pub(crate) fn cmd_parallelize(
 
     tx.finish(ui, format!("parallelize {} commits", target_commits.len()))
 }
-
-/// Returns the new parents of the parallelized commits or an error if any of
-/// the following preconditions are not met:
-///
-/// 1. If the heads of the target revset must not have different children.
-/// 2. The roots of the target revset must not have different parents.
-/// 3. The parents of all target revisions except the roots must also be
-///    parallelized. This means that the target revisions must be connected.
-///
-/// The `target_revset` must evaluate to the commits in `target_commits` when
-/// the provided `repo` is used.
-fn check_preconditions_and_get_new_parents(
-    target_revset: &Rc<RevsetExpression>,
-    target_commits: &[Commit],
-    repo: &dyn Repo,
-) -> Result<Vec<Commit>, CommandError> {
-    check_target_heads(target_revset, repo)?;
-    let target_roots = check_target_roots(target_revset, repo)?;
-    check_target_commits_are_connected(&target_roots, target_commits)?;
-
-    // We already verified that the roots have the same parents, so we can just
-    // use the first root.
-    Ok(target_roots[0].parents())
-}
-
-/// Returns an error if the heads of the target revset have children which are
-/// different.
-fn check_target_heads(
-    target_revset: &Rc<RevsetExpression>,
-    repo: &dyn Repo,
-) -> Result<(), CommandError> {
-    let target_heads = target_revset
-        .heads()
-        .evaluate_programmatic(repo)?
-        .iter()
-        .sorted()
-        .collect_vec();
-    if target_heads.len() == 1 {
-        return Ok(());
-    }
-    let all_children: Vec<Commit> = target_revset
-        .heads()
-        .children()
-        .evaluate_programmatic(repo)?
-        .iter()
-        .commits(repo.store())
-        .try_collect()?;
-    // Every child must have every target head as a parent, otherwise it means
-    // that the target heads have different children.
-    for child in all_children {
-        let parents = child.parent_ids().iter().sorted();
-        if !parents.eq(target_heads.iter()) {
-            return Err(user_error(
-                "All heads of the target revisions must have the same children.",
-            ));
-        }
-    }
-    Ok(())
-}
-
-/// Returns the roots of the target revset or an error if their parents are
-/// different.
-fn check_target_roots(
-    target_revset: &Rc<RevsetExpression>,
-    repo: &dyn Repo,
-) -> Result<Vec<Commit>, CommandError> {
-    let target_roots: Vec<Commit> = target_revset
-        .roots()
-        .evaluate_programmatic(repo)?
-        .iter()
-        .commits(repo.store())
-        .try_collect()?;
-    let expected_parents = target_roots[0].parent_ids().iter().sorted().collect_vec();
-    for root in target_roots[1..].iter() {
-        let root_parents = root.parent_ids().iter().sorted();
-        if !root_parents.eq(expected_parents.iter().copied()) {
-            return Err(user_error(
-                "All roots of the target revisions must have the same parents.",
-            ));
-        }
-    }
-    Ok(target_roots)
-}
-
-/// The target commits must be connected. The parents of every target commit
-/// except the root commit must also be target commits. Returns an error if this
-/// requirement is not met.
-fn check_target_commits_are_connected(
-    target_roots: &[Commit],
-    target_commits: &[Commit],
-) -> Result<(), CommandError> {
-    let target_commit_ids: HashSet<CommitId> = target_commits.iter().ids().cloned().collect();
-    for target_commit in target_commits.iter() {
-        if target_roots.iter().ids().contains(target_commit.id()) {
-            continue;
-        }
-        for parent in target_commit.parent_ids() {
-            if !target_commit_ids.contains(parent) {
-                // We check this condition to return a more useful error to the user.
-                if target_commit.parent_ids().len() == 1 {
-                    return Err(user_error(
-                        "Cannot parallelize since the target revisions are not connected.",
-                    ));
-                }
-                return Err(user_error(
-                    "Only the roots of the target revset are allowed to have parents which are \
-                     not being parallelized.",
-                ));
-            }
-        }
-    }
-    Ok(())
-}
diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap
index c3a50add63..4dd0ba62cb 100644
--- a/cli/tests/cli-reference@.md.snap
+++ b/cli/tests/cli-reference@.md.snap
@@ -1369,18 +1369,18 @@ Running `jj parallelize 1::2` will transform the history like this:
 0
 ```
 
-Each of the target revisions is rebased onto the parents of the root(s) of
-the target revset (not to be confused with the repo root). The children of
-the head(s) of the target revset are rebased onto the target revisions.
-
-The target revset is the union of the `revisions` arguments and must satisfy
-several conditions, otherwise the command will fail.
-
-1. The heads of the target revset must have either the same children as the
-   other heads or none.
-2. The roots of the target revset have the same parents.
-3. The parents of all target revisions except the roots must also be
-   parallelized. This means that the target revisions must be connected.
+The command effectively says "these revisions are actually independent",
+meaning that they should no longer be ancestors/descendants of each other.
+However, revisions outside the set that were previously ancestors of a
+revision in the set will remain ancestors of it. For example, revision 0
+above remains an ancestor of both 1 and 2. Similarly,
+revisions outside the set that were previously descendants of a revision
+in the set will remain descendants of it. For example, revision 3 above
+remains a descendant of both 1 and 2.
+
+Therefore, `jj parallelize '1 | 3'` is a no-op. That's because 2, which is
+not in the target set, was a descendant of 1 before, so it remains a
+descendant, and it was an ancestor of 3 before, so it remains an ancestor.
 
 **Usage:** `jj parallelize [REVISIONS]...`
 
diff --git a/cli/tests/test_parallelize_command.rs b/cli/tests/test_parallelize_command.rs
index eb58700813..ea43756cfb 100644
--- a/cli/tests/test_parallelize_command.rs
+++ b/cli/tests/test_parallelize_command.rs
@@ -226,7 +226,7 @@ fn test_parallelize_with_merge_commit_child() {
 }
 
 #[test]
-fn test_parallelize_failure_disconnected_target_commits() {
+fn test_parallelize_disconnected_target_commits() {
     let 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");
@@ -242,9 +242,19 @@ fn test_parallelize_failure_disconnected_target_commits() {
     ◉  000000000000 parents:
     "###);
 
-    insta::assert_snapshot!(test_env.jj_cmd_failure(
-        &workspace_path, &["parallelize", "description(1)", "description(3)"]),@r###"
-    Error: Cannot parallelize since the target revisions are not connected.
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &workspace_path,
+        &["parallelize", "description(1)", "description(3)"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Nothing changed.
+    "###);
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @  9f5b59fa4622 3 parents: 2
+    ◉  d826910d21fb 2 parents: 1
+    ◉  dc0e5d6135ce 1 parents:
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -275,9 +285,19 @@ fn test_parallelize_head_is_a_merge() {
     ◉  000000000000 parents:
     "###);
 
-    insta::assert_snapshot!(test_env.jj_cmd_failure(&workspace_path,&["parallelize", "description(1)::"]),
-        @r###"
-    Error: Only the roots of the target revset are allowed to have parents which are not being parallelized.
+    test_env.jj_cmd_ok(&workspace_path, &["parallelize", "description(1)::"]);
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @    babb4191912d merged-head parents: 0 b
+    ├─╮
+    │ ◉  5164ab888473 b parents: a
+    │ ◉  f16fe8ac5ce9 a parents:
+    │ │ ◉  36b2f866a798 2 parents: 0
+    ├───╯
+    │ │ ◉  a915696cf0ad 1 parents: 0
+    ├───╯
+    ◉ │  a56846756248 0 parents:
+    ├─╯
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -305,9 +325,18 @@ fn test_parallelize_interior_target_is_a_merge() {
     ◉  000000000000 parents:
     "###);
 
-    insta::assert_snapshot!(test_env.jj_cmd_failure(&workspace_path,&["parallelize", "description(1)::"]),
-        @r###"
-    Error: Only the roots of the target revset are allowed to have parents which are not being parallelized.
+    test_env.jj_cmd_ok(&workspace_path, &["parallelize", "description(1)::"]);
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @    cd0ac6ad1415 3 parents: 0 a
+    ├─╮
+    │ │ ◉  1c240e875670 2 parents: 0 a
+    ╭─┬─╯
+    │ ◉  427890ea3f2b a parents:
+    │ │ ◉  a915696cf0ad 1 parents: 0
+    ├───╯
+    ◉ │  a56846756248 0 parents:
+    ├─╯
+    ◉  000000000000 parents:
     "###);
 }
 
@@ -449,7 +478,7 @@ fn test_parallelize_multiple_roots() {
 }
 
 #[test]
-fn test_parallelize_failure_multiple_heads_with_different_children() {
+fn test_parallelize_multiple_heads_with_different_children() {
     let 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");
@@ -472,21 +501,33 @@ fn test_parallelize_failure_multiple_heads_with_different_children() {
     ◉  000000000000 parents:
     "###);
 
-    insta::assert_snapshot!(
-    test_env.jj_cmd_failure(
+    test_env.jj_cmd_ok(
         &workspace_path,
         &[
             "parallelize",
             "description(1)::description(2)",
             "description(a)::description(b)",
         ],
-    ),@r###"
-    Error: All heads of the target revisions must have the same children.
+    );
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @  582c6bd1e1fd parents: c
+    ◉    dd2db8b60a69 c parents: a b
+    ├─╮
+    │ ◉  190b857f6cdd b parents:
+    ◉ │  f16fe8ac5ce9 a parents:
+    ├─╯
+    │ ◉    bbc313370f45 3 parents: 1 2
+    │ ├─╮
+    │ │ ◉  96ce11389312 2 parents:
+    ├───╯
+    │ ◉  dc0e5d6135ce 1 parents:
+    ├─╯
+    ◉  000000000000 parents:
     "###);
 }
 
 #[test]
-fn test_parallelize_failure_multiple_roots_with_different_parents() {
+fn test_parallelize_multiple_roots_with_different_parents() {
     let 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");
@@ -510,12 +551,21 @@ fn test_parallelize_failure_multiple_roots_with_different_parents() {
     ◉  000000000000 parents:
     "###);
 
-    insta::assert_snapshot!(
-    test_env.jj_cmd_failure(
+    test_env.jj_cmd_ok(
         &workspace_path,
         &["parallelize", "description(2)::", "description(b)::"],
-    ),@r###"
-    Error: All roots of the target revisions must have the same parents.
+    );
+    insta::assert_snapshot!(get_log_output(&test_env, &workspace_path), @r###"
+    @    4224f9c9e598 merged-head parents: 1 a
+    ├─╮
+    │ │ ◉  401e43e9461f b parents: a
+    │ ├─╯
+    │ ◉  66ea2ab19a70 a parents:
+    │ │ ◉  d826910d21fb 2 parents: 1
+    ├───╯
+    ◉ │  dc0e5d6135ce 1 parents:
+    ├─╯
+    ◉  000000000000 parents:
     "###);
 }
 

From fa6b01da966eb93c6da783f02b9ffc5e44c32dda Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Wed, 17 Apr 2024 21:55:50 -0700
Subject: [PATCH 9/9] parallelize: drop redundant "Nothing changed." case

The rewritten code is already a no-op when there's a single input. I
don't think the case is common enough to warrant having a special case
for performance reasons either. Also, by not having the special case,
`jj parallelize <immutable commit>` fails consistently with the
non-singleton case.
---
 cli/src/commands/parallelize.rs | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/cli/src/commands/parallelize.rs b/cli/src/commands/parallelize.rs
index 3a2b4c5953..083a36e030 100644
--- a/cli/src/commands/parallelize.rs
+++ b/cli/src/commands/parallelize.rs
@@ -13,7 +13,6 @@
 // limitations under the License.
 
 use std::collections::HashMap;
-use std::io::Write;
 
 use indexmap::IndexSet;
 use itertools::Itertools;
@@ -70,10 +69,6 @@ pub(crate) fn cmd_parallelize(
         .parse_union_revsets(&args.revisions)?
         .evaluate_to_commits()?
         .try_collect()?;
-    if target_commits.len() < 2 {
-        writeln!(ui.status(), "Nothing changed.")?;
-        return Ok(());
-    }
     workspace_command.check_rewritable(target_commits.iter().ids())?;
 
     let mut tx = workspace_command.start_transaction();