diff --git a/lib/src/repo.rs b/lib/src/repo.rs index e7bf6e41b4..a6a0aba055 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -859,10 +859,6 @@ impl MutableRepo { .insert(new_id); } - pub fn clear_rewritten_commits(&mut self) { - self.rewritten_commits.clear(); - } - /// Record a commit as having been abandoned in this transaction. /// /// This record is used by `rebase_descendants` to know which commits have @@ -877,7 +873,8 @@ impl MutableRepo { self.abandoned_commits.insert(old_id); } - pub fn clear_abandoned_commits(&mut self) { + fn clear_descendant_rebaser_plans(&mut self) { + self.rewritten_commits.clear(); self.abandoned_commits.clear(); } @@ -885,21 +882,8 @@ impl MutableRepo { !(self.rewritten_commits.is_empty() && self.abandoned_commits.is_empty()) } - /// Creates a `DescendantRebaser` to rebase descendants of the recorded - /// rewritten and abandoned commits. - // TODO(ilyagr): Inline this. It's only used in tests. - pub fn create_descendant_rebaser<'settings, 'repo>( - &'repo mut self, - settings: &'settings UserSettings, - ) -> DescendantRebaser<'settings, 'repo> { - DescendantRebaser::new( - settings, - self, - self.rewritten_commits.clone(), - self.abandoned_commits.clone(), - ) - } - + /// After the rebaser returned by this function is dropped, + /// self.clear_descendant_rebaser_plans() needs to be called. fn rebase_descendants_return_rebaser<'settings, 'repo>( &'repo mut self, settings: &'settings UserSettings, @@ -909,7 +893,12 @@ impl MutableRepo { // Optimization return Ok(None); } - let mut rebaser = self.create_descendant_rebaser(settings); + let mut rebaser = DescendantRebaser::new( + settings, + self, + self.rewritten_commits.clone(), + self.abandoned_commits.clone(), + ); *rebaser.mut_options() = options; rebaser.rebase_all()?; Ok(Some(rebaser)) @@ -923,9 +912,39 @@ impl MutableRepo { settings: &UserSettings, options: RebaseOptions, ) -> Result { - Ok(self + let result = self + .rebase_descendants_return_rebaser(settings, options)? + .map_or(0, |rebaser| rebaser.rebased().len()); + self.clear_descendant_rebaser_plans(); + Ok(result) + } + + /// This is similar to `rebase_descendants_return_map`, but the return value + /// needs more explaining. + /// + /// If the `options.empty` is the default, this function will only + /// rebase commits, and the return value is what you'd expect it to be. + /// + /// Otherwise, this function may rebase some commits and abandon others. The + /// behavior is such that only commits with a single parent will ever be + /// abandoned. In the returned map, an abandoned commit will look as a + /// key-value pair where the key is the abandoned commit and the value is + /// **its parent**. One can tell this case apart since the change ids of the + /// key and the value will not match. The parent will inherit the + /// descendants and the branches of the abandoned commit. + pub fn rebase_descendants_with_options_return_map( + &mut self, + settings: &UserSettings, + options: RebaseOptions, + ) -> Result, TreeMergeError> { + let result = Ok(self + // We do not set RebaseOptions here, since this function does not currently return + // enough information to describe the results of a rebase if some commits got + // abandoned .rebase_descendants_return_rebaser(settings, options)? - .map_or(0, |rebaser| rebaser.rebased().len())) + .map_or(HashMap::new(), |rebaser| rebaser.rebased().clone())); + self.clear_descendant_rebaser_plans(); + result } pub fn rebase_descendants(&mut self, settings: &UserSettings) -> Result { @@ -936,12 +955,7 @@ impl MutableRepo { &mut self, settings: &UserSettings, ) -> Result, TreeMergeError> { - Ok(self - // We do not set RebaseOptions here, since this function does not currently return - // enough information to describe the results of a rebase if some commits got - // abandoned - .rebase_descendants_return_rebaser(settings, Default::default())? - .map_or(HashMap::new(), |rebaser| rebaser.rebased().clone())) + self.rebase_descendants_with_options_return_map(settings, Default::default()) } pub fn set_wc_commit( diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index f6d449b4ee..64c728470f 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -242,7 +242,7 @@ pub struct RebaseOptions { } /// Rebases descendants of a commit onto a new commit (or several). -pub struct DescendantRebaser<'settings, 'repo> { +pub(crate) struct DescendantRebaser<'settings, 'repo> { settings: &'settings UserSettings, mut_repo: &'repo mut MutableRepo, // The commit identified by the key has been replaced by all the ones in the value, typically @@ -521,7 +521,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { // TODO: Perhaps change the interface since it's not just about rebasing // commits. - pub fn rebase_next(&mut self) -> Result, TreeMergeError> { + fn rebase_next(&mut self) -> Result, TreeMergeError> { while let Some(old_commit) = self.to_visit.pop() { let old_commit_id = old_commit.id().clone(); if let Some(new_parent_ids) = self.parent_mapping.get(&old_commit_id).cloned() { @@ -587,9 +587,12 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { new_commit, })); } - // TODO: As the TODO above says, we should probably change the API. Even if we - // don't, we should at least make this code not do any work if you call - // rebase_next() after we've returned None. + Ok(None) + } + + pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> { + while self.rebase_next()?.is_some() {} + // TODO: As the TODO above says, we should probably change the API. let mut view = self.mut_repo.view().store_view().clone(); for commit_id in &self.heads_to_remove { view.head_ids.remove(commit_id); @@ -600,13 +603,6 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { self.heads_to_remove.clear(); self.heads_to_add.clear(); self.mut_repo.set_view(view); - self.mut_repo.clear_rewritten_commits(); - self.mut_repo.clear_abandoned_commits(); - Ok(None) - } - - pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> { - while self.rebase_next()?.is_some() {} Ok(()) } } diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index 4d38162737..1269fb79f2 100644 --- a/lib/tests/test_commit_builder.rs +++ b/lib/tests/test_commit_builder.rs @@ -19,7 +19,7 @@ use jj_lib::repo::Repo; use jj_lib::repo_path::{RepoPath, RepoPathBuf}; use jj_lib::settings::UserSettings; use test_case::test_case; -use testutils::{assert_rebased, create_tree, CommitGraphBuilder, TestRepo, TestRepoBackend}; +use testutils::{assert_rebased_onto, create_tree, CommitGraphBuilder, TestRepo, TestRepoBackend}; fn to_owned_path_vec(paths: &[&RepoPath]) -> Vec { paths.iter().map(|&path| path.to_owned()).collect() @@ -269,8 +269,11 @@ fn test_commit_builder_descendants(backend: TestRepoBackend) { ) .write() .unwrap(); - let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); - assert!(rebaser.rebase_next().unwrap().is_none()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_eq!(rebase_map.len(), 0); // Test with for_rewrite_from() let mut tx = repo.start_transaction(&settings); @@ -279,9 +282,12 @@ fn test_commit_builder_descendants(backend: TestRepoBackend) { .rewrite_commit(&settings, &commit2) .write() .unwrap(); - let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); - assert_rebased(rebaser.rebase_next().unwrap(), &commit3, &[&commit4]); - assert!(rebaser.rebase_next().unwrap().is_none()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit3, &[commit4.id()]); + assert_eq!(rebase_map.len(), 1); // Test with for_rewrite_from() but new change id let mut tx = repo.start_transaction(&settings); @@ -290,6 +296,9 @@ fn test_commit_builder_descendants(backend: TestRepoBackend) { .generate_new_change_id() .write() .unwrap(); - let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); - assert!(rebaser.rebase_next().unwrap().is_none()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert!(rebase_map.is_empty()); } diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index c28e8785f5..1533f2fd8e 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -17,7 +17,7 @@ use jj_lib::op_store::{RefTarget, RemoteRef, RemoteRefState, WorkspaceId}; use jj_lib::repo::Repo; use maplit::hashset; use testutils::{ - assert_rebased, create_random_commit, write_random_commit, CommitGraphBuilder, TestRepo, + assert_rebased_onto, create_random_commit, write_random_commit, CommitGraphBuilder, TestRepo, }; #[test] @@ -517,9 +517,7 @@ fn test_has_changed() { #[test] fn test_rebase_descendants_simple() { - // Tests that MutableRepo::create_descendant_rebaser() creates a - // DescendantRebaser that rebases descendants of rewritten and abandoned - // commits. + // There are many additional tests of this functionality in `test_rewrite.rs`. let settings = testutils::user_settings(); let test_repo = TestRepo::init(); let repo = &test_repo.repo; @@ -539,24 +537,29 @@ fn test_rebase_descendants_simple() { let commit6 = graph_builder.commit_with_parents(&[&commit1]); mut_repo.record_rewritten_commit(commit2.id().clone(), commit6.id().clone()); mut_repo.record_abandoned_commit(commit4.id().clone()); - let mut rebaser = mut_repo.create_descendant_rebaser(&settings); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); // Commit 3 got rebased onto commit 2's replacement, i.e. commit 6 - assert_rebased(rebaser.rebase_next().unwrap(), &commit3, &[&commit6]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit3, &[commit6.id()]); // Commit 5 got rebased onto commit 4's parent, i.e. commit 1 - assert_rebased(rebaser.rebase_next().unwrap(), &commit5, &[&commit1]); - assert!(rebaser.rebase_next().unwrap().is_none()); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit5, &[commit1.id()]); + assert_eq!(rebase_map.len(), 2); + // No more descendants to rebase if we try again. - assert!(mut_repo - .create_descendant_rebaser(&settings) - .rebase_next() - .unwrap() - .is_none()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_eq!(rebase_map.len(), 0); } #[test] fn test_rebase_descendants_conflicting_rewrite() { - // Tests MutableRepo::create_descendant_rebaser() when a commit has been marked - // as rewritten to several other commits. + // Test rebasing descendants when one commit was rewritten to several other + // commits. There are many additional tests of this functionality in + // `test_rewrite.rs`. let settings = testutils::user_settings(); let test_repo = TestRepo::init(); let repo = &test_repo.repo; @@ -575,16 +578,13 @@ fn test_rebase_descendants_conflicting_rewrite() { let commit5 = graph_builder.commit_with_parents(&[&commit1]); mut_repo.record_rewritten_commit(commit2.id().clone(), commit4.id().clone()); mut_repo.record_rewritten_commit(commit2.id().clone(), commit5.id().clone()); - let mut rebaser = mut_repo.create_descendant_rebaser(&settings); // Commit 3 does *not* get rebased because it's unclear if it should go onto // commit 4 or commit 5 - assert!(rebaser.rebase_next().unwrap().is_none()); - // No more descendants to rebase if we try again. - assert!(mut_repo - .create_descendant_rebaser(&settings) - .rebase_next() - .unwrap() - .is_none()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert!(rebase_map.is_empty()); } #[test] diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 7e278d6d1e..67882a1ba2 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -19,14 +19,12 @@ use jj_lib::merged_tree::MergedTree; use jj_lib::op_store::{RefTarget, RemoteRef, RemoteRefState, WorkspaceId}; use jj_lib::repo::Repo; use jj_lib::repo_path::RepoPath; -use jj_lib::rewrite::{ - restore_tree, DescendantRebaser, EmptyBehaviour, RebaseOptions, RebasedDescendant, -}; +use jj_lib::rewrite::{restore_tree, EmptyBehaviour, RebaseOptions}; use maplit::{hashmap, hashset}; use test_case::test_case; use testutils::{ - assert_rebased, create_random_commit, create_tree, write_random_commit, CommitGraphBuilder, - TestRepo, + assert_abandoned_with_parent, assert_rebased_onto, create_random_commit, create_tree, + write_random_commit, CommitGraphBuilder, TestRepo, }; #[test] @@ -87,19 +85,17 @@ fn test_rebase_descendants_sideways() { let commit_e = graph_builder.commit_with_parents(&[&commit_b]); let commit_f = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_f.id().clone()} - }, - hashset! {}, - ); - let new_commit_c = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_f]); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&new_commit_c]); - let new_commit_e = assert_rebased(rebaser.rebase_next().unwrap(), &commit_e, &[&commit_f]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 3); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_eq!(rebase_map.len(), 3); + let new_commit_c = assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[commit_f.id()]); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[new_commit_c.id()]); + let new_commit_e = assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_e, &[commit_f.id()]); assert_eq!( *tx.mut_repo().view().heads(), @@ -143,21 +139,23 @@ fn test_rebase_descendants_forward() { let commit_f = graph_builder.commit_with_parents(&[&commit_d]); let commit_g = graph_builder.commit_with_parents(&[&commit_f]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_f.id().clone()} - }, - hashset! {}, - ); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_f]); - let new_commit_f = assert_rebased(rebaser.rebase_next().unwrap(), &commit_f, &[&new_commit_d]); - let new_commit_c = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&new_commit_f]); - let new_commit_e = assert_rebased(rebaser.rebase_next().unwrap(), &commit_e, &[&new_commit_d]); - let new_commit_g = assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_f]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 5); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&commit_f.id()]); + let new_commit_f = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_f, &[&new_commit_d.id()]); + let new_commit_c = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[&new_commit_f.id()]); + let new_commit_e = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_e, &[&new_commit_d.id()]); + let new_commit_g = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_g, &[&new_commit_f.id()]); + assert_eq!(rebase_map.len(), 5); assert_eq!( *tx.mut_repo().view().heads(), @@ -198,19 +196,19 @@ fn test_rebase_descendants_reorder() { let commit_h = graph_builder.commit_with_parents(&[&commit_f]); let commit_i = graph_builder.commit_with_parents(&[&commit_g]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_e.id().clone() => hashset!{commit_d.id().clone()}, - commit_c.id().clone() => hashset!{commit_f.id().clone()}, - commit_g.id().clone() => hashset!{commit_h.id().clone()}, - }, - hashset! {}, - ); - let new_commit_i = assert_rebased(rebaser.rebase_next().unwrap(), &commit_i, &[&commit_h]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + tx.mut_repo() + .record_rewritten_commit(commit_e.id().clone(), commit_d.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_g.id().clone(), commit_h.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_i = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_i, &[&commit_h.id()]); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -239,17 +237,15 @@ fn test_rebase_descendants_backward() { let commit_c = graph_builder.commit_with_parents(&[&commit_b]); let commit_d = graph_builder.commit_with_parents(&[&commit_c]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_c.id().clone() => hashset!{commit_b.id().clone()} - }, - hashset! {}, - ); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_b]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + tx.mut_repo() + .record_rewritten_commit(commit_c.id().clone(), commit_b.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&commit_b.id()]); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -282,19 +278,19 @@ fn test_rebase_descendants_chain_becomes_branchy() { let commit_e = graph_builder.commit_with_parents(&[&commit_a]); let commit_f = graph_builder.commit_with_parents(&[&commit_b]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_e.id().clone()}, - commit_c.id().clone() => hashset!{commit_f.id().clone()}, - }, - hashset! {}, - ); - let new_commit_f = assert_rebased(rebaser.rebase_next().unwrap(), &commit_f, &[&commit_e]); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&new_commit_f]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 2); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_e.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_f = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_f, &[&commit_e.id()]); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&new_commit_f.id()]); + assert_eq!(rebase_map.len(), 2); assert_eq!( *tx.mut_repo().view().heads(), @@ -329,23 +325,23 @@ fn test_rebase_descendants_internal_merge() { let commit_e = graph_builder.commit_with_parents(&[&commit_c, &commit_d]); let commit_f = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_c = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[&commit_f.id()]); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&commit_f.id()]); + let new_commit_e = assert_rebased_onto( tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_f.id().clone()} - }, - hashset! {}, - ); - let new_commit_c = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_f]); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_f]); - let new_commit_e = assert_rebased( - rebaser.rebase_next().unwrap(), + &rebase_map, &commit_e, - &[&new_commit_c, &new_commit_d], + &[&new_commit_c.id(), &new_commit_d.id()], ); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 3); + assert_eq!(rebase_map.len(), 3); assert_eq!( *tx.mut_repo().view().heads(), @@ -379,21 +375,19 @@ fn test_rebase_descendants_external_merge() { let commit_e = graph_builder.commit_with_parents(&[&commit_c, &commit_d]); let commit_f = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, + tx.mut_repo() + .record_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_e = assert_rebased_onto( tx.mut_repo(), - hashmap! { - commit_c.id().clone() => hashset!{commit_f.id().clone()} - }, - hashset! {}, - ); - let new_commit_e = assert_rebased( - rebaser.rebase_next().unwrap(), + &rebase_map, &commit_e, - &[&commit_f, &commit_d], + &[&commit_f.id(), &commit_d.id()], ); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -425,17 +419,19 @@ fn test_rebase_descendants_abandon() { let commit_e = graph_builder.commit_with_parents(&[&commit_d]); let commit_f = graph_builder.commit_with_parents(&[&commit_e]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! {}, - hashset! {commit_b.id().clone(), commit_e.id().clone()}, - ); - let new_commit_c = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_a]); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_a]); - let new_commit_f = assert_rebased(rebaser.rebase_next().unwrap(), &commit_f, &[&new_commit_d]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 3); + tx.mut_repo().record_abandoned_commit(commit_b.id().clone()); + tx.mut_repo().record_abandoned_commit(commit_e.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_c = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[&commit_a.id()]); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&commit_a.id()]); + let new_commit_f = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_f, &[&new_commit_d.id()]); + assert_eq!(rebase_map.len(), 3); assert_eq!( *tx.mut_repo().view().heads(), @@ -463,14 +459,13 @@ fn test_rebase_descendants_abandon_no_descendants() { let commit_b = graph_builder.commit_with_parents(&[&commit_a]); let commit_c = graph_builder.commit_with_parents(&[&commit_b]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! {}, - hashset! {commit_b.id().clone(), commit_c.id().clone()}, - ); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 0); + tx.mut_repo().record_abandoned_commit(commit_b.id().clone()); + tx.mut_repo().record_abandoned_commit(commit_c.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_eq!(rebase_map.len(), 0); assert_eq!( *tx.mut_repo().view().heads(), @@ -502,15 +497,16 @@ fn test_rebase_descendants_abandon_and_replace() { let commit_d = graph_builder.commit_with_parents(&[&commit_c]); let commit_e = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! {commit_b.id().clone() => hashset!{commit_e.id().clone()}}, - hashset! {commit_c.id().clone()}, - ); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_e]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_e.id().clone()); + tx.mut_repo().record_abandoned_commit(commit_c.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&commit_e.id()]); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -518,6 +514,7 @@ fn test_rebase_descendants_abandon_and_replace() { ); } +// TODO(#2600): This behavior may need to change #[test] fn test_rebase_descendants_abandon_degenerate_merge() { let settings = testutils::user_settings(); @@ -539,15 +536,14 @@ fn test_rebase_descendants_abandon_degenerate_merge() { let commit_c = graph_builder.commit_with_parents(&[&commit_a]); let commit_d = graph_builder.commit_with_parents(&[&commit_b, &commit_c]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! {}, - hashset! {commit_b.id().clone()}, - ); - let new_commit_d = assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_c]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + tx.mut_repo().record_abandoned_commit(commit_b.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_d = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[&commit_c.id()]); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -580,19 +576,18 @@ fn test_rebase_descendants_abandon_widen_merge() { let commit_e = graph_builder.commit_with_parents(&[&commit_b, &commit_c]); let commit_f = graph_builder.commit_with_parents(&[&commit_e, &commit_d]); - let mut rebaser = DescendantRebaser::new( - &settings, + tx.mut_repo().record_abandoned_commit(commit_e.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_f = assert_rebased_onto( tx.mut_repo(), - hashmap! {}, - hashset! {commit_e.id().clone()}, - ); - let new_commit_f = assert_rebased( - rebaser.rebase_next().unwrap(), + &rebase_map, &commit_f, - &[&commit_b, &commit_c, &commit_d], + &[&commit_b.id(), &commit_c.id(), &commit_d.id()], ); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -623,19 +618,19 @@ fn test_rebase_descendants_multiple_sideways() { let commit_e = graph_builder.commit_with_parents(&[&commit_d]); let commit_f = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_f.id().clone()}, - commit_d.id().clone() => hashset!{commit_f.id().clone()}, - }, - hashset! {}, - ); - let new_commit_c = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_f]); - let new_commit_e = assert_rebased(rebaser.rebase_next().unwrap(), &commit_e, &[&commit_f]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 2); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_d.id().clone(), commit_f.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_c = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[&commit_f.id()]); + let new_commit_e = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_e, &[&commit_f.id()]); + assert_eq!(rebase_map.len(), 2); assert_eq!( *tx.mut_repo().view().heads(), @@ -668,18 +663,15 @@ fn test_rebase_descendants_multiple_swap() { let commit_d = graph_builder.commit_with_parents(&[&commit_a]); let _commit_e = graph_builder.commit_with_parents(&[&commit_d]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_d.id().clone()}, - commit_d.id().clone() => hashset!{commit_b.id().clone()}, - }, - hashset! {}, - ); - let _ = rebaser.rebase_next(); // Panics because of the cycle + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_d.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_d.id().clone(), commit_b.id().clone()); + let _ = tx.mut_repo().rebase_descendants_return_map(&settings); // Panics because of the cycle } +// Unlike `test_rebase_descendants_multiple_swap`, this does not currently +// panic, but it would probably be OK if it did. #[test] fn test_rebase_descendants_multiple_no_descendants() { let settings = testutils::user_settings(); @@ -697,17 +689,15 @@ fn test_rebase_descendants_multiple_no_descendants() { let commit_b = graph_builder.commit_with_parents(&[&commit_a]); let commit_c = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_c.id().clone()}, - commit_c.id().clone() => hashset!{commit_b.id().clone()}, - }, - hashset! {}, - ); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert!(rebaser.rebased().is_empty()); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_c.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_c.id().clone(), commit_b.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert!(rebase_map.is_empty()); assert_eq!( *tx.mut_repo().view().heads(), @@ -758,20 +748,24 @@ fn test_rebase_descendants_divergent_rewrite() { let commit_d3 = graph_builder.commit_with_parents(&[&commit_a]); let commit_f2 = graph_builder.commit_with_parents(&[&commit_a]); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_b2.id().clone()}, - commit_d.id().clone() => hashset!{commit_d2.id().clone(), commit_d3.id().clone()}, - commit_f.id().clone() => hashset!{commit_f2.id().clone()}, - }, - hashset! {}, - ); - let new_commit_c = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_b2]); - let new_commit_g = assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&commit_f2]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 2); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_b2.id().clone()); + // Commit D becomes divergent + tx.mut_repo() + .record_rewritten_commit(commit_d.id().clone(), commit_d2.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_d.id().clone(), commit_d3.id().clone()); + tx.mut_repo() + .record_rewritten_commit(commit_f.id().clone(), commit_f2.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let new_commit_c = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[commit_b2.id()]); + let new_commit_g = + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_g, &[&commit_f2.id()]); + assert_eq!(rebase_map.len(), 2); // Commit E is not rebased assert_eq!( *tx.mut_repo().view().heads(), @@ -814,10 +808,12 @@ fn test_rebase_descendants_repeated() { .set_description("b2") .write() .unwrap(); - let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); - let commit_c2 = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_b2]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let commit_c2 = assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[commit_b2.id()]); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -827,9 +823,11 @@ fn test_rebase_descendants_repeated() { ); // We made no more changes, so nothing should be rebased. - let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 0); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_eq!(rebase_map.len(), 0); // Now mark B3 as rewritten from B2 and rebase descendants again. let commit_b3 = tx @@ -838,10 +836,12 @@ fn test_rebase_descendants_repeated() { .set_description("b3") .write() .unwrap(); - let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings); - let commit_c3 = assert_rebased(rebaser.rebase_next().unwrap(), &commit_c2, &[&commit_b3]); - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 1); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + let commit_c3 = assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c2, &[commit_b3.id()]); + assert_eq!(rebase_map.len(), 1); assert_eq!( *tx.mut_repo().view().heads(), @@ -900,20 +900,16 @@ fn test_rebase_descendants_contents() { .write() .unwrap(); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_d.id().clone()} - }, - hashset! {}, - ); - rebaser.rebase_all().unwrap(); - let rebased = rebaser.rebased(); - assert_eq!(rebased.len(), 1); + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_d.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_return_map(&settings) + .unwrap(); + assert_eq!(rebase_map.len(), 1); let new_commit_c = repo .store() - .get_commit(rebased.get(commit_c.id()).unwrap()) + .get_commit(rebase_map.get(commit_c.id()).unwrap()) .unwrap(); let tree_b = commit_b.tree().unwrap(); @@ -1114,18 +1110,21 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() { // Branch "main" points to commit B. B gets rewritten as B2, B3, B4. Branch main // should become a conflict pointing to all of them. // - // B4 main? - // | B3 main? + // C other + // C other | B4 main? + // | |/B3 main? // B main |/B2 main? // | => |/ // A A - // TODO(ilyagr): Check what happens if B had a descendant with a branch on it. let mut tx = repo.start_transaction(&settings); let mut graph_builder = CommitGraphBuilder::new(&settings, tx.mut_repo()); let commit_a = graph_builder.initial_commit(); let commit_b = graph_builder.commit_with_parents(&[&commit_a]); + let commit_c = graph_builder.commit_with_parents(&[&commit_b]); tx.mut_repo() .set_local_branch_target("main", RefTarget::normal(commit_b.id().clone())); + tx.mut_repo() + .set_local_branch_target("other", RefTarget::normal(commit_c.id().clone())); let repo = tx.commit("test"); let mut tx = repo.start_transaction(&settings); @@ -1150,14 +1149,14 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() { .unwrap(); tx.mut_repo().rebase_descendants(&settings).unwrap(); - let target = tx.mut_repo().get_local_branch("main"); - assert!(target.has_conflict()); + let main_target = tx.mut_repo().get_local_branch("main"); + assert!(main_target.has_conflict()); assert_eq!( - target.removed_ids().counts(), + main_target.removed_ids().counts(), hashmap! { commit_b.id() => 2 }, ); assert_eq!( - target.added_ids().counts(), + main_target.added_ids().counts(), hashmap! { commit_b2.id() => 1, commit_b3.id() => 1, @@ -1165,12 +1164,16 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() { }, ); + let other_target = tx.mut_repo().get_local_branch("other"); + assert_eq!(other_target.as_normal(), Some(commit_c.id())); + assert_eq!( *tx.mut_repo().view().heads(), hashset! { commit_b2.id().clone(), commit_b3.id().clone(), commit_b4.id().clone(), + commit_c.id().clone(), } ); } @@ -1469,30 +1472,10 @@ fn test_rebase_descendants_update_checkout_abandoned_merge() { assert_eq!(checkout.parent_ids(), vec![commit_b.id().clone()]); } -fn assert_rebase_skipped( - rebased: Option, - expected_old_commit: &Commit, - expected_new_commit: &Commit, -) -> Commit { - if let Some(RebasedDescendant { - old_commit, - new_commit, - }) = rebased - { - assert_eq!(old_commit, *expected_old_commit,); - assert_eq!(new_commit, *expected_new_commit); - // Since it was abandoned, the change ID should be different. - assert_ne!(old_commit.change_id(), new_commit.change_id()); - new_commit - } else { - panic!("expected rebased commit: {rebased:?}"); - } -} - #[test_case(EmptyBehaviour::Keep; "keep all commits")] #[test_case(EmptyBehaviour::AbandonNewlyEmpty; "abandon newly empty commits")] #[test_case(EmptyBehaviour::AbandonAllEmpty ; "abandon all empty commits")] -fn test_empty_commit_option(empty: EmptyBehaviour) { +fn test_empty_commit_option(empty_behavior: EmptyBehaviour) { let settings = testutils::user_settings(); let test_repo = TestRepo::init(); let repo = &test_repo.repo; @@ -1552,74 +1535,75 @@ fn test_empty_commit_option(empty: EmptyBehaviour) { let commit_h = create_commit(&[&commit_g], &tree_g); let commit_bd = create_commit(&[&commit_a], &tree_d); - let mut rebaser = DescendantRebaser::new( - &settings, - tx.mut_repo(), - hashmap! { - commit_b.id().clone() => hashset!{commit_bd.id().clone()} - }, - hashset! {}, - ); - *rebaser.mut_options() = RebaseOptions { - empty: empty.clone(), - }; + tx.mut_repo() + .record_rewritten_commit(commit_b.id().clone(), commit_bd.id().clone()); + let rebase_map = tx + .mut_repo() + .rebase_descendants_with_options_return_map( + &settings, + RebaseOptions { + empty: empty_behavior.clone(), + }, + ) + .unwrap(); - let new_head = match empty { + let new_head = match empty_behavior { EmptyBehaviour::Keep => { // The commit C isn't empty. let new_commit_c = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_bd]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[commit_bd.id()]); let new_commit_d = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_d, &[&commit_bd]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_d, &[commit_bd.id()]); let new_commit_e = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_e, &[&commit_bd]); - let new_commit_f = assert_rebased( - rebaser.rebase_next().unwrap(), + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_e, &[commit_bd.id()]); + let new_commit_f = assert_rebased_onto( + tx.mut_repo(), + &rebase_map, &commit_f, - &[&new_commit_c, &new_commit_d, &new_commit_e], + &[&new_commit_c.id(), &new_commit_d.id(), &new_commit_e.id()], ); let new_commit_g = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_f]); - assert_rebased(rebaser.rebase_next().unwrap(), &commit_h, &[&new_commit_g]) + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_g, &[new_commit_f.id()]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_h, &[new_commit_g.id()]) } EmptyBehaviour::AbandonAllEmpty => { // The commit C isn't empty. let new_commit_c = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_bd]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[commit_bd.id()]); // D and E are empty, and F is a clean merge with only one child. Thus, F is // also considered empty. - assert_rebase_skipped(rebaser.rebase_next().unwrap(), &commit_d, &commit_bd); - assert_rebase_skipped(rebaser.rebase_next().unwrap(), &commit_e, &commit_bd); - assert_rebase_skipped(rebaser.rebase_next().unwrap(), &commit_f, &new_commit_c); + assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_d, commit_bd.id()); + assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_e, commit_bd.id()); + assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_f, new_commit_c.id()); let new_commit_g = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_c]); - assert_rebase_skipped(rebaser.rebase_next().unwrap(), &commit_h, &new_commit_g) + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_g, &[new_commit_c.id()]); + assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_h, new_commit_g.id()) } EmptyBehaviour::AbandonNewlyEmpty => { // The commit C isn't empty. let new_commit_c = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_c, &[&commit_bd]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_c, &[commit_bd.id()]); // The changes in D are included in BD, so D is newly empty. - assert_rebase_skipped(rebaser.rebase_next().unwrap(), &commit_d, &commit_bd); + assert_abandoned_with_parent(tx.mut_repo(), &rebase_map, &commit_d, commit_bd.id()); // E was already empty, so F is a merge commit with C and E as parents. // Although it's empty, we still keep it because we don't want to drop merge // commits. let new_commit_e = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_e, &[&commit_bd]); - let new_commit_f = assert_rebased( - rebaser.rebase_next().unwrap(), + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_e, &[commit_bd.id()]); + let new_commit_f = assert_rebased_onto( + tx.mut_repo(), + &rebase_map, &commit_f, - &[&new_commit_c, &new_commit_e], + &[&new_commit_c.id(), &new_commit_e.id()], ); let new_commit_g = - assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_f]); - assert_rebased(rebaser.rebase_next().unwrap(), &commit_h, &[&new_commit_g]) + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_g, &[&new_commit_f.id()]); + assert_rebased_onto(tx.mut_repo(), &rebase_map, &commit_h, &[&new_commit_g.id()]) } }; - assert!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 6); + assert_eq!(rebase_map.len(), 6); assert_eq!( *tx.mut_repo().view().heads(), diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 7fd1f1fd0c..e8d998401f 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::collections::HashMap; use std::env; use std::fs::{self, OpenOptions}; use std::io::{Read, Write}; @@ -20,8 +21,8 @@ use std::sync::{Arc, Once}; use itertools::Itertools; use jj_lib::backend::{ - self, Backend, BackendInitError, ChangeId, FileId, MergedTreeId, MillisSinceEpoch, ObjectId, - Signature, Timestamp, TreeValue, + self, Backend, BackendInitError, ChangeId, CommitId, FileId, MergedTreeId, MillisSinceEpoch, + ObjectId, Signature, Timestamp, TreeValue, }; use jj_lib::commit::Commit; use jj_lib::commit_builder::CommitBuilder; @@ -30,7 +31,6 @@ use jj_lib::local_backend::LocalBackend; use jj_lib::merged_tree::MergedTree; use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo, RepoLoader, StoreFactories}; use jj_lib::repo_path::{RepoPath, RepoPathBuf}; -use jj_lib::rewrite::RebasedDescendant; use jj_lib::settings::UserSettings; use jj_lib::signing::Signer; use jj_lib::store::Store; @@ -452,27 +452,55 @@ impl<'settings, 'repo> CommitGraphBuilder<'settings, 'repo> { } } -pub fn assert_rebased( - rebased: Option, +fn assert_in_rebased_map( + repo: &impl Repo, + rebased: &HashMap, expected_old_commit: &Commit, - expected_new_parents: &[&Commit], ) -> Commit { - if let Some(RebasedDescendant { - old_commit, - new_commit, - }) = rebased - { - assert_eq!(old_commit, *expected_old_commit); - assert_eq!(new_commit.change_id(), expected_old_commit.change_id()); - assert_eq!( - new_commit.parent_ids(), - expected_new_parents - .iter() - .map(|commit| commit.id().clone()) - .collect_vec() - ); - new_commit - } else { - panic!("expected rebased commit: {rebased:?}"); - } + let new_commit_id = rebased.get(expected_old_commit.id()).unwrap_or_else(|| { + panic!( + "Expected commit to have been rebased: {}", + expected_old_commit.id().hex() + ) + }); + let new_commit = repo.store().get_commit(new_commit_id).unwrap().clone(); + new_commit +} + +pub fn assert_rebased_onto( + repo: &impl Repo, + rebased: &HashMap, + expected_old_commit: &Commit, + expected_new_parent_ids: &[&CommitId], +) -> Commit { + let new_commit = assert_in_rebased_map(repo, rebased, expected_old_commit); + assert_eq!( + new_commit.parent_ids().to_vec(), + expected_new_parent_ids + .iter() + .map(|x| (*x).clone()) + .collect_vec() + ); + assert_eq!(new_commit.change_id(), expected_old_commit.change_id()); + new_commit +} + +/// If `expected_old_commit` was abandoned, the `rebased` map indicates the +/// commit the children of `expected_old_commit` should be rebased to, which +/// would have a different change id. This happens when the EmptyBehavior in +/// RebaseOptions is not the default; because of the details of the +/// implementation this returned parent commit is always singular. +pub fn assert_abandoned_with_parent( + repo: &impl Repo, + rebased: &HashMap, + expected_old_commit: &Commit, + expected_new_parent_id: &CommitId, +) -> Commit { + let new_parent_commit = assert_in_rebased_map(repo, rebased, expected_old_commit); + assert_eq!(new_parent_commit.id(), expected_new_parent_id); + assert_ne!( + new_parent_commit.change_id(), + expected_old_commit.change_id() + ); + new_parent_commit }