mirrors/jj
1
0
Fork 1
mirror of https://github.com/martinvonz/jj.git synced 2024-12-26 14:00:51 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 6

repo: delete record_rewritten_commit()

Browse source 
I don't think we have any callers left that call
`record_rewritten_commit()` multiple times within a transaction and
expect it to result in divergence. I think we should consider it a bug
to do that.
This commit is contained in:
Martin von Zweigbergk 2024-03-23 06:13:38 -07:00 committed by Martin von Zweigbergk
parent e55168fa3e
commit d2043f069e
6 changed files with 50 additions and 61 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
cli/src/commands/split.rs
View file

@ -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());

2
lib/src/commit_builder.rs
View file

@ -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)

27
lib/src/repo.rs
View file

@ -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.
///

2
lib/src/rewrite.rs
View file

@ -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(),
});

10
lib/tests/test_mut_repo.rs
View file

@ -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

66
lib/tests/test_rewrite.rs
View file

@ -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(