rewrite: move_commits: add MoveCommitsTarget enum to specify roots or commits to move

This also allows some minor optimizations to be performed, such as
avoiding recomputation of the connected target set when
`MoveCommitsTarget::Roots` is used since the connected target set is
identical to the target set (all descendants of the roots).
This commit is contained in:
Benjamin Tan 2024-10-15 21:30:09 +08:00
parent 11648ee2e3
commit 913c56a98e
No known key found for this signature in database
GPG key ID: A853F0716C413825
2 changed files with 125 additions and 84 deletions

View file

@ -30,6 +30,7 @@ use jj_lib::revset::RevsetIteratorExt;
use jj_lib::rewrite::move_commits;
use jj_lib::rewrite::EmptyBehaviour;
use jj_lib::rewrite::MoveCommitsStats;
use jj_lib::rewrite::MoveCommitsTarget;
use jj_lib::rewrite::RebaseOptions;
use jj_lib::settings::UserSettings;
use tracing::instrument;
@ -325,7 +326,7 @@ fn rebase_revisions(
workspace_command,
&new_parents.iter().ids().cloned().collect_vec(),
&new_children,
&target_commits,
target_commits,
rebase_options,
)
}
@ -358,7 +359,7 @@ fn rebase_source(
workspace_command,
&new_parents.iter().ids().cloned().collect_vec(),
&new_children,
&source_commits,
source_commits,
rebase_options,
)
}
@ -396,7 +397,7 @@ fn rebase_branch(
workspace_command,
&parent_ids,
&[],
&root_commits,
root_commits,
&rebase_options,
)
}
@ -407,7 +408,7 @@ fn rebase_descendants_transaction(
workspace_command: &mut WorkspaceCommandHelper,
new_parent_ids: &[CommitId],
new_children: &[Commit],
target_roots: &[Commit],
target_roots: Vec<Commit>,
rebase_options: &RebaseOptions,
) -> Result<(), CommandError> {
if target_roots.is_empty() {
@ -427,15 +428,6 @@ fn rebase_descendants_transaction(
)
};
let target_commits: Vec<_> =
RevsetExpression::commits(target_roots.iter().ids().cloned().collect_vec())
.descendants()
.evaluate_programmatic(tx.repo())?
.iter()
.commits(tx.repo().store())
.try_collect()?;
let target_roots = target_roots.iter().ids().cloned().collect_vec();
let MoveCommitsStats {
num_rebased_targets,
num_rebased_descendants,
@ -446,8 +438,7 @@ fn rebase_descendants_transaction(
tx.repo_mut(),
new_parent_ids,
new_children,
&target_commits,
Some(&target_roots),
MoveCommitsTarget::Roots(target_roots),
rebase_options,
)?;
@ -551,7 +542,7 @@ fn rebase_revisions_transaction(
workspace_command: &mut WorkspaceCommandHelper,
new_parent_ids: &[CommitId],
new_children: &[Commit],
target_commits: &[Commit],
target_commits: Vec<Commit>,
rebase_options: &RebaseOptions,
) -> Result<(), CommandError> {
if target_commits.is_empty() {
@ -579,8 +570,7 @@ fn rebase_revisions_transaction(
tx.repo_mut(),
new_parent_ids,
new_children,
target_commits,
None,
MoveCommitsTarget::Commits(target_commits),
rebase_options,
)?;
// TODO(ilyagr): Consider making it possible for descendants of the target set

View file

@ -464,85 +464,107 @@ pub struct MoveCommitsStats {
pub num_abandoned: u32,
}
pub enum MoveCommitsTarget {
/// The commits to be moved. Commits should be mutable and in reverse
/// topological order.
Commits(Vec<Commit>),
/// The root commits to be moved, along with all their descendants.
/// Root commits should be mutable and in reverse topological order.
Roots(Vec<Commit>),
}
/// Moves `target_commits` from their current location to a new location in the
/// graph.
///
/// Commits in `target_roots` are rebased onto the new parents
/// given by `new_parent_ids`, while the `new_children` commits are
/// rebased onto the heads of `target_commits`. If `target_roots` is
/// `None`, it will be computed as the roots of the connected set of
/// target commits. This assumes that `target_commits` and
/// `new_children` can be rewritten, and there will be no cycles in
/// the resulting graph. `target_commits` should be in reverse
/// topological order. `target_roots`, if provided, should be a subset
/// of `target_commits`.
/// Commits in `target` are rebased onto the new parents given by
/// `new_parent_ids`, while the `new_children` commits are rebased onto the
/// heads of the commits in `targets`. This assumes that commits in `target` and
/// `new_children` can be rewritten, and there will be no cycles in the
/// resulting graph. Commits in `target` should be in reverse topological order.
pub fn move_commits(
settings: &UserSettings,
mut_repo: &mut MutableRepo,
new_parent_ids: &[CommitId],
new_children: &[Commit],
target_commits: &[Commit],
target_roots: Option<&[CommitId]>,
target: MoveCommitsTarget,
options: &RebaseOptions,
) -> BackendResult<MoveCommitsStats> {
if target_commits.is_empty() {
return Ok(MoveCommitsStats {
num_rebased_targets: 0,
num_rebased_descendants: 0,
num_skipped_rebases: 0,
num_abandoned: 0,
});
}
let target_commits: Vec<Commit>;
let target_commit_ids: HashSet<_>;
let connected_target_commits: Vec<Commit>;
let connected_target_commits_internal_parents: HashMap<CommitId, Vec<CommitId>>;
let target_roots: HashSet<CommitId>;
let target_commit_ids: HashSet<_> = target_commits.iter().ids().cloned().collect();
let connected_target_commits: Vec<_> =
RevsetExpression::commits(target_commits.iter().ids().cloned().collect_vec())
.connected()
.evaluate_programmatic(mut_repo)
.map_err(|err| match err {
RevsetEvaluationError::StoreError(err) => err,
RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
})?
.iter()
.commits(mut_repo.store())
.try_collect()?;
// Compute the parents of all commits in the connected target set, allowing only
// commits in the target set as parents. The parents of each commit are
// identical to the ones found using a preorder DFS of the node's ancestors,
// starting from the node itself, and avoiding traversing an edge if the
// parent is in the target set.
let mut connected_target_commits_internal_parents: HashMap<CommitId, Vec<CommitId>> =
HashMap::new();
for commit in connected_target_commits.iter().rev() {
// The roots of the set will not have any parents found in
// `connected_target_commits_internal_parents`, and will be stored as an empty
// vector.
let mut new_parents = vec![];
for old_parent in commit.parent_ids() {
if target_commit_ids.contains(old_parent) {
new_parents.push(old_parent.clone());
} else if let Some(parents) = connected_target_commits_internal_parents.get(old_parent)
{
new_parents.extend(parents.iter().cloned());
match target {
MoveCommitsTarget::Commits(commits) => {
if commits.is_empty() {
return Ok(MoveCommitsStats {
num_rebased_targets: 0,
num_rebased_descendants: 0,
num_skipped_rebases: 0,
num_abandoned: 0,
});
}
}
connected_target_commits_internal_parents.insert(commit.id().clone(), new_parents);
}
// Compute the roots of `target_commits` if not provided.
let target_roots: HashSet<_> = if let Some(target_roots) = target_roots {
target_roots.iter().cloned().collect()
} else {
connected_target_commits_internal_parents
.iter()
.filter(|(commit_id, parents)| {
target_commit_ids.contains(commit_id) && parents.is_empty()
})
.map(|(commit_id, _)| commit_id.clone())
.collect()
};
target_commits = commits;
target_commit_ids = target_commits.iter().ids().cloned().collect();
connected_target_commits =
RevsetExpression::commits(target_commits.iter().ids().cloned().collect_vec())
.connected()
.evaluate_programmatic(mut_repo)
.map_err(|err| match err {
RevsetEvaluationError::StoreError(err) => err,
RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
})?
.iter()
.commits(mut_repo.store())
.try_collect()?;
connected_target_commits_internal_parents =
compute_connected_target_commits_internal_parents(
&target_commit_ids,
&connected_target_commits,
);
target_roots = connected_target_commits_internal_parents
.iter()
.filter(|(commit_id, parents)| {
target_commit_ids.contains(commit_id) && parents.is_empty()
})
.map(|(commit_id, _)| commit_id.clone())
.collect();
}
MoveCommitsTarget::Roots(roots) => {
if roots.is_empty() {
return Ok(MoveCommitsStats {
num_rebased_targets: 0,
num_rebased_descendants: 0,
num_skipped_rebases: 0,
num_abandoned: 0,
});
}
target_commits = RevsetExpression::commits(roots.iter().ids().cloned().collect_vec())
.descendants()
.evaluate_programmatic(mut_repo)
.map_err(|err| match err {
RevsetEvaluationError::StoreError(err) => err,
RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"),
})?
.iter()
.commits(mut_repo.store())
.try_collect()?;
target_commit_ids = target_commits.iter().ids().cloned().collect();
connected_target_commits = target_commits.iter().cloned().collect_vec();
connected_target_commits_internal_parents =
compute_connected_target_commits_internal_parents(
&target_commit_ids,
&connected_target_commits,
);
target_roots = roots.iter().ids().cloned().collect();
}
}
// If a commit outside the target set has a commit in the target set as a
// parent, then - after the transformation - it should have that commit's
@ -846,6 +868,35 @@ pub fn move_commits(
})
}
/// Computes the parents of all commits in the connected target set, allowing
/// only commits in the target set as parents. The parents of each commit are
/// identical to the ones found using a preorder DFS of the node's ancestors,
/// starting from the node itself, and avoiding traversing an edge if the
/// parent is in the target set.
fn compute_connected_target_commits_internal_parents(
target_commit_ids: &HashSet<CommitId>,
connected_target_commits: &[Commit],
) -> HashMap<CommitId, Vec<CommitId>> {
let mut connected_target_commits_internal_parents: HashMap<CommitId, Vec<CommitId>> =
HashMap::new();
for commit in connected_target_commits.iter().rev() {
// The roots of the set will not have any parents found in
// `connected_target_commits_internal_parents`, and will be stored as an empty
// vector.
let mut new_parents = vec![];
for old_parent in commit.parent_ids() {
if target_commit_ids.contains(old_parent) {
new_parents.push(old_parent.clone());
} else if let Some(parents) = connected_target_commits_internal_parents.get(old_parent)
{
new_parents.extend(parents.iter().cloned());
}
}
connected_target_commits_internal_parents.insert(commit.id().clone(), new_parents);
}
connected_target_commits_internal_parents
}
pub struct CommitToSquash {
pub commit: Commit,
pub selected_tree: MergedTree,