diff --git a/CHANGELOG.md b/CHANGELOG.md index aad38dc042d..2923d2fbc98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 literals. This means that `snapshot.max-new-file-size="1"` and `snapshot.max-new-file-size=1` are now equivalent. +* `jj squash ` is now a no-op if the path argument didn't match any paths + (it used to create new commits with bumped timestamp). + [#3334](https://github.com/martinvonz/jj/issues/3334) + ## [0.16.0] - 2024-04-03 ### Deprecations diff --git a/cli/src/commands/squash.rs b/cli/src/commands/squash.rs index 09dcb47d121..bbf6112a2fb 100644 --- a/cli/src/commands/squash.rs +++ b/cli/src/commands/squash.rs @@ -213,6 +213,11 @@ from the source will be moved into the destination. diff_selector.select(&parent_tree, &source_tree, matcher, Some(&instructions))?; let selected_tree = tx.repo().store().get_root_tree(&selected_tree_id)?; let abandon = selected_tree.id() == source_tree.id(); + if !abandon && selected_tree_id == parent_tree.id() { + // Nothing selected from this commit. If it was already empty, we still include + // it so it gets abandoned. + continue; + } // 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. source_commits.push(SourceCommit { @@ -222,10 +227,7 @@ from the source will be moved into the destination. abandon, }); } - if source_commits - .iter() - .all(|source| source.selected_tree == source.parent_tree) - { + if source_commits.is_empty() { if diff_selector.is_interactive() { return Err(user_error("No changes selected")); } @@ -245,6 +247,8 @@ from the source will be moved into the destination. )?; } } + + return Ok(()); } for source in &source_commits { diff --git a/cli/tests/test_move_command.rs b/cli/tests/test_move_command.rs index 07c297aec06..156a77a83a3 100644 --- a/cli/tests/test_move_command.rs +++ b/cli/tests/test_move_command.rs @@ -345,16 +345,14 @@ fn test_move_partial() { a "###); - // If we specify only a non-existent file, then the move still succeeds and - // creates unchanged commits. + // If we specify only a non-existent file, then nothing changes. test_env.jj_cmd_ok(&repo_path, &["undo"]); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["move", "--from", "c", "nonexistent"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent Warning: `jj move` will be removed in a future version, and this will be a hard error - Working copy now at: vruxwmqv b670567d d | (no description set) - Parent commit : qpvuntsm 3db0a2f5 a | (no description set) + Nothing changed. "###); } diff --git a/cli/tests/test_squash_command.rs b/cli/tests/test_squash_command.rs index eb5c284eb6c..c65c3bfaf57 100644 --- a/cli/tests/test_squash_command.rs +++ b/cli/tests/test_squash_command.rs @@ -241,15 +241,12 @@ fn test_squash_partial() { b "###); - // If we specify only a non-existent file, then the squash still succeeds and - // creates unchanged commits. + // If we specify only a non-existent file, then nothing changes. test_env.jj_cmd_ok(&repo_path, &["undo"]); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "-r", "b", "nonexistent"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Rebased 2 descendant commits - Working copy now at: mzvwutvl 5e297967 c | (no description set) - Parent commit : kkmpptxz ac258609 b | (no description set) + Nothing changed. "###); // We get a warning if we pass a positional argument that looks like a revset @@ -257,9 +254,7 @@ fn test_squash_partial() { let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "b"]); insta::assert_snapshot!(stderr, @r###" Warning: The argument "b" is being interpreted as a path. To specify a revset, pass -r "b" instead. - Rebased 1 descendant commits - Working copy now at: mzvwutvl 1c4e5596 c | (no description set) - Parent commit : kkmpptxz 16cc94b4 b | (no description set) + Nothing changed. "###); insta::assert_snapshot!(stdout, @""); } @@ -570,15 +565,13 @@ fn test_squash_from_to_partial() { a "###); - // If we specify only a non-existent file, then the move still succeeds and - // creates unchanged commits. + // If we specify only a non-existent file, then nothing changes. test_env.jj_cmd_ok(&repo_path, &["undo"]); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "--from", "c", "nonexistent"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: vruxwmqv b670567d d | (no description set) - Parent commit : qpvuntsm 3db0a2f5 a | (no description set) + Nothing changed. "###); } @@ -692,12 +685,11 @@ fn test_squash_from_multiple() { f "###); - // Empty squash shouldn't crash (could be noop) + // Empty squash shouldn't crash let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["squash", "--from=none()"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: xznxytkn 68d54010 (empty) (no description set) - Parent commit : yostqsxw b7bc1dda e f | (no description set) + Nothing changed. "###); } @@ -893,7 +885,6 @@ fn test_squash_from_multiple_partial_no_op() { "###); // Source commits that didn't match the paths are not rewritten - // TODO: Comit c should be unchanged let (stdout, stderr) = test_env.jj_cmd_ok( &repo_path, &["squash", "--from=@-+ ~ @", "--into=@", "-m=d", "b"], @@ -906,7 +897,7 @@ fn test_squash_from_multiple_partial_no_op() { "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" @ 9227d0d780fa d - │ ◉ 8907be4aab96 c + │ ◉ 5ad3ca4090a7 c ├─╯ ◉ 3df52ee1f8a9 a ◉ 000000000000 @@ -932,7 +923,6 @@ fn test_squash_from_multiple_partial_no_op() { "###); // If no source commits match the paths, then the whole operation is a no-op - // TODO: All commits should be unchanged test_env.jj_cmd_ok(&repo_path, &["undo"]); let (stdout, stderr) = test_env.jj_cmd_ok( &repo_path, @@ -940,14 +930,13 @@ fn test_squash_from_multiple_partial_no_op() { ); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" - Working copy now at: mzvwutvl 17f5bc7a d - Parent commit : qpvuntsm 3df52ee1 a + Nothing changed. "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ 17f5bc7ab82a d - │ ◉ 6eba26a0b46d c + @ 09441f0a6266 d + │ ◉ 5ad3ca4090a7 c ├─╯ - │ ◉ 88c573358ed5 b + │ ◉ 285201979c90 b ├─╯ ◉ 3df52ee1f8a9 a ◉ 000000000000 diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 66dd6350cc9..b233e435bd0 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -186,9 +186,10 @@ impl<'repo> CommitRewriter<'repo> { /// Records the old commit as abandoned with the new parents. pub fn record_abandoned_commit(self) { - let old_commit_id =self.old_commit.id().clone(); + let old_commit_id = self.old_commit.id().clone(); let new_parents = self.new_parents; - self.mut_repo.record_abandoned_commit_with_parents(old_commit_id, new_parents); + self.mut_repo + .record_abandoned_commit_with_parents(old_commit_id, new_parents); } /// Rebase the old commit onto the new parents. Returns a `CommitBuilder`