diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index a96f5bbd5..5c3d0a6d3 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -154,7 +154,7 @@ pub fn rebase_commit_with_options( EmptyBehaviour::Keep => false, EmptyBehaviour::AbandonNewlyEmpty => { *parent.tree_id() == new_tree_id - && old_base_tree_id != Some(old_commit.tree_id().clone()) + && old_base_tree_id.map_or(false, |id| id != *old_commit.tree_id()) } EmptyBehaviour::AbandonAllEmpty => *parent.tree_id() == new_tree_id, }; diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index c0ec951d1..cae5c35e7 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -1490,7 +1490,7 @@ fn assert_rebase_skipped( rebased: Option, expected_old_commit: &Commit, expected_new_commit: &Commit, -) { +) -> Commit { if let Some(RebasedDescendant { old_commit, new_commit, @@ -1500,6 +1500,7 @@ fn assert_rebase_skipped( 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:?}"); } @@ -1517,11 +1518,13 @@ fn test_empty_commit_option(empty: EmptyBehaviour) { // actual changes. // // BD (commit B joined with commit D) + // | H (empty, no parent tree changes) + // | | // | G // | | // | F (clean merge) // | /|\ - // | C D E (empty) + // | C D E (empty, but parent tree changes) // | \|/ // | B // A__/ @@ -1567,6 +1570,7 @@ fn test_empty_commit_option(empty: EmptyBehaviour) { let commit_e = create_commit(&[&commit_b], &tree_b); let commit_f = create_commit(&[&commit_c, &commit_d, &commit_e], &tree_f); let commit_g = create_commit(&[&commit_f], &tree_g); + let commit_h = create_commit(&[&commit_g], &tree_g); let commit_bd = create_commit(&[&commit_a], &tree_d); let mut rebaser = DescendantRebaser::new( @@ -1595,7 +1599,9 @@ fn test_empty_commit_option(empty: EmptyBehaviour) { &commit_f, &[&new_commit_c, &new_commit_d, &new_commit_e], ); - assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_f]) + 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]) } EmptyBehaviour::AbandonAllEmpty => { // The commit C isn't empty. @@ -1606,7 +1612,9 @@ fn test_empty_commit_option(empty: EmptyBehaviour) { 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_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_c]) + 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) } EmptyBehaviour::AbandonNewlyEmpty => { // The commit C isn't empty. @@ -1625,12 +1633,14 @@ fn test_empty_commit_option(empty: EmptyBehaviour) { &commit_f, &[&new_commit_c, &new_commit_e], ); - assert_rebased(rebaser.rebase_next().unwrap(), &commit_g, &[&new_commit_f]) + 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!(rebaser.rebase_next().unwrap().is_none()); - assert_eq!(rebaser.rebased().len(), 5); + assert_eq!(rebaser.rebased().len(), 6); assert_eq!( *tx.mut_repo().view().heads(),