Skip to content

Commit

Permalink
rebase: skip rebasing if commit's parents are identical to new parents
Browse files Browse the repository at this point in the history
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed Mar 27, 2024
1 parent 423a2a5 commit 7f0c02f
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 46 deletions.
24 changes: 23 additions & 1 deletion cli/src/commands/rebase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,27 @@ fn rebase_descendants(
rebase_options: RebaseOptions,
) -> Result<(), CommandError> {
workspace_command.check_rewritable(old_commits)?;
let (skipped_commits, old_commits) = old_commits
.iter()
.partition::<Vec<_>, _>(|commit| commit.parents().eq(new_parents));
for commit in skipped_commits.iter() {
writeln!(
ui.stderr(),
"Skipping rebase of commit {}",
short_commit_hash(commit.id()),
)?;
continue;
}
if old_commits.is_empty() {
return Ok(());
}
for old_commit in old_commits.iter() {
check_rebase_destinations(workspace_command.repo(), new_parents, old_commit)?;
}
let mut tx = workspace_command.start_transaction();
// `rebase_descendants` takes care of sorting in reverse topological order, so
// no need to do it here.
for old_commit in old_commits {
for old_commit in old_commits.iter() {
rebase_commit_with_options(
settings,
tx.mut_repo(),
Expand Down Expand Up @@ -330,6 +344,14 @@ fn rebase_revision(
) -> Result<(), CommandError> {
let old_commit = workspace_command.resolve_single_rev(rev_str)?;
workspace_command.check_rewritable([&old_commit])?;
if old_commit.parents().eq(new_parents) {
writeln!(
ui.stderr(),
"Skipping rebase of commit {}",
short_commit_hash(old_commit.id()),
)?;
return Ok(());
}
if new_parents.contains(&old_commit) {
return Err(user_error(format!(
"Cannot rebase {} onto itself",
Expand Down
36 changes: 21 additions & 15 deletions cli/tests/test_duplicate_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -293,49 +293,55 @@ fn test_rebase_duplicates() {

create_commit(&test_env, &repo_path, "a", &[]);
create_commit(&test_env, &repo_path, "b", &["a"]);
create_commit(&test_env, &repo_path, "c", &["b"]);
// Test the setup
insta::assert_snapshot!(get_log_output_with_ts(&test_env, &repo_path), @r###"
@ 1394f625cbbd b @ 2001-02-03 04:05:11.000 +07:00
@ 7e4fbf4f2759 c @ 2001-02-03 04:05:13.000 +07:00
◉ 1394f625cbbd b @ 2001-02-03 04:05:11.000 +07:00
◉ 2443ea76b0b1 a @ 2001-02-03 04:05:09.000 +07:00
◉ 000000000000 @ 1970-01-01 00:00:00.000 +00:00
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["duplicate", "b"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["duplicate", "c"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Duplicated 1394f625cbbd as yqosqzyt fdaaf395 b
Duplicated 7e4fbf4f2759 as yostqsxw 0ac2063b c
"###);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["duplicate", "b"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["duplicate", "c"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Duplicated 1394f625cbbd as vruxwmqv 870cf438 b
Duplicated 7e4fbf4f2759 as znkkpsqq ce5f4eeb c
"###);
insta::assert_snapshot!(get_log_output_with_ts(&test_env, &repo_path), @r###"
◉ 870cf438ccbb b @ 2001-02-03 04:05:14.000 +07:00
│ ◉ fdaaf3950f07 b @ 2001-02-03 04:05:13.000 +07:00
◉ ce5f4eeb69d1 c @ 2001-02-03 04:05:16.000 +07:00
│ ◉ 0ac2063b1bee c @ 2001-02-03 04:05:15.000 +07:00
├─╯
│ @ 1394f625cbbd b @ 2001-02-03 04:05:11.000 +07:00
│ @ 7e4fbf4f2759 c @ 2001-02-03 04:05:13.000 +07:00
├─╯
◉ 1394f625cbbd b @ 2001-02-03 04:05:11.000 +07:00
◉ 2443ea76b0b1 a @ 2001-02-03 04:05:09.000 +07:00
◉ 000000000000 @ 1970-01-01 00:00:00.000 +00:00
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "a", "-d", "a-"]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "b", "-d", "root()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 4 commits
Working copy now at: zsuskuln 29bd36b6 b | b
Parent commit : rlvkpnrz 2f6dc5a1 a | a
Working copy now at: royxmykx ed671a3c c | c
Parent commit : zsuskuln 4c6f1569 b | b
Added 0 files, modified 0 files, removed 1 files
"###);
// Some of the duplicate commits' timestamps were changed a little to make them
// have distinct commit ids.
insta::assert_snapshot!(get_log_output_with_ts(&test_env, &repo_path), @r###"
◉ b43fe7354758 b @ 2001-02-03 04:05:14.000 +07:00
│ ◉ 08beb14c3ead b @ 2001-02-03 04:05:15.000 +07:00
◉ b86e9f27d085 c @ 2001-02-03 04:05:16.000 +07:00
│ ◉ 8033590fe04d c @ 2001-02-03 04:05:17.000 +07:00
├─╯
│ @ 29bd36b60e60 b @ 2001-02-03 04:05:16.000 +07:00
│ @ ed671a3cbf35 c @ 2001-02-03 04:05:18.000 +07:00
├─╯
◉ 4c6f1569e2a9 b @ 2001-02-03 04:05:18.000 +07:00
│ ◉ 2443ea76b0b1 a @ 2001-02-03 04:05:09.000 +07:00
├─╯
◉ 2f6dc5a1ffc2 a @ 2001-02-03 04:05:16.000 +07:00
◉ 000000000000 @ 1970-01-01 00:00:00.000 +00:00
"###);
}
Expand Down
124 changes: 94 additions & 30 deletions cli/tests/test_rebase_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,14 +145,15 @@ fn test_rebase_branch() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-b=e", "-b=d", "-d=b"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 2 commits
Skipping rebase of commit 514fa6b265d4
Rebased 1 commits
Working copy now at: znkkpsqq 9ca2a154 e | e
Parent commit : zsuskuln 1394f625 b | b
Added 1 files, modified 0 files, removed 0 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ d
│ @ e
@ e
│ ◉ d
├─╯
│ ◉ c
├─╯
Expand All @@ -174,14 +175,15 @@ fn test_rebase_branch() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-b=all:e|d", "-d=b"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 2 commits
Skipping rebase of commit 514fa6b265d4
Rebased 1 commits
Working copy now at: znkkpsqq 817e3fb0 e | e
Parent commit : zsuskuln 1394f625 b | b
Added 1 files, modified 0 files, removed 0 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ d
│ @ e
@ e
│ ◉ d
├─╯
│ ◉ c
├─╯
Expand Down Expand Up @@ -691,12 +693,10 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
let (stdout, stderr) =
test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "base", "-d", "root()"]);
insta::assert_snapshot!(stdout, @"");
// This should be a no-op
insta::assert_snapshot!(stderr, @r###"
Rebased 4 commits
Working copy now at: vruxwmqv 13b23fa1 c | c
Parent commit : royxmykx 15092f8a b | b
Skipping rebase of commit 0c61db1be8c8
"###);
// This should be a no-op
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ c
◉ b
Expand All @@ -710,12 +710,10 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "a", "-d", "base"]);
insta::assert_snapshot!(stdout, @"");
// This should be a no-op
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits
Working copy now at: vruxwmqv 2d130d92 c | c
Parent commit : royxmykx f9e26e2a b | b
Skipping rebase of commit 2c5b785856e8
"###);
// This should be a no-op
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ c
◉ b
Expand Down Expand Up @@ -761,13 +759,11 @@ fn test_rebase_with_child_and_descendant_bug_2600() {

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-b", "c", "-d", "base"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits
Working copy now at: vruxwmqv fe9741ac c | c
Parent commit : royxmykx 9da147b4 b | b
"###);
// The commits in roots(base..c), i.e. commit "a" should be rebased onto "base",
// which is a no-op
insta::assert_snapshot!(stderr, @r###"
Skipping rebase of commit 2c5b785856e8
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ c
◉ b
Expand Down Expand Up @@ -799,12 +795,10 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-b", "a", "-d", "root()"]);
insta::assert_snapshot!(stdout, @"");
// This should be a no-op
insta::assert_snapshot!(stderr, @r###"
Rebased 4 commits
Working copy now at: vruxwmqv 631fc029 c | c
Parent commit : royxmykx 1cd34b4d b | b
Skipping rebase of commit 0c61db1be8c8
"###);
// This should be a no-op
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ c
◉ b
Expand Down Expand Up @@ -832,10 +826,7 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "base", "-d", "root()"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Also rebased 4 descendant commits onto parent of rebased commit
Working copy now at: vruxwmqv 57aaa944 c | c
Parent commit : royxmykx c8495a71 b | b
Added 0 files, modified 0 files, removed 1 files
Skipping rebase of commit 0c61db1be8c8
"###);
// The user would expect unsimplified ancestry here.
// ◉ base
Expand All @@ -847,11 +838,12 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
// ├─╯
// ◉
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ base
│ @ c
│ ◉ b
@ c
◉ b
├─╮
│ ◉ a
├─╯
◉ base
◉
"###);

Expand Down Expand Up @@ -1102,3 +1094,75 @@ fn test_rebase_skip_empty() {
◉
"###);
}

#[test]
fn test_rebase_skip_if_on_destination() {
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, "b", &["a"]);
create_commit(&test_env, &repo_path, "c", &["b"]);
create_commit(&test_env, &repo_path, "d", &["b"]);
create_commit(&test_env, &repo_path, "e", &["a"]);
// Test the setup
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
│ ◉ d
│ │ ◉ c
│ ├─╯
│ ◉ b
├─╯
◉ a
◉
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-b", "c", "-d", "a"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Skipping rebase of commit 1394f625cbbd
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
│ ◉ d
│ │ ◉ c
│ ├─╯
│ ◉ b
├─╯
◉ a
◉
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "b", "-d", "a"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Skipping rebase of commit 1394f625cbbd
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
│ ◉ d
│ │ ◉ c
│ ├─╯
│ ◉ b
├─╯
◉ a
◉
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "b", "-d", "a"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Skipping rebase of commit 1394f625cbbd
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
│ ◉ d
│ │ ◉ c
│ ├─╯
│ ◉ b
├─╯
◉ a
◉
"###);
}

0 comments on commit 7f0c02f

Please sign in to comment.