Skip to content

Commit

Permalink
rebase: do not modify commit IDs if commit is unchanged after rebase
Browse files Browse the repository at this point in the history
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed Mar 28, 2024
1 parent 6c5a54a commit 472af5b
Show file tree
Hide file tree
Showing 3 changed files with 230 additions and 57 deletions.
78 changes: 64 additions & 14 deletions cli/src/commands/rebase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,13 +290,22 @@ 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() == new_parents);
if !skipped_commits.is_empty() {
log_skipped_rebase_commits_message(ui, workspace_command, &skipped_commits)?;
}
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 @@ -344,7 +353,7 @@ fn rebase_revision(
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;
// Currently, immutable commits are defied so that a child of a rewriteable
// Currently, immutable commits are defined so that a child of a rewriteable
// commit is always rewriteable.
debug_assert!(workspace_command.check_rewritable(&child_commits).is_ok());

Expand Down Expand Up @@ -432,21 +441,40 @@ fn rebase_revision(
})
.try_collect()?;

// Finally, it's safe to rebase `old_commit`. At this point, it should no longer
// have any children; they have all been rebased and the originals have been
// abandoned.
rebase_commit(settings, tx.mut_repo(), &old_commit, &new_parents)?;
debug_assert_eq!(tx.mut_repo().rebase_descendants(settings)?, 0);
// Finally, it's safe to rebase `old_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.
let skipped_commit_rebase = if old_commit.parents() == new_parents {
write!(ui.stderr(), "Skipping rebase of commit ")?;
tx.write_commit_summary(ui.stderr_formatter().as_mut(), &old_commit)?;
writeln!(ui.stderr())?;
true
} else {
rebase_commit(settings, tx.mut_repo(), &old_commit, &new_parents)?;
debug_assert_eq!(tx.mut_repo().rebase_descendants(settings)?, 0);
false
};

if num_rebased_descendants > 0 {
writeln!(
ui.stderr(),
"Also rebased {num_rebased_descendants} descendant commits onto parent of rebased \
commit"
)?;
if skipped_commit_rebase {
writeln!(
ui.stderr(),
"Rebased {num_rebased_descendants} descendant commits onto parent of commit"
)?;
} else {
writeln!(
ui.stderr(),
"Also rebased {num_rebased_descendants} descendant commits onto parent of rebased \
commit"
)?;
}
}
if tx.mut_repo().has_changes() {
tx.finish(ui, format!("rebase commit {}", old_commit.id().hex()))
} else {
Ok(()) // Do not print "Nothing changed."
}
tx.finish(ui, format!("rebase commit {}", old_commit.id().hex()))?;
Ok(())
}

fn check_rebase_destinations(
Expand All @@ -465,3 +493,25 @@ fn check_rebase_destinations(
}
Ok(())
}

fn log_skipped_rebase_commits_message(
ui: &Ui,
workspace_command: &WorkspaceCommandHelper,
commits: &[&Commit],
) -> Result<(), CommandError> {
let mut fmt = ui.stderr_formatter();
let template = workspace_command.commit_summary_template();
if commits.len() == 1 {
write!(fmt, "Skipping rebase of commit ")?;
template.format(commits[0], fmt.as_mut())?;
writeln!(fmt)?;
} else {
writeln!(fmt, "Skipping rebase of commits:")?;
for commit in commits {
write!(fmt, " ")?;
template.format(commit, fmt.as_mut())?;
writeln!(fmt)?;
}
}
Ok(())
}
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
Loading

0 comments on commit 472af5b

Please sign in to comment.