Skip to content

Commit

Permalink
split.rs: stop using DescendantRebaser::new
Browse files Browse the repository at this point in the history
This requires creating a new public API as a substitute. I took the opportunity
to also add some comments to the
`MutRepo::record_rewritten_commit`/`record_abandoned_commit` functions.

I imade the simplest possible addition to the API; it is not a very elegant
one. Eventually, the entire `record_rewritten_commit` API should probably be
refactored again.
  • Loading branch information
ilyagr committed Dec 23, 2023
1 parent 8e4d274 commit 2cda46d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 20 deletions.
19 changes: 9 additions & 10 deletions cli/src/commands/split.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ use std::io::Write;

use jj_lib::backend::ObjectId;
use jj_lib::repo::Repo;
use jj_lib::rewrite::{merge_commit_trees, DescendantRebaser};
use maplit::{hashmap, hashset};
use jj_lib::rewrite::merge_commit_trees;
use tracing::instrument;

use crate::cli_util::{CommandError, CommandHelper, RevisionArg};
Expand Down Expand Up @@ -135,14 +134,14 @@ don't make any changes, then the operation will be aborted.
.generate_new_change_id()
.set_description(second_description)
.write()?;
let mut rebaser = DescendantRebaser::new(
command.settings(),
tx.mut_repo(),
hashmap! { commit.id().clone() => hashset!{second_commit.id().clone()} },
hashset! {},
);
rebaser.rebase_all()?;
let num_rebased = rebaser.rebased().len();

// 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
// descendants.
tx.mut_repo()
.set_rewritten_commit(commit.id().clone(), [second_commit.id().clone()]);
let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
if num_rebased > 0 {
writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?;
}
Expand Down
40 changes: 30 additions & 10 deletions lib/src/repo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -810,12 +810,30 @@ impl MutableRepo {
Ok(commit)
}

/// Record a commit as having been rewritten in this transaction. This
/// record is used by `rebase_descendants()`.
/// Record a commit as having been rewritten to one or more ids in this
/// transaction.
///
/// Rewritten commits don't have to be recorded here. This is just a
/// convenient place to record it. It won't matter after the transaction
/// has been committed.
/// This record is used by `rebase_descendants` to know which commits have
/// children that need to be rebased, and where to rebase them to.
// TODO(ilyagr): Make this API saner, e.g. make `self.rewritten_commits` public
// and make empty values correspond to abandoned commits.
pub fn set_rewritten_commit(
&mut self,
old_id: CommitId,
new_ids: impl IntoIterator<Item = CommitId>,
) -> Option<HashSet<CommitId>> {
assert_ne!(old_id, *self.store().root_commit_id());
self.rewritten_commits
.insert(old_id, new_ids.into_iter().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.
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
Expand All @@ -828,12 +846,11 @@ impl MutableRepo {
self.rewritten_commits.clear();
}

/// Record a commit as having been abandoned in this transaction. This
/// record is used by `rebase_descendants()`.
/// Record a commit as having been abandoned in this transaction.
///
/// Abandoned commits don't have to be recorded here. This is just a
/// convenient place to record it. It won't matter after the transaction
/// has been committed.
/// 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.
pub fn record_abandoned_commit(&mut self, old_id: CommitId) {
assert_ne!(old_id, *self.store().root_commit_id());
self.abandoned_commits.insert(old_id);
Expand Down Expand Up @@ -877,6 +894,9 @@ impl MutableRepo {
Ok(Some(rebaser))
}

// TODO(ilyagr): Either document that this also moves branches (rename the
// function and the related functions?) or change things so that this only
// rebases descendants.
pub fn rebase_descendants_with_options(
&mut self,
settings: &UserSettings,
Expand Down

0 comments on commit 2cda46d

Please sign in to comment.