diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 95c73ae7a..66589e510 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -144,9 +144,7 @@ don't make any changes, then the operation will be aborted. .set_description(second_description) .write()?; - // Currently, `rebase_descendents` would treat `commit` as being rewritten to - // *both* `first_commit` and `second_commit`, as if it was becoming divergent. - // However, we want only the `second_commit` to inherit `commit`'s branches and + // We want only the `second_commit` to inherit `commit`'s branches and // descendants. tx.mut_repo() .set_rewritten_commit(commit.id().clone(), second_commit.id().clone()); diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index c4fd0df0f..926ad322e 100644 --- a/lib/src/commit_builder.rs +++ b/lib/src/commit_builder.rs @@ -198,7 +198,7 @@ impl CommitBuilder<'_> { if let Some(rewrite_source) = self.rewrite_source { if rewrite_source.change_id() == commit.change_id() { self.mut_repo - .record_rewritten_commit(rewrite_source.id().clone(), commit.id().clone()); + .set_rewritten_commit(rewrite_source.id().clone(), commit.id().clone()); } } Ok(commit) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index c45a449ef..0162c700e 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -820,33 +820,6 @@ impl MutableRepo { .insert(old_id, std::iter::once(new_id).collect()); } - /// Record a commit as having been rewritten in this transaction. If it was - /// already rewritten, mark it as divergent (unlike `set_rewritten_commit`) - /// - /// This record is used by `rebase_descendants` to know which commits have - /// children that need to be rebased, and where to rebase the children (as - /// well as branches) to. - /// - /// The `rebase_descendants` logic treats these records as follows: - /// - /// - If a commit is recorded as rewritten to a single commit, its - /// descendants would be rebased to become descendants of `new_id`. Any - /// branches at `old_id` are also moved to `new_id`. - /// - If a commit is recorded as rewritten to more than one commit, it is - /// assumed to have become divergent. Its descendants are *not* rebased. - /// However, the *branches* that were at `old_id` are moved to each of the - /// new ids, and thus become conflicted. - /// - /// In neither case would `rebase_descendants` modify the `old_id` commit - /// itself. - pub fn record_rewritten_commit(&mut self, old_id: CommitId, new_id: CommitId) { - assert_ne!(old_id, *self.store().root_commit_id()); - self.rewritten_commits - .entry(old_id) - .or_default() - .insert(new_id); - } - /// Record a commit as being rewritten into multiple other commits in this /// transaction. /// diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 9c40bd5aa..1d84d6aef 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -191,7 +191,7 @@ pub fn rebase_commit_with_options( // Record old_commit as being succeeded by the parent. // This ensures that when we stack commits, the second commit knows to // rebase on top of the parent commit, rather than the abandoned commit. - mut_repo.record_rewritten_commit(old_commit.id().clone(), parent.id().clone()); + mut_repo.set_rewritten_commit(old_commit.id().clone(), parent.id().clone()); return Ok(RebasedCommit::Abandoned { parent: parent.clone(), }); diff --git a/lib/tests/test_mut_repo.rs b/lib/tests/test_mut_repo.rs index b3838c7d1..ca9dec82a 100644 --- a/lib/tests/test_mut_repo.rs +++ b/lib/tests/test_mut_repo.rs @@ -453,7 +453,7 @@ fn test_rebase_descendants_simple() { let mut_repo = tx.mut_repo(); let mut graph_builder = CommitGraphBuilder::new(&settings, mut_repo); let commit6 = graph_builder.commit_with_parents(&[&commit1]); - mut_repo.record_rewritten_commit(commit2.id().clone(), commit6.id().clone()); + mut_repo.set_rewritten_commit(commit2.id().clone(), commit6.id().clone()); mut_repo.record_abandoned_commit(commit4.id().clone()); let rebase_map = tx .mut_repo() @@ -474,7 +474,7 @@ fn test_rebase_descendants_simple() { } #[test] -fn test_rebase_descendants_conflicting_rewrite() { +fn test_rebase_descendants_divergent_rewrite() { // Test rebasing descendants when one commit was rewritten to several other // commits. There are many additional tests of this functionality in // `test_rewrite.rs`. @@ -494,8 +494,10 @@ fn test_rebase_descendants_conflicting_rewrite() { let mut graph_builder = CommitGraphBuilder::new(&settings, mut_repo); let commit4 = graph_builder.commit_with_parents(&[&commit1]); 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()); + mut_repo.set_divergent_rewrite( + commit2.id().clone(), + vec![commit4.id().clone(), commit5.id().clone()], + ); // Commit 3 does *not* get rebased because it's unclear if it should go onto // commit 4 or commit 5 let rebase_map = tx diff --git a/lib/tests/test_rewrite.rs b/lib/tests/test_rewrite.rs index 7285c070d..e81136790 100644 --- a/lib/tests/test_rewrite.rs +++ b/lib/tests/test_rewrite.rs @@ -86,7 +86,7 @@ fn test_rebase_descendants_sideways() { let commit_f = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -140,7 +140,7 @@ fn test_rebase_descendants_forward() { let commit_g = graph_builder.commit_with_parents(&[&commit_f]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -197,11 +197,11 @@ fn test_rebase_descendants_reorder() { let commit_i = graph_builder.commit_with_parents(&[&commit_g]); tx.mut_repo() - .record_rewritten_commit(commit_e.id().clone(), commit_d.id().clone()); + .set_rewritten_commit(commit_e.id().clone(), commit_d.id().clone()); tx.mut_repo() - .record_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); tx.mut_repo() - .record_rewritten_commit(commit_g.id().clone(), commit_h.id().clone()); + .set_rewritten_commit(commit_g.id().clone(), commit_h.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -237,7 +237,7 @@ fn test_rebase_descendants_backward() { let commit_d = graph_builder.commit_with_parents(&[&commit_c]); tx.mut_repo() - .record_rewritten_commit(commit_c.id().clone(), commit_b.id().clone()); + .set_rewritten_commit(commit_c.id().clone(), commit_b.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -277,9 +277,9 @@ fn test_rebase_descendants_chain_becomes_branchy() { let commit_f = graph_builder.commit_with_parents(&[&commit_b]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_e.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_e.id().clone()); tx.mut_repo() - .record_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -323,7 +323,7 @@ fn test_rebase_descendants_internal_merge() { let commit_f = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -371,7 +371,7 @@ fn test_rebase_descendants_external_merge() { let commit_f = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_c.id().clone(), commit_f.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -491,7 +491,7 @@ fn test_rebase_descendants_abandon_and_replace() { let commit_e = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_e.id().clone()); + .set_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() @@ -661,9 +661,9 @@ fn test_rebase_descendants_multiple_sideways() { let commit_f = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone()); tx.mut_repo() - .record_rewritten_commit(commit_d.id().clone(), commit_f.id().clone()); + .set_rewritten_commit(commit_d.id().clone(), commit_f.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -704,9 +704,9 @@ fn test_rebase_descendants_multiple_swap() { let _commit_e = graph_builder.commit_with_parents(&[&commit_d]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_d.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_d.id().clone()); tx.mut_repo() - .record_rewritten_commit(commit_d.id().clone(), commit_b.id().clone()); + .set_rewritten_commit(commit_d.id().clone(), commit_b.id().clone()); let _ = tx.mut_repo().rebase_descendants_return_map(&settings); // Panics because of the cycle } @@ -730,9 +730,9 @@ fn test_rebase_descendants_multiple_no_descendants() { let commit_c = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_c.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_c.id().clone()); tx.mut_repo() - .record_rewritten_commit(commit_c.id().clone(), commit_b.id().clone()); + .set_rewritten_commit(commit_c.id().clone(), commit_b.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -789,14 +789,14 @@ fn test_rebase_descendants_divergent_rewrite() { let commit_f2 = graph_builder.commit_with_parents(&[&commit_a]); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_b2.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_b2.id().clone()); // Commit D becomes divergent + tx.mut_repo().set_divergent_rewrite( + commit_d.id().clone(), + vec![commit_d2.id().clone(), commit_d3.id().clone()], + ); 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()); + .set_rewritten_commit(commit_f.id().clone(), commit_f2.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -941,7 +941,7 @@ fn test_rebase_descendants_contents() { .unwrap(); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_d.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_d.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_return_map(&settings) @@ -1187,6 +1187,14 @@ fn test_rebase_descendants_update_branches_after_divergent_rewrite() { .set_description("more different") .write() .unwrap(); + tx.mut_repo().set_divergent_rewrite( + commit_b.id().clone(), + vec![ + commit_b2.id().clone(), + commit_b3.id().clone(), + commit_b4.id().clone(), + ], + ); tx.mut_repo().rebase_descendants(&settings).unwrap(); let main_target = tx.mut_repo().get_local_branch("main"); @@ -1266,6 +1274,14 @@ fn test_rebase_descendants_rewrite_updates_branch_conflict() { .set_description("different") .write() .unwrap(); + tx.mut_repo().set_divergent_rewrite( + commit_a.id().clone(), + vec![commit_a2.id().clone(), commit_a3.id().clone()], + ); + tx.mut_repo().set_divergent_rewrite( + commit_b.id().clone(), + vec![commit_b2.id().clone(), commit_b3.id().clone()], + ); tx.mut_repo().rebase_descendants(&settings).unwrap(); let target = tx.mut_repo().get_local_branch("main"); @@ -1576,7 +1592,7 @@ fn test_empty_commit_option(empty_behavior: EmptyBehaviour) { let commit_bd = create_commit(&[&commit_a], &tree_d); tx.mut_repo() - .record_rewritten_commit(commit_b.id().clone(), commit_bd.id().clone()); + .set_rewritten_commit(commit_b.id().clone(), commit_bd.id().clone()); let rebase_map = tx .mut_repo() .rebase_descendants_with_options_return_map(