rewrite: take commit and new parents by value in rebase_commit()

I'm going to add a helper struct to help with rewriting commits. I
want to make that struct own the old commit and the new parents to
simplify lifetimes. This patch prepares for that by passing the
commits by value to `rebase_commit()`.
This commit is contained in:
Martin von Zweigbergk 2024-04-11 08:54:45 -07:00 committed by Martin von Zweigbergk
parent dca9c6f884
commit 057b7c8d0b
7 changed files with 33 additions and 33 deletions

View file

@ -139,8 +139,8 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
rebase_commit(
command.settings(),
tx.mut_repo(),
&child_commit,
&[new_commit.clone()],
child_commit,
vec![new_commit.clone()],
)?;
}
} else {
@ -179,8 +179,8 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
rebase_commit(
command.settings(),
tx.mut_repo(),
&child_commit,
&new_parent_commits,
child_commit,
new_parent_commits,
)?;
}
}

View file

@ -114,7 +114,7 @@ pub(crate) fn cmd_parallelize(
rebase_descendants(
&mut tx,
command.settings(),
&new_parents.into_iter().collect_vec(),
new_parents.into_iter().collect_vec(),
&[child],
Default::default(),
)?;
@ -135,7 +135,7 @@ pub(crate) fn cmd_parallelize(
rebase_descendants(
&mut tx,
command.settings(),
&new_parents,
new_parents,
&target_commits,
Default::default(),
)?;

View file

@ -226,7 +226,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
ui,
command.settings(),
&mut workspace_command,
&new_parents,
new_parents,
&source_commits,
rebase_options,
)?;
@ -240,7 +240,7 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
ui,
command.settings(),
&mut workspace_command,
&new_parents,
new_parents,
&branch_commits,
rebase_options,
)?;
@ -252,7 +252,7 @@ fn rebase_branch(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
new_parents: &[Commit],
new_parents: Vec<Commit>,
branch_commits: &IndexSet<Commit>,
rebase_options: RebaseOptions,
) -> Result<(), CommandError> {
@ -287,7 +287,7 @@ fn rebase_branch(
pub fn rebase_descendants(
tx: &mut WorkspaceCommandTransaction,
settings: &UserSettings,
new_parents: &[Commit],
new_parents: Vec<Commit>,
old_commits: &[impl Borrow<Commit>],
rebase_options: RebaseOptions,
) -> Result<usize, CommandError> {
@ -295,8 +295,8 @@ pub fn rebase_descendants(
rebase_commit_with_options(
settings,
tx.mut_repo(),
old_commit.borrow(),
new_parents,
old_commit.borrow().clone(),
new_parents.to_vec(),
&rebase_options,
)?;
}
@ -310,7 +310,7 @@ fn rebase_descendants_transaction(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
new_parents: &[Commit],
new_parents: Vec<Commit>,
old_commits: &IndexSet<Commit>,
rebase_options: RebaseOptions,
) -> Result<(), CommandError> {
@ -327,7 +327,7 @@ fn rebase_descendants_transaction(
return Ok(());
}
for old_commit in old_commits.iter() {
check_rebase_destinations(workspace_command.repo(), new_parents, old_commit)?;
check_rebase_destinations(workspace_command.repo(), &new_parents, old_commit)?;
}
let mut tx = workspace_command.start_transaction();
let num_rebased =
@ -377,7 +377,7 @@ fn rebase_revision(
// First, rebase the children of `old_commit`.
let mut tx = workspace_command.start_transaction();
let mut rebased_commit_ids = HashMap::new();
for child_commit in &child_commits {
for child_commit in child_commits {
let new_child_parent_ids: Vec<CommitId> = child_commit
.parents()
.iter()
@ -411,7 +411,7 @@ fn rebase_revision(
rebased_commit_ids.insert(
child_commit.id().clone(),
rebase_commit(settings, tx.mut_repo(), child_commit, &new_child_parents)?
rebase_commit(settings, tx.mut_repo(), child_commit, new_child_parents)?
.id()
.clone(),
);
@ -458,6 +458,7 @@ fn rebase_revision(
})
.try_collect()?;
let tx_description = format!("rebase commit {}", old_commit.id().hex());
// Finally, it's safe to rebase `old_commit`. We can skip rebasing if it is
// already a child of `new_parents`. Otherwise, at this point, it should no
// longer have any children; they have all been rebased and the originals
@ -470,7 +471,7 @@ fn rebase_revision(
}
true
} else {
rebase_commit(settings, tx.mut_repo(), &old_commit, &new_parents)?;
rebase_commit(settings, tx.mut_repo(), old_commit, new_parents)?;
debug_assert_eq!(tx.mut_repo().rebase_descendants(settings)?, 0);
false
};
@ -490,7 +491,7 @@ fn rebase_revision(
}
}
if tx.mut_repo().has_changes() {
tx.finish(ui, format!("rebase commit {}", old_commit.id().hex()))
tx.finish(ui, tx_description)
} else {
Ok(()) // Do not print "Nothing changed."
}

View file

@ -232,8 +232,7 @@ fn rebase_children_for_siblings_split(
}
})
.collect_vec();
num_rebased +=
rebase_descendants(tx, settings, &new_parents, &[child], Default::default())?;
num_rebased += rebase_descendants(tx, settings, new_parents, &[child], Default::default())?;
}
Ok(num_rebased)
}

View file

@ -95,8 +95,8 @@ pub fn restore_tree(
pub fn rebase_commit(
settings: &UserSettings,
mut_repo: &mut MutableRepo,
old_commit: &Commit,
new_parents: &[Commit],
old_commit: Commit,
new_parents: Vec<Commit>,
) -> BackendResult<Commit> {
let rebased_commit = rebase_commit_with_options(
settings,
@ -119,8 +119,8 @@ pub enum RebasedCommit {
pub fn rebase_commit_with_options(
settings: &UserSettings,
mut_repo: &mut MutableRepo,
old_commit: &Commit,
new_parents: &[Commit],
old_commit: Commit,
new_parents: Vec<Commit>,
options: &RebaseOptions,
) -> BackendResult<RebasedCommit> {
// If specified, don't create commit where one parent is an ancestor of another.
@ -139,7 +139,7 @@ pub fn rebase_commit_with_options(
.collect_vec();
&simplified_new_parents[..]
} else {
new_parents
&new_parents
};
let old_parents = old_commit.parents();
@ -195,7 +195,7 @@ pub fn rebase_commit_with_options(
.map(|commit| commit.id().clone())
.collect();
let new_commit = mut_repo
.rewrite_commit(settings, old_commit)
.rewrite_commit(settings, &old_commit)
.set_parents(new_parent_ids)
.set_tree_id(new_tree_id)
.write()?;
@ -327,8 +327,8 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
let rebased_commit: RebasedCommit = rebase_commit_with_options(
self.settings,
self.mut_repo,
&old_commit,
&new_parents,
old_commit,
new_parents,
&self.options,
)?;
let new_commit = match rebased_commit {

View file

@ -581,9 +581,9 @@ fn test_simplify_conflict_after_resolving_parent() {
.write()
.unwrap();
let commit_b2 = rebase_commit(&settings, tx.mut_repo(), &commit_b, &[commit_d]).unwrap();
let commit_b2 = rebase_commit(&settings, tx.mut_repo(), commit_b, vec![commit_d]).unwrap();
let commit_c2 =
rebase_commit(&settings, tx.mut_repo(), &commit_c, &[commit_b2.clone()]).unwrap();
rebase_commit(&settings, tx.mut_repo(), commit_c, vec![commit_b2.clone()]).unwrap();
// Test the setup: Both B and C should have conflicts.
let tree_b2 = commit_b2.tree().unwrap();
@ -599,7 +599,7 @@ fn test_simplify_conflict_after_resolving_parent() {
.set_tree_id(tree_b3.id())
.write()
.unwrap();
let commit_c3 = rebase_commit(&settings, tx.mut_repo(), &commit_c2, &[commit_b3]).unwrap();
let commit_c3 = rebase_commit(&settings, tx.mut_repo(), commit_c2, vec![commit_b3]).unwrap();
tx.mut_repo().rebase_descendants(&settings).unwrap();
let repo = tx.commit("test");

View file

@ -1729,8 +1729,8 @@ fn test_rebase_abandoning_empty() {
rebase_commit_with_options(
&settings,
tx.mut_repo(),
&commit_b,
&[commit_b2.clone()],
commit_b,
vec![commit_b2.clone()],
&rebase_options,
)
.unwrap();