diff --git a/CHANGELOG.md b/CHANGELOG.md index 031a8c796..40fa841e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 key](https://toml.io/en/v1.0.0#keys). Quote meta characters as needed. Example: `jj config get "revset-aliases.'trunk()'"` +* If a new working-copy commit is created because the old one was abandoned, and + the old commit was merge, then the new commit will now also be. + [#2859](https://github.com/martinvonz/jj/issues/2859) + ### Deprecations - Attempting to alias a built-in command now gives a warning, rather than being silently ignored. diff --git a/cli/tests/test_abandon_command.rs b/cli/tests/test_abandon_command.rs index 9cb975529..b4249190a 100644 --- a/cli/tests/test_abandon_command.rs +++ b/cli/tests/test_abandon_command.rs @@ -78,39 +78,44 @@ fn test_basics() { insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Abandoned commit znkkpsqq 5557ece3 e | e - Working copy now at: nkmrtpmo 6b527513 (empty) (no description set) + Working copy now at: nkmrtpmo d4f8ea73 (empty) (no description set) Parent commit : rlvkpnrz 2443ea76 a e?? | a - Added 0 files, modified 0 files, removed 3 files + Parent commit : vruxwmqv b7c62f28 d e?? | d + Added 0 files, modified 0 files, removed 1 files "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ [nkm] - │ ◉ [zsu] b - ├─╯ - ◉ [rlv] a e?? + @ [nkm] + ├─╮ │ ◉ [vru] d e?? │ ◉ [roy] c + │ │ ◉ [zsu] b + ├───╯ + ◉ │ [rlv] a e?? ├─╯ ◉ [zzz] "###); test_env.jj_cmd_ok(&repo_path, &["undo"]); - let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "descendants(c)"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "descendants(d)"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Abandoned the following commits: znkkpsqq 5557ece3 e | e vruxwmqv b7c62f28 d | d - royxmykx fe2e8e8b c | c - Working copy now at: xtnwkqum e7bb0612 (empty) (no description set) + Working copy now at: xtnwkqum fa4ee8e6 (empty) (no description set) Parent commit : rlvkpnrz 2443ea76 a e?? | a - Added 0 files, modified 0 files, removed 3 files + Parent commit : royxmykx fe2e8e8b c d e?? | c + Added 0 files, modified 0 files, removed 2 files "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ [xtn] - │ ◉ [zsu] b + @ [xtn] + ├─╮ + │ ◉ [roy] c d e?? + │ │ ◉ [zsu] b + ├───╯ + ◉ │ [rlv] a e?? ├─╯ - ◉ [rlv] a e?? - ◉ [zzz] c d e?? + ◉ [zzz] "###); // Test abandoning the same commit twice directly @@ -132,23 +137,26 @@ fn test_basics() { // Test abandoning the same commit twice indirectly test_env.jj_cmd_ok(&repo_path, &["undo"]); - let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "d::", "a::"]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "d::", "e"]); insta::assert_snapshot!(stdout, @""); insta::assert_snapshot!(stderr, @r###" Abandoned the following commits: znkkpsqq 5557ece3 e | e vruxwmqv b7c62f28 d | d - zsuskuln 1394f625 b | b - rlvkpnrz 2443ea76 a | a - Working copy now at: xlzxqlsl af874bff (empty) (no description set) - Parent commit : zzzzzzzz 00000000 a b e?? | (empty) (no description set) - Added 0 files, modified 0 files, removed 4 files + Working copy now at: xlzxqlsl 14991aec (empty) (no description set) + Parent commit : rlvkpnrz 2443ea76 a e?? | a + Parent commit : royxmykx fe2e8e8b c d e?? | c + Added 0 files, modified 0 files, removed 2 files "###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" - @ [xlz] + @ [xlz] + ├─╮ │ ◉ [roy] c d e?? + │ │ ◉ [zsu] b + ├───╯ + ◉ │ [rlv] a e?? ├─╯ - ◉ [zzz] a b e?? + ◉ [zzz] "###); let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "none()"]); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 5e129a533..a624f0594 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -51,7 +51,7 @@ use crate::refs::{ diff_named_ref_targets, diff_named_remote_refs, merge_ref_targets, merge_remote_refs, }; use crate::revset::{RevsetEvaluationError, RevsetExpression, RevsetIteratorExt}; -use crate::rewrite::{CommitRewriter, DescendantRebaser, RebaseOptions}; +use crate::rewrite::{merge_commit_trees, CommitRewriter, DescendantRebaser, RebaseOptions}; use crate::settings::{RepoSettings, UserSettings}; use crate::signing::{SignInitError, Signer}; use crate::simple_op_heads_store::SimpleOpHeadsStore; @@ -1027,7 +1027,6 @@ impl MutableRepo { old_commit_id: CommitId, new_commit_ids: Vec, ) -> BackendResult<()> { - // We arbitrarily pick a new working-copy commit among the candidates. let abandoned_old_commit = matches!( self.parent_mapping.get(&old_commit_id), Some(Rewrite::Abandoned(_)) @@ -1035,7 +1034,7 @@ impl MutableRepo { self.update_wc_commits( settings, &old_commit_id, - &new_commit_ids[0], + &new_commit_ids, abandoned_old_commit, )?; @@ -1082,7 +1081,7 @@ impl MutableRepo { &mut self, settings: &UserSettings, old_commit_id: &CommitId, - new_commit_id: &CommitId, + new_commit_ids: &[CommitId], abandoned_old_commit: bool, ) -> BackendResult<()> { let workspaces_to_update = self.view().workspaces_for_wc_commit_id(old_commit_id); @@ -1090,14 +1089,19 @@ impl MutableRepo { return Ok(()); } - let new_commit = self.store().get_commit(new_commit_id)?; let new_wc_commit = if !abandoned_old_commit { - new_commit + // We arbitrarily pick a new working-copy commit among the candidates. + self.store().get_commit(&new_commit_ids[0])? } else { + let new_commits: Vec<_> = new_commit_ids + .iter() + .map(|id| self.store().get_commit(id)) + .try_collect()?; + let merged_parents_tree = merge_commit_trees(self, &new_commits)?; self.new_commit( settings, - vec![new_commit.id().clone()], - new_commit.tree_id().clone(), + new_commit_ids.to_vec(), + merged_parents_tree.id().clone(), ) .write()? }; diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 682df0017..417bc38a0 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -1492,7 +1492,7 @@ fn test_rebase_descendants_update_checkout_abandoned_merge() { let test_repo = TestRepo::init(); let repo = &test_repo.repo; - // Checked-out merge commit D was abandoned. A parent commit should become + // Checked-out merge commit D was abandoned. A new merge commit should become // checked out. // // D @@ -1527,7 +1527,10 @@ fn test_rebase_descendants_update_checkout_abandoned_merge() { let new_checkout_id = repo.view().get_wc_commit_id(&workspace_id).unwrap(); let checkout = repo.store().get_commit(new_checkout_id).unwrap(); - assert_eq!(checkout.parent_ids(), vec![commit_b.id().clone()]); + assert_eq!( + checkout.parent_ids(), + vec![commit_b.id().clone(), commit_c.id().clone()] + ); } #[test_case(EmptyBehaviour::Keep; "keep all commits")]