diff --git a/CHANGELOG.md b/CHANGELOG.md index 169f2650d..e7513f1e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,7 +24,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 elided. * `jj squash` now accepts `--from` and `--into` (mutually exclusive with `-r`). - It can thereby be for all use cases where `jj move` can be used. + It can thereby be for all use cases where `jj move` can be used. The `--from` + argument accepts a revset that resolves to move than one revision. ### Fixed bugs diff --git a/cli/src/commands/move.rs b/cli/src/commands/move.rs index 8b2f892dc..0759655be 100644 --- a/cli/src/commands/move.rs +++ b/cli/src/commands/move.rs @@ -89,8 +89,8 @@ pub(crate) fn cmd_move( ui, &mut tx, command.settings(), - source, - destination, + &[source], + &destination, matcher.as_ref(), &diff_selector, None, diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 918be7515..1409398ce 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -80,20 +80,26 @@ pub(crate) fn cmd_squash( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; - let source; + let mut sources; let destination; if args.from.is_some() || args.into.is_some() { - source = workspace_command.resolve_single_rev(args.from.as_deref().unwrap_or("@"))?; + sources = workspace_command.resolve_revset(args.from.as_deref().unwrap_or("@"))?; destination = workspace_command.resolve_single_rev(args.into.as_deref().unwrap_or("@"))?; - if source.id() == destination.id() { + if sources.iter().any(|source| source.id() == destination.id()) { return Err(user_error("Source and destination cannot be the same")); } + // Reverse the set so we apply the oldest commits first. It shouldn't affect the + // result, but it avoids creating transient conflicts and is therefore probably + // a little faster. + sources.reverse(); } else { - source = workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; + let source = + workspace_command.resolve_single_rev(args.revision.as_deref().unwrap_or("@"))?; let mut parents = source.parents(); if parents.len() != 1 { return Err(user_error("Cannot squash merge commits")); } + sources = vec![source]; destination = parents.pop().unwrap(); } @@ -101,15 +107,15 @@ pub(crate) fn cmd_squash( let diff_selector = workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?; let mut tx = workspace_command.start_transaction(); - let tx_description = format!("squash commit {}", source.id().hex()); + let tx_description = format!("squash commits into {}", destination.id().hex()); let description = (!args.message_paragraphs.is_empty()) .then(|| join_message_paragraphs(&args.message_paragraphs)); move_diff( ui, &mut tx, command.settings(), - source, - destination, + &sources, + &destination, matcher.as_ref(), &diff_selector, description, @@ -125,8 +131,8 @@ pub fn move_diff( ui: &mut Ui, tx: &mut WorkspaceCommandTransaction, settings: &UserSettings, - source: Commit, - mut destination: Commit, + sources: &[Commit], + destination: &Commit, matcher: &dyn Matcher, diff_selector: &DiffSelector, description: Option, @@ -134,11 +140,15 @@ pub fn move_diff( path_arg: &[String], ) -> Result<(), CommandError> { tx.base_workspace_helper() - .check_rewritable([&source, &destination])?; - let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?; - let source_tree = source.tree()?; - let instructions = format!( - "\ + .check_rewritable(sources.iter().chain(std::iter::once(destination)))?; + // Tree diffs to apply to the destination + let mut tree_diffs = vec![]; + let mut abandoned_commits = vec![]; + for source in sources { + let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?; + let source_tree = source.tree()?; + let instructions = format!( + "\ You are moving changes from: {} into commit: {} @@ -150,12 +160,32 @@ Adjust the right side until the diff shows the changes you want to move to the destination. If you don't make any changes, then all the changes from the source will be moved into the destination. ", - tx.format_commit_summary(&source), - tx.format_commit_summary(&destination) - ); - let new_parent_tree_id = - diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?; - if new_parent_tree_id == parent_tree.id() { + tx.format_commit_summary(source), + tx.format_commit_summary(destination) + ); + let new_parent_tree_id = + diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?; + let new_parent_tree = tx.repo().store().get_root_tree(&new_parent_tree_id)?; + // TODO: Do we want to optimize the case of moving to the parent commit (`jj + // squash -r`)? The source tree will be unchanged in that case. + + // Apply the reverse of the selected changes onto the source + let new_source_tree = source_tree.merge(&new_parent_tree, &parent_tree)?; + let abandon_source = new_source_tree.id() == parent_tree.id(); + if abandon_source { + abandoned_commits.push(source); + tx.mut_repo().record_abandoned_commit(source.id().clone()); + } else { + tx.mut_repo() + .rewrite_commit(settings, source) + .set_tree_id(new_source_tree.id().clone()) + .write()?; + } + if new_parent_tree_id != parent_tree.id() { + tree_diffs.push((parent_tree, new_parent_tree)); + } + } + if tree_diffs.is_empty() { if diff_selector.is_interactive() { return Err(user_error("No changes selected")); } @@ -176,47 +206,34 @@ from the source will be moved into the destination. } } } - let new_parent_tree = tx.repo().store().get_root_tree(&new_parent_tree_id)?; - // TODO: Do we want to optimize the case of moving to the parent commit (`jj - // squash -r`)? The source tree will be unchanged in that case. - - // Apply the reverse of the selected changes onto the source - let new_source_tree = source_tree.merge(&new_parent_tree, &parent_tree)?; - let abandon_source = new_source_tree.id() == parent_tree.id(); - if abandon_source { - tx.mut_repo().record_abandoned_commit(source.id().clone()); - } else { - tx.mut_repo() - .rewrite_commit(settings, &source) - .set_tree_id(new_source_tree.id().clone()) - .write()?; - } - if tx.repo().index().is_ancestor(source.id(), destination.id()) { + let mut rewritten_destination = destination.clone(); + if sources + .iter() + .any(|source| tx.repo().index().is_ancestor(source.id(), destination.id())) + { // If we're moving changes to a descendant, first rebase descendants onto the - // rewritten source. Otherwise it will likely already have the content + // rewritten sources. Otherwise it will likely already have the content // changes we're moving, so applying them will have no effect and the // changes will disappear. let rebase_map = tx.mut_repo().rebase_descendants_return_map(settings)?; let rebased_destination_id = rebase_map.get(destination.id()).unwrap().clone(); - destination = tx.mut_repo().store().get_commit(&rebased_destination_id)?; + rewritten_destination = tx.mut_repo().store().get_commit(&rebased_destination_id)?; } // Apply the selected changes onto the destination - let destination_tree = destination.tree()?; - let new_destination_tree = destination_tree.merge(&parent_tree, &new_parent_tree)?; + let mut destination_tree = rewritten_destination.tree()?; + for (tree1, tree2) in tree_diffs { + destination_tree = destination_tree.merge(&tree1, &tree2)?; + } let description = match description { Some(description) => description, - None => { - if abandon_source { - combine_messages(tx.base_repo(), &[&source], &destination, settings)? - } else { - destination.description().to_owned() - } - } + None => combine_messages(tx.base_repo(), &abandoned_commits, destination, settings)?, }; + let mut predecessors = vec![destination.id().clone()]; + predecessors.extend(sources.iter().map(|source| source.id().clone())); tx.mut_repo() - .rewrite_commit(settings, &destination) - .set_tree_id(new_destination_tree.id().clone()) - .set_predecessors(vec![destination.id().clone(), source.id().clone()]) + .rewrite_commit(settings, &rewritten_destination) + .set_tree_id(destination_tree.id().clone()) + .set_predecessors(predecessors) .set_description(description) .write()?; Ok(()) diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index ab084e422..24daa6a5c 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -582,6 +582,274 @@ fn test_squash_from_to_partial() { "###); } +#[test] +fn test_squash_from_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 history like this: + // F + // | + // E + // /|\ + // B C D + // \|/ + // A + let file = repo_path.join("file"); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]); + std::fs::write(&file, "a\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]); + std::fs::write(&file, "b\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "@-"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "c"]); + std::fs::write(&file, "c\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "@-"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "d"]); + std::fs::write(&file, "d\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "all:visible_heads()"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "e"]); + std::fs::write(&file, "e\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "f"]); + std::fs::write(&file, "f\n").unwrap(); + // Test the setup + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 7c982f87d244 f + ◉ 90fb23310e1d e + ├─┬─╮ + │ │ ◉ 512dff087306 b + │ ◉ │ 5ee503da2262 c + │ ├─╯ + ◉ │ cb214cffd91a d + ├─╯ + ◉ 37941ee54ace a + ◉ 000000000000 + "###); + + // Squash a few commits sideways + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "--from=b|c", "--into=d"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 2 descendant commits + New conflicts appeared in these commits: + yqosqzyt d5401742 d | (conflict) (no description set) + To resolve the conflicts, start by updating to it: + jj new yqosqzytrlsw + Then use `jj resolve`, or edit the conflict markers in the file directly. + Once the conflicts are resolved, you may want inspect the result with `jj diff`. + Then run `jj squash` to move the resolution into the conflicted commit. + Working copy now at: kpqxywon cc9f4cad f | (no description set) + Parent commit : yostqsxw 9f25b62d e | (no description set) + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ cc9f4cad1a29 f + ◉ 9f25b62ddffc e + ├─╮ + ◉ │ d54017421f3f d + ├─╯ + ◉ 37941ee54ace a b c + ◉ 000000000000 + "###); + // The changes from the sources have been applied + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=d", "file"]); + insta::assert_snapshot!(stdout, @r###" + <<<<<<< + %%%%%%% + -a + +d + %%%%%%% + -a + +b + +++++++ + c + >>>>>>> + "###); + + // Squash a few commits up an down + test_env.jj_cmd_ok(&repo_path, &["undo"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "--from=b|c|f", "--into=e"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 1 descendant commits + Working copy now at: xznxytkn 59801ce3 (empty) (no description set) + Parent commit : yostqsxw b7bc1dda e f | (no description set) + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 59801ce3ff81 + ◉ b7bc1dda247e e f + ├─╮ + ◉ │ cb214cffd91a d + ├─╯ + ◉ 37941ee54ace a b c + ◉ 000000000000 + "###); + // The changes from the sources have been applied to the destination + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=e", "file"]); + insta::assert_snapshot!(stdout, @r###" + f + "###); +} + +#[test] +fn test_squash_from_multiple_partial() { + 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 history like this: + // F + // | + // E + // /|\ + // B C D + // \|/ + // A + let file1 = repo_path.join("file1"); + let file2 = repo_path.join("file2"); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "a"]); + std::fs::write(&file1, "a\n").unwrap(); + std::fs::write(&file2, "a\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "b"]); + std::fs::write(&file1, "b\n").unwrap(); + std::fs::write(&file2, "b\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "@-"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "c"]); + std::fs::write(&file1, "c\n").unwrap(); + std::fs::write(&file2, "c\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "@-"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "d"]); + std::fs::write(&file1, "d\n").unwrap(); + std::fs::write(&file2, "d\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new", "all:visible_heads()"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "e"]); + std::fs::write(&file1, "e\n").unwrap(); + std::fs::write(&file2, "e\n").unwrap(); + test_env.jj_cmd_ok(&repo_path, &["new"]); + test_env.jj_cmd_ok(&repo_path, &["branch", "create", "f"]); + std::fs::write(&file1, "f\n").unwrap(); + std::fs::write(&file2, "f\n").unwrap(); + // Test the setup + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 5adc4b1fb0f9 f + ◉ 8ba764396a28 e + ├─┬─╮ + │ │ ◉ 2a2d19a3283f b + │ ◉ │ 864a16169cef c + │ ├─╯ + ◉ │ 5def0e76dfaf d + ├─╯ + ◉ 47a1e795d146 a + ◉ 000000000000 + "###); + + // Partially squash a few commits sideways + let (stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["squash", "--from=b|c", "--into=d", "file1"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 2 descendant commits + New conflicts appeared in these commits: + yqosqzyt 13468b54 d | (conflict) (no description set) + To resolve the conflicts, start by updating to it: + jj new yqosqzytrlsw + Then use `jj resolve`, or edit the conflict markers in the file directly. + Once the conflicts are resolved, you may want inspect the result with `jj diff`. + Then run `jj squash` to move the resolution into the conflicted commit. + Working copy now at: kpqxywon 8aaa7910 f | (no description set) + Parent commit : yostqsxw 5aad25ea e | (no description set) + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 8aaa79109163 f + ◉ 5aad25eae5aa e + ├─┬─╮ + │ │ ◉ ba60ddff2d41 b + │ ◉ │ 8ef5a315bf7d c + │ ├─╯ + ◉ │ 13468b546ba3 d + ├─╯ + ◉ 47a1e795d146 a + ◉ 000000000000 + "###); + // The selected changes have been removed from the sources + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=b", "file1"]); + insta::assert_snapshot!(stdout, @r###" + a + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=c", "file1"]); + insta::assert_snapshot!(stdout, @r###" + a + "###); + // The selected changes from the sources have been applied + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=d", "file1"]); + insta::assert_snapshot!(stdout, @r###" + <<<<<<< + %%%%%%% + -a + +d + %%%%%%% + -a + +b + +++++++ + c + >>>>>>> + "###); + // The unselected change from the sources have not been applied to the + // destination + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=d", "file2"]); + insta::assert_snapshot!(stdout, @r###" + d + "###); + + // Partially squash a few commits up an down + test_env.jj_cmd_ok(&repo_path, &["undo"]); + let (stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["squash", "--from=b|c|f", "--into=e", "file1"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Rebased 1 descendant commits + Working copy now at: kpqxywon 610a144d f | (no description set) + Parent commit : yostqsxw ac27a136 e | (no description set) + "###); + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ 610a144de39b f + ◉ ac27a1361b09 e + ├─┬─╮ + │ │ ◉ 0c8eab864a32 b + │ ◉ │ ad1776ad0b1b c + │ ├─╯ + ◉ │ 5def0e76dfaf d + ├─╯ + ◉ 47a1e795d146 a + ◉ 000000000000 + "###); + // The selected changes have been removed from the sources + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=b", "file1"]); + insta::assert_snapshot!(stdout, @r###" + a + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=c", "file1"]); + insta::assert_snapshot!(stdout, @r###" + a + "###); + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=f", "file1"]); + insta::assert_snapshot!(stdout, @r###" + f + "###); + // The selected changes from the sources have been applied to the destination + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=e", "file1"]); + insta::assert_snapshot!(stdout, @r###" + f + "###); + // The unselected changes from the sources have not been applied + let stdout = test_env.jj_cmd_success(&repo_path, &["print", "-r=d", "file2"]); + insta::assert_snapshot!(stdout, @r###" + d + "###); +} + fn get_log_output(test_env: &TestEnvironment, repo_path: &Path) -> String { let template = r#"commit_id.short() ++ " " ++ branches"#; test_env.jj_cmd_success(repo_path, &["log", "-T", template])