From 2ac431c5585f1a5ebcc0abde217823b7db123752 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Fri, 29 Mar 2024 21:32:55 +0800 Subject: [PATCH] rebase: add `--insert-after` and `--insert-before` options --- cli/src/commands/rebase.rs | 326 +++++++++++++++++++++++++++++-- cli/tests/cli-reference@.md.snap | 4 +- cli/tests/test_rebase_command.rs | 310 ++++++++++++++++++++++++++++- 3 files changed, 622 insertions(+), 18 deletions(-) diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index 248ecfefc43..6c1c9cf8ec4 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -15,6 +15,7 @@ use std::borrow::Borrow; use std::collections::HashMap; use std::io::Write; +use std::rc::Rc; use std::sync::Arc; use clap::ArgGroup; @@ -125,6 +126,7 @@ use crate::ui::Ui; #[derive(clap::Args, Clone, Debug)] #[command(verbatim_doc_comment)] #[command(group(ArgGroup::new("to_rebase").args(&["branch", "source", "revision"])))] +#[command(group(ArgGroup::new("dest").args(&["destination", "insert_after", "insert_before"]).required(true)))] pub(crate) struct RebaseArgs { /// Rebase the whole branch relative to destination's ancestors (can be /// repeated) @@ -157,8 +159,32 @@ pub(crate) struct RebaseArgs { revision: Option, /// The revision(s) to rebase onto (can be repeated to create a merge /// commit) - #[arg(long, short, required = true)] + #[arg(long, short)] destination: Vec, + /// The revision(s) to insert after (can be repeated to create a merge + /// commit) + /// + /// Only works with `-r`. + #[arg( + long, + short = 'A', + visible_alias = "after", + conflicts_with = "source", + conflicts_with = "branch" + )] + insert_after: Vec, + /// The revision(s) to insert before (can be repeated to create a merge + /// commit) + /// + /// Only works with `-r`. + #[arg( + long, + short = 'B', + visible_alias = "before", + conflicts_with = "source", + conflicts_with = "branch" + )] + insert_before: Vec, /// If true, when rebasing would produce an empty commit, the commit is /// abandoned. It will not be abandoned if it was already empty before the @@ -193,10 +219,14 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets simplify_ancestor_merge: false, }; let mut workspace_command = command.workspace_helper(ui)?; - let new_parents = workspace_command - .resolve_some_revsets_default_single(&args.destination)? - .into_iter() - .collect_vec(); + let new_parents = if args.destination.is_empty() { + vec![] + } else { + workspace_command + .resolve_some_revsets_default_single(&args.destination)? + .into_iter() + .collect_vec() + }; if let Some(rev_arg) = &args.revision { assert_eq!( // In principle, `-r --skip-empty` could mean to abandon the `-r` @@ -213,13 +243,35 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets EmptyBehaviour::Keep, "clap should forbid `-r --skip-empty`" ); - rebase_revision( - ui, - command.settings(), - &mut workspace_command, - &new_parents, - rev_arg, - )?; + if !args.insert_after.is_empty() { + let after_commits = + workspace_command.resolve_some_revsets_default_single(&args.insert_after)?; + rebase_revision_after( + ui, + command.settings(), + &mut workspace_command, + &after_commits, + rev_arg, + )?; + } else if !args.insert_before.is_empty() { + let before_commits = + workspace_command.resolve_some_revsets_default_single(&args.insert_before)?; + rebase_revision_before( + ui, + command.settings(), + &mut workspace_command, + &before_commits, + rev_arg, + )?; + } else { + rebase_revision( + ui, + command.settings(), + &mut workspace_command, + &new_parents, + rev_arg, + )?; + } } else if !args.source.is_empty() { let source_commits = workspace_command.resolve_some_revsets_default_single(&args.source)?; rebase_descendants_transaction( @@ -433,6 +485,85 @@ fn rebase_revision( } } +fn rebase_revision_after( + ui: &mut Ui, + settings: &UserSettings, + workspace_command: &mut WorkspaceCommandHelper, + after_commits: &IndexSet, + rev_arg: &RevisionArg, +) -> Result<(), CommandError> { + let old_commit = workspace_command.resolve_single_rev(rev_arg)?; + workspace_command.check_rewritable([&old_commit])?; + + let after_commit_ids = after_commits.iter().map(|c| c.id().clone()).collect_vec(); + let new_parents_expression = RevsetExpression::commits(after_commit_ids); + let new_children_expression = new_parents_expression.children(); + + ensure_no_commit_loop( + workspace_command.repo().as_ref(), + &new_children_expression, + &new_parents_expression, + )?; + + let new_parents = after_commits; + let new_children: IndexSet = new_children_expression + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + workspace_command.check_rewritable(&new_children)?; + + move_commit( + ui, + settings, + workspace_command, + new_parents, + &new_children, + old_commit, + ) +} + +fn rebase_revision_before( + ui: &mut Ui, + settings: &UserSettings, + workspace_command: &mut WorkspaceCommandHelper, + before_commits: &IndexSet, + rev_arg: &RevisionArg, +) -> Result<(), CommandError> { + let old_commit = workspace_command.resolve_single_rev(rev_arg)?; + workspace_command.check_rewritable([&old_commit])?; + workspace_command.check_rewritable(before_commits.iter())?; + + let before_commit_ids = before_commits.iter().map(|c| c.id().clone()).collect_vec(); + let new_children_expression = RevsetExpression::commits(before_commit_ids); + let new_parents_expression = new_children_expression.parents(); + + ensure_no_commit_loop( + workspace_command.repo().as_ref(), + &new_children_expression, + &new_parents_expression, + )?; + + let new_parents: IndexSet = new_parents_expression + // Exclude parents that are ancestors of each other. + .minus(&new_parents_expression.parents().ancestors()) + .clone() + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + let new_children = before_commits; + + move_commit( + ui, + settings, + workspace_command, + &new_parents, + new_children, + old_commit, + ) +} + /// Extracts `commit` from the graph by rebasing its children on its parents. /// This assumes that it can be rewritten. fn extract_commit( @@ -491,6 +622,155 @@ fn extract_commit( rebase_onto_new_parents(tx, settings, &commits_to_rebase) } +/// Moves `commit` from its current location to a new location, given by the set +/// of `new_parents` and `new_children_with_parents`. +fn move_commit( + ui: &mut Ui, + settings: &UserSettings, + workspace_command: &mut WorkspaceCommandHelper, + new_parents: &IndexSet, + new_children: &IndexSet, + commit: Commit, +) -> Result<(), CommandError> { + let (new_parents, new_children_with_parents) = move_revision_compute_parent_children( + workspace_command, + &commit, + new_parents, + new_children, + )?; + + let mut tx = workspace_command.start_transaction(); + // Extract `commit` from its previous location by rebasing its children + // onto its parents. + let (mut rebased_commit_ids, _) = extract_commit(&mut tx, settings, &commit)?; + + // We now update `new_parents` to account for the rebase of all of + // `commit`'s descendants. Even if some of the original `new_parents` + // were descendants of `old_commit`, this will no longer be the case after + // the update. + // + // See comment in `rebase_revision` for the bug in this code. + let new_parents: Vec<_> = new_parents + .iter() + .map(|new_parent| get_possibly_rewritten_commit(&mut tx, &rebased_commit_ids, new_parent)) + .try_collect()?; + + // Finally, it's safe to rebase `commit`. We can skip rebasing if it is + // already a child of `new_parents`. Otherwise, at this point, it should no + // longer have any children; they have all been rebased and the originals + // have been abandoned. + if commit.parents() == new_parents { + write!(ui.stderr(), "Skipping rebase of commit ")?; + tx.write_commit_summary(ui.stderr_formatter().as_mut(), &commit)?; + writeln!(ui.stderr())?; + } else { + let new_commit = rebase_commit(settings, tx.mut_repo(), &commit, &new_parents)?; + rebased_commit_ids.insert(commit.id().clone(), new_commit.id().clone()); + }; + + // Now, rebase all the new children onto the newly rebased commit. + // TODO: Make this no-op if the new children already have the required parents. + let new_children_with_parents: Vec<_> = new_children_with_parents + .iter() + .map(|(commit, parents)| { + // Both the original child commit and its parents could have been rewritten. + let commit = get_possibly_rewritten_commit(&mut tx, &rebased_commit_ids, commit)?; + let new_parents: Vec<_> = parents + .iter() + .map(|parent| get_possibly_rewritten_commit(&mut tx, &rebased_commit_ids, parent)) + .try_collect()?; + Ok::<_, BackendError>((commit, new_parents)) + }) + .try_collect()?; + + let (new_rebased_commit_ids, _) = + rebase_onto_new_parents(&mut tx, settings, &new_children_with_parents)?; + + let num_rebased_commits = rebased_commit_ids + .iter() + .filter(|(_, new_commit_id)| !new_rebased_commit_ids.contains_key(new_commit_id)) + .count() + + new_rebased_commit_ids.len(); + + if num_rebased_commits > 0 { + writeln!(ui.stderr(), "Rebased {num_rebased_commits} commits")?; + } + if tx.mut_repo().has_changes() { + tx.finish(ui, format!("move commit {}", commit.id().hex())) + } else { + Ok(()) // Do not print "Nothing changed." + } +} + +type NewParentsAndChildren = (Vec, Vec<(Commit, Vec)>); + +fn move_revision_compute_parent_children( + workspace_command: &mut WorkspaceCommandHelper, + old_commit: &Commit, + new_parents_set: &IndexSet, + new_children_set: &IndexSet, +) -> Result { + // If the new parents include the old commit, replace it with the old commit's + // parents. + // e.g. `jj rebase -r A --before A` + let new_parents: Vec<_> = new_parents_set + .iter() + .flat_map(|c| { + if c == old_commit { + old_commit.parents().clone() + } else { + [c.clone()].to_vec() + } + }) + .collect(); + + let old_commit_expression = RevsetExpression::commit(old_commit.id().clone()); + let old_commit_children: Vec = old_commit_expression + .children() + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + + // If the new children include the old commit, replace it with the old commit's + // children. + // e.g. `jj rebase -r A --after A` + let new_children: Vec<_> = new_children_set + .iter() + .flat_map(|c| { + if c == old_commit { + old_commit_children.clone() + } else { + [c.clone()].to_vec() + } + }) + .collect(); + + let new_children_to_rebase: Vec<_> = new_children + .into_iter() + .map(|child_commit| { + // Exclude any of the new parents, since we are "inserting" the new commit + // in between the new parents and the new children. + let mut new_child_parents: Vec<_> = child_commit + .parent_ids() + .iter() + .filter(|&id| !new_parents_set.iter().any(|commit| commit.id() == id)) + .map(|id| workspace_command.repo().store().get_commit(id)) + .try_collect()?; + + // Add the commit as a parent of the new child commit if it is not already + // a parent. + if !new_child_parents.iter().any(|c| c == old_commit) { + new_child_parents.push(old_commit.clone()); + } + + Ok::<_, BackendError>((child_commit, new_child_parents)) + }) + .try_collect()?; + + Ok((new_parents, new_children_to_rebase)) +} + /// Rebases each commit in `commits_to_rebase` onto its new parents, and calls /// `rebase_descendants` after. /// This assumes that each commit can be rewritten. @@ -534,6 +814,28 @@ fn get_possibly_rewritten_commit( }) } +/// Ensure that there is no possible cycle between the potential children and +/// parents of a rebased commit. +fn ensure_no_commit_loop( + repo: &ReadonlyRepo, + children_expression: &Rc, + parents_expression: &Rc, +) -> Result<(), CommandError> { + if let Some(commit_id) = children_expression + .dag_range_to(parents_expression) + .evaluate_programmatic(repo)? + .iter() + .next() + { + return Err(user_error(format!( + "Refusing to create a loop: commit {} would be both an ancestor and a descendant of \ + the rebased commit", + short_commit_hash(&commit_id), + ))); + } + Ok(()) +} + fn check_rebase_destinations( repo: &Arc, new_parents: &[Commit], diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 70a936cece9..67df1d2936c 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1481,7 +1481,7 @@ J J If a working-copy commit gets abandoned, it will be given a new, empty commit. This is true in general; it is not specific to this command. -**Usage:** `jj rebase [OPTIONS] --destination ` +**Usage:** `jj rebase [OPTIONS] <--destination |--insert-after |--insert-before >` ###### **Options:** @@ -1489,6 +1489,8 @@ commit. This is true in general; it is not specific to this command. * `-s`, `--source ` — Rebase specified revision(s) together with their trees of descendants (can be repeated) * `-r`, `--revision ` — Rebase only this revision, rebasing descendants onto this revision's parent(s) * `-d`, `--destination ` — The revision(s) to rebase onto (can be repeated to create a merge commit) +* `-A`, `--insert-after ` — The revision(s) to insert after (can be repeated to create a merge commit) +* `-B`, `--insert-before ` — The revision(s) to insert before (can be repeated to create a merge commit) * `--skip-empty` — If true, when rebasing would produce an empty commit, the commit is abandoned. It will not be abandoned if it was already empty before the rebase. Will never skip merge commits with multiple non-empty parents Possible values: `true`, `false` diff --git a/cli/tests/test_rebase_command.rs b/cli/tests/test_rebase_command.rs index fec0f235bc2..7116cc48746 100644 --- a/cli/tests/test_rebase_command.rs +++ b/cli/tests/test_rebase_command.rs @@ -41,9 +41,9 @@ fn test_rebase_invalid() { let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase"]); insta::assert_snapshot!(stderr, @r###" error: the following required arguments were not provided: - --destination + <--destination |--insert-after |--insert-before > - Usage: jj rebase --destination + Usage: jj rebase <--destination |--insert-after |--insert-before > For more information, try '--help'. "###); @@ -54,7 +54,7 @@ fn test_rebase_invalid() { insta::assert_snapshot!(stderr, @r###" error: the argument '--revision ' cannot be used with '--source ' - Usage: jj rebase --destination --revision + Usage: jj rebase --revision <--destination |--insert-after |--insert-before > For more information, try '--help'. "###); @@ -65,7 +65,7 @@ fn test_rebase_invalid() { insta::assert_snapshot!(stderr, @r###" error: the argument '--branch ' cannot be used with '--source ' - Usage: jj rebase --destination --branch + Usage: jj rebase --branch <--destination |--insert-after |--insert-before > For more information, try '--help'. "###); @@ -78,7 +78,73 @@ fn test_rebase_invalid() { insta::assert_snapshot!(stderr, @r###" error: the argument '--revision ' cannot be used with '--skip-empty' - Usage: jj rebase --destination --revision + Usage: jj rebase --revision <--destination |--insert-after |--insert-before > + + For more information, try '--help'. + "###); + + // Both -d and --after + let stderr = test_env.jj_cmd_cli_error( + &repo_path, + &["rebase", "-r", "a", "-d", "b", "--after", "b"], + ); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--destination ' cannot be used with '--insert-after ' + + Usage: jj rebase --revision <--destination |--insert-after |--insert-before > + + For more information, try '--help'. + "###); + + // -s with --after + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-s", "a", "--after", "b"]); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--source ' cannot be used with '--insert-after ' + + Usage: jj rebase --source <--destination |--insert-after |--insert-before > + + For more information, try '--help'. + "###); + + // -b with --after + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-b", "a", "--after", "b"]); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--branch ' cannot be used with '--insert-after ' + + Usage: jj rebase --branch <--destination |--insert-after |--insert-before > + + For more information, try '--help'. + "###); + + // Both -d and --before + let stderr = test_env.jj_cmd_cli_error( + &repo_path, + &["rebase", "-r", "a", "-d", "b", "--before", "b"], + ); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--destination ' cannot be used with '--insert-before ' + + Usage: jj rebase --revision <--destination |--insert-after |--insert-before > + + For more information, try '--help'. + "###); + + // -s with --before + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-s", "a", "--before", "b"]); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--source ' cannot be used with '--insert-before ' + + Usage: jj rebase --source <--destination |--insert-after |--insert-before > + + For more information, try '--help'. + "###); + + // -b with --before + let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-b", "a", "--before", "b"]); + insta::assert_snapshot!(stderr, @r###" + error: the argument '--branch ' cannot be used with '--insert-before ' + + Usage: jj rebase --branch <--destination |--insert-after |--insert-before > For more information, try '--help'. "###); @@ -1055,6 +1121,240 @@ fn test_rebase_with_child_and_descendant_bug_2600() { "###); } +#[test] +fn test_rebase_revision_after() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + create_commit(&test_env, &repo_path, "a", &[]); + create_commit(&test_env, &repo_path, "b1", &["a"]); + create_commit(&test_env, &repo_path, "b2", &["a"]); + create_commit(&test_env, &repo_path, "c", &["b1", "b2"]); + create_commit(&test_env, &repo_path, "d", &["c"]); + create_commit(&test_env, &repo_path, "e", &["c"]); + create_commit(&test_env, &repo_path, "f", &["e"]); + // Test the setup + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + @ f lylxulpl 88f778c5 + ◉ e kmkuslsw 48dd9e3f + │ ◉ d znkkpsqq 92438fc9 + ├─╯ + ◉ c vruxwmqv c41e416e + ├─╮ + │ ◉ b2 royxmykx 903ab0d6 + ◉ │ b1 zsuskuln 072d5ae1 + ├─╯ + ◉ a rlvkpnrz 2443ea76 + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "c", "--after", "e"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 4 commits + Working copy now at: lylxulpl 41d72a5b f | f + Parent commit : vruxwmqv fe35083c c | c + "###); + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + @ f lylxulpl 41d72a5b + ◉ c vruxwmqv fe35083c + ◉ e kmkuslsw 1f75714f + ├─╮ + │ │ ◉ d znkkpsqq f7326458 + ╭─┬─╯ + │ ◉ b1 zsuskuln 072d5ae1 + ◉ │ b2 royxmykx 903ab0d6 + ├─╯ + ◉ a rlvkpnrz 2443ea76 + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["rebase", "-r", "f", "--after", "e", "--after", "d"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 2 commits + Working copy now at: lylxulpl 60b88d56 f | f + Parent commit : kmkuslsw 1f75714f e | e + Parent commit : znkkpsqq f7326458 d | d + Added 1 files, modified 0 files, removed 1 files + "###); + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + ◉ c vruxwmqv dc516a33 + @ f lylxulpl 60b88d56 + ├─╮ + │ ◉ d znkkpsqq f7326458 + │ ├─╮ + ◉ │ │ e kmkuslsw 1f75714f + ╰─┬─╮ + │ ◉ b1 zsuskuln 072d5ae1 + ◉ │ b2 royxmykx 903ab0d6 + ├─╯ + ◉ a rlvkpnrz 2443ea76 + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["rebase", "-r", "b1", "--after", "d", "--after", "e"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 5 commits + Working copy now at: lylxulpl bef37a3d f | f + Parent commit : zsuskuln 697fcb31 b1 | b1 + "###); + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + ◉ c vruxwmqv 997ed500 + @ f lylxulpl bef37a3d + ◉ b1 zsuskuln 697fcb31 + ├─╮ + │ ◉ e kmkuslsw 362b37d6 + ◉ │ d znkkpsqq b9b77883 + ├─╯ + ◉ b2 royxmykx 903ab0d6 + ◉ a rlvkpnrz 2443ea76 + ◉ zzzzzzzz 00000000 + "###); + + let stderr = test_env.jj_cmd_failure( + &repo_path, + &["rebase", "-r", "e", "--after", "a", "--after", "b2"], + ); + insta::assert_snapshot!(stderr, @r###" + Error: Refusing to create a loop: commit 903ab0d6ef88 would be both an ancestor and a descendant of the rebased commit + "###); +} + +#[test] +fn test_rebase_revision_before() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + create_commit(&test_env, &repo_path, "a", &[]); + create_commit(&test_env, &repo_path, "b1", &["a"]); + create_commit(&test_env, &repo_path, "b2", &["a"]); + create_commit(&test_env, &repo_path, "c", &["b1", "b2"]); + create_commit(&test_env, &repo_path, "d", &["c"]); + create_commit(&test_env, &repo_path, "e", &["c"]); + create_commit(&test_env, &repo_path, "f", &["e"]); + // Test the setup + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + @ f lylxulpl 88f778c5 + ◉ e kmkuslsw 48dd9e3f + │ ◉ d znkkpsqq 92438fc9 + ├─╯ + ◉ c vruxwmqv c41e416e + ├─╮ + │ ◉ b2 royxmykx 903ab0d6 + ◉ │ b1 zsuskuln 072d5ae1 + ├─╯ + ◉ a rlvkpnrz 2443ea76 + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "c", "--before", "c"]); + insta::assert_snapshot!(stdout, @""); + // Rebasing a commit before itself should be a no-op. + // TODO: order of parents of C is modified here. + insta::assert_snapshot!(stderr, @r###" + Rebased 4 commits + Working copy now at: lylxulpl 3af16359 f | f + Parent commit : kmkuslsw 9e43a546 e | e + "###); + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + @ f lylxulpl 3af16359 + ◉ e kmkuslsw 9e43a546 + │ ◉ d znkkpsqq 419fbe68 + ├─╯ + ◉ c vruxwmqv 9ddf4993 + ├─╮ + │ ◉ b1 zsuskuln 072d5ae1 + ◉ │ b2 royxmykx 903ab0d6 + ├─╯ + ◉ a rlvkpnrz 2443ea76 + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "b2", "--before", "a"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 11 commits + Working copy now at: lylxulpl de47a5a1 f | f + Parent commit : kmkuslsw 838e25e4 e | e + "###); + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + @ f lylxulpl de47a5a1 + ◉ e kmkuslsw 838e25e4 + │ ◉ d znkkpsqq 89a3c005 + ├─╯ + ◉ c vruxwmqv 58607443 + ◉ b1 zsuskuln 838cb981 + ◉ a rlvkpnrz 4f18a645 + ◉ b2 royxmykx 430dee5b + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "f", "--before", "d"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 2 commits + Working copy now at: lylxulpl 744b4d80 f | f + Parent commit : vruxwmqv 58607443 c | c + Added 0 files, modified 0 files, removed 1 files + "###); + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + ◉ d znkkpsqq e5879ff9 + @ f lylxulpl 744b4d80 + │ ◉ e kmkuslsw 838e25e4 + ├─╯ + ◉ c vruxwmqv 58607443 + ◉ b1 zsuskuln 838cb981 + ◉ a rlvkpnrz 4f18a645 + ◉ b2 royxmykx 430dee5b + ◉ zzzzzzzz 00000000 + "###); + + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["rebase", "-r", "b1", "--before", "d", "--before", "e"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 5 commits + Working copy now at: lylxulpl e317b6c3 f | f + Parent commit : vruxwmqv 7d122618 c | c + Added 0 files, modified 0 files, removed 1 files + "###); + // Note that B1 only has a parent of F instead of F (D's parent) and C (E's + // parent) because C is an ancestor of F. + insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###" + ◉ e kmkuslsw 47251327 + ├─╮ + │ │ ◉ d znkkpsqq e17a1532 + │ ├─╯ + │ ◉ b1 zsuskuln 97220e29 + │ @ f lylxulpl e317b6c3 + ├─╯ + ◉ c vruxwmqv 7d122618 + ◉ a rlvkpnrz 4f18a645 + ◉ b2 royxmykx 430dee5b + ◉ zzzzzzzz 00000000 + "###); + + let stderr = test_env.jj_cmd_failure( + &repo_path, + &["rebase", "-r", "e", "--before", "b2", "--before", "c"], + ); + insta::assert_snapshot!(stderr, @r###" + Error: Refusing to create a loop: commit 4f18a645525f would be both an ancestor and a descendant of the rebased commit + "###); +} + #[test] fn test_rebase_skip_empty() { let test_env = TestEnvironment::default();