From cd41bc358486f4f32455adc8fe3331ceb96ebb50 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Sun, 7 Jul 2024 05:06:56 +0800 Subject: [PATCH] backout: accept multiple revisions to back out Closes #3339. --- CHANGELOG.md | 5 ++ cli/src/commands/backout.rs | 71 ++++++++++++------- cli/tests/cli-reference@.md.snap | 2 +- cli/tests/test_backout_command.rs | 112 +++++++++++++++++++++++++++++- 4 files changed, 160 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 878a8fceb0..76b0f7e752 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj rebase --skip-empty` has been renamed to `jj rebase --skip-emptied` +* `jj backout --revision` has been renamed to `jj backout --revisions`. + The short alias `-r` is still supported. + ### Deprecations ### New features @@ -33,6 +36,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * `jj backout` now includes the backed out commit's subject in the new commit message. +* `jj backout` can now back out multiple commits at once. + ### Fixed bugs ## [0.19.0] - 2024-07-03 diff --git a/cli/src/commands/backout.rs b/cli/src/commands/backout.rs index 9562b808d1..b56c1c86c6 100644 --- a/cli/src/commands/backout.rs +++ b/cli/src/commands/backout.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use itertools::Itertools as _; use jj_lib::object_id::ObjectId; use jj_lib::rewrite::merge_commit_trees; use tracing::instrument; @@ -23,9 +24,9 @@ use crate::ui::Ui; /// Apply the reverse of a revision on top of another revision #[derive(clap::Args, Clone, Debug)] pub(crate) struct BackoutArgs { - /// The revision to apply the reverse of + /// The revision(s) to apply the reverse of #[arg(long, short, default_value = "@")] - revision: RevisionArg, + revisions: Vec, /// The revision to apply the reverse changes on top of // TODO: It seems better to default this to `@-`. Maybe the working // copy should be rebased on top? @@ -40,36 +41,54 @@ pub(crate) fn cmd_backout( args: &BackoutArgs, ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let commit_to_back_out = workspace_command.resolve_single_rev(&args.revision)?; + let to_back_out: Vec<_> = workspace_command + .parse_union_revsets(&args.revisions)? + .evaluate_to_commits()? + .try_collect()?; // in reverse topological order + if to_back_out.is_empty() { + writeln!(ui.status(), "No revisions to back out.")?; + return Ok(()); + } let mut parents = vec![]; for revision_str in &args.destination { let destination = workspace_command.resolve_single_rev(revision_str)?; parents.push(destination); } let mut tx = workspace_command.start_transaction(); - let commit_to_back_out_subject = commit_to_back_out - .description() - .lines() - .next() - .unwrap_or_default(); - let new_commit_description = format!( - "Back out \"{}\"\n\nThis backs out commit {}.\n", - commit_to_back_out_subject, - &commit_to_back_out.id().hex() - ); - let old_base_tree = commit_to_back_out.parent_tree(tx.mut_repo())?; - let new_base_tree = merge_commit_trees(tx.mut_repo(), &parents)?; - let old_tree = commit_to_back_out.tree()?; - let new_tree = new_base_tree.merge(&old_tree, &old_base_tree)?; - let new_parent_ids = parents.iter().map(|commit| commit.id().clone()).collect(); - tx.mut_repo() - .new_commit(command.settings(), new_parent_ids, new_tree.id()) - .set_description(new_commit_description) - .write()?; - tx.finish( - ui, - format!("back out commit {}", commit_to_back_out.id().hex()), - )?; + let transaction_description = if to_back_out.len() == 1 { + format!("back out commit {}", to_back_out[0].id().hex()) + } else { + format!( + "back out commit {} and {} more", + to_back_out[0].id().hex(), + to_back_out.len() - 1 + ) + }; + let mut new_base_tree = merge_commit_trees(tx.mut_repo(), &parents)?; + for commit_to_back_out in to_back_out { + let commit_to_back_out_subject = commit_to_back_out + .description() + .lines() + .next() + .unwrap_or_default(); + let new_commit_description = format!( + "Back out \"{}\"\n\nThis backs out commit {}.\n", + commit_to_back_out_subject, + &commit_to_back_out.id().hex() + ); + let old_base_tree = commit_to_back_out.parent_tree(tx.mut_repo())?; + let old_tree = commit_to_back_out.tree()?; + let new_tree = new_base_tree.merge(&old_tree, &old_base_tree)?; + let new_parent_ids = parents.iter().map(|commit| commit.id().clone()).collect(); + let new_commit = tx + .mut_repo() + .new_commit(command.settings(), new_parent_ids, new_tree.id()) + .set_description(new_commit_description) + .write()?; + parents = vec![new_commit]; + new_base_tree = new_tree; + } + tx.finish(ui, transaction_description)?; Ok(()) } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 9547c31583..c944c89327 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -215,7 +215,7 @@ Apply the reverse of a revision on top of another revision ###### **Options:** -* `-r`, `--revision ` — The revision to apply the reverse of +* `-r`, `--revisions ` — The revision(s) to apply the reverse of Default value: `@` * `-d`, `--destination ` — The revision to apply the reverse changes on top of diff --git a/cli/tests/test_backout_command.rs b/cli/tests/test_backout_command.rs index 6d0aa53e14..d44ab492e1 100644 --- a/cli/tests/test_backout_command.rs +++ b/cli/tests/test_backout_command.rs @@ -16,7 +16,13 @@ use std::path::Path; use crate::common::TestEnvironment; -fn create_commit(test_env: &TestEnvironment, repo_path: &Path, name: &str, parents: &[&str]) { +fn create_commit( + test_env: &TestEnvironment, + repo_path: &Path, + name: &str, + parents: &[&str], + files: &[(&str, &str)], +) { if parents.is_empty() { test_env.jj_cmd_ok(repo_path, &["new", "root()", "-m", name]); } else { @@ -24,7 +30,9 @@ fn create_commit(test_env: &TestEnvironment, repo_path: &Path, name: &str, paren args.extend(parents); test_env.jj_cmd_ok(repo_path, &args); } - std::fs::write(repo_path.join(name), format!("{name}\n")).unwrap(); + for (name, contents) in files { + std::fs::write(repo_path.join(name), contents).unwrap(); + } test_env.jj_cmd_ok(repo_path, &["branch", "create", name]); } @@ -34,7 +42,7 @@ fn test_backout() { 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, "a", &[], &[("a", "a\n")]); // Test the setup insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" @ 2443ea76b0b1 a @@ -82,6 +90,104 @@ fn test_backout() { "###); } +#[test] +fn test_backout_multiple() { + 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", &[], &[("a", "a\n")]); + create_commit(&test_env, &repo_path, "b", &["a"], &[("a", "a\nb\n")]); + create_commit( + &test_env, + &repo_path, + "c", + &["b"], + &[("a", "a\nb\n"), ("b", "b\n")], + ); + create_commit(&test_env, &repo_path, "d", &["c"], &[]); + create_commit(&test_env, &repo_path, "e", &["d"], &[("a", "a\nb\nc\n")]); + + // Test the setup + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 208f8612074a e + ◉ ceeec03be46b d + ◉ 413337bbd11f c + ◉ 46cc97af6802 b + ◉ 2443ea76b0b1 a + ◉ 000000000000 + "###); + + // Backout multiple commits + let (stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["backout", "-r", "b", "-r", "c", "-r", "e"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @""); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + ◉ 6504c4ded177 Back out "b" + │ + │ This backs out commit 46cc97af6802301d8db381386e8485ff3ff24ae6. + ◉ d31d42e0267f Back out "c" + │ + │ This backs out commit 413337bbd11f7a6636c010d9e196acf801d8df2f. + ◉ 8ff3fbc2ccb0 Back out "e" + │ + │ This backs out commit 208f8612074af4c219d06568a8e1f04f2e80dc25. + @ 208f8612074a e + ◉ ceeec03be46b d + ◉ 413337bbd11f c + ◉ 46cc97af6802 b + ◉ 2443ea76b0b1 a + ◉ 000000000000 + "###); + // View the output of each backed out commit + let stdout = test_env.jj_cmd_success(&repo_path, &["show", "@+"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: 8ff3fbc2ccb0d66985f558c461d1643cebb4c7d6 + Change ID: wqnwkozpkustnxypnnntnykwrqrkrpvv + Author: Test User (2001-02-03 08:05:19) + Committer: Test User (2001-02-03 08:05:19) + + Back out "e" + + This backs out commit 208f8612074af4c219d06568a8e1f04f2e80dc25. + + Modified regular file a: + 1 1: a + 2 2: b + 3 : c + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["show", "@++"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: d31d42e0267f6524d445348b1dd00926c62a6b57 + Change ID: mouksmquosnpvwqrpsvvxtxpywpnxlss + Author: Test User (2001-02-03 08:05:19) + Committer: Test User (2001-02-03 08:05:19) + + Back out "c" + + This backs out commit 413337bbd11f7a6636c010d9e196acf801d8df2f. + + Removed regular file b: + 1 : b + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["show", "@+++"]); + insta::assert_snapshot!(stdout, @r###" + Commit ID: 6504c4ded177fba2334f76683d1aa643700d5073 + Change ID: tqvpomtpwrqsylrpsxknultrymmqxmxv + Author: Test User (2001-02-03 08:05:19) + Committer: Test User (2001-02-03 08:05:19) + + Back out "b" + + This backs out commit 46cc97af6802301d8db381386e8485ff3ff24ae6. + + Modified regular file a: + 1 1: a + 2 : b + "###); +} + fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"commit_id.short() ++ " " ++ description"#; test_env.jj_cmd_success(cwd, &["log", "-T", template])