From f5e989d77bcb76944070a39819c2bdfd77755d62 Mon Sep 17 00:00:00 2001 From: Benjamin Tan Date: Tue, 30 Apr 2024 20:01:54 +0800 Subject: [PATCH] rebase: add `compute_rebase_destination` function --- cli/src/commands/rebase.rs | 320 ++++++++++++++----------------------- 1 file changed, 116 insertions(+), 204 deletions(-) diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index 6674bb830..67130cf8b 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -241,60 +241,16 @@ pub(crate) fn cmd_rebase( EmptyBehaviour::Keep, "clap should forbid `-r --skip-empty`" ); - let target_commits: Vec<_> = workspace_command - .parse_union_revsets(ui, &args.revisions)? - .evaluate_to_commits()? - .try_collect()?; // in reverse topological order - if !args.insert_after.is_empty() && !args.insert_before.is_empty() { - let after_commits = - workspace_command.resolve_some_revsets_default_single(ui, &args.insert_after)?; - let before_commits = - workspace_command.resolve_some_revsets_default_single(ui, &args.insert_before)?; - rebase_revisions_after_before( - ui, - command.settings(), - &mut workspace_command, - &after_commits, - &before_commits, - &target_commits, - &rebase_options, - )?; - } else if !args.insert_after.is_empty() { - let after_commits = - workspace_command.resolve_some_revsets_default_single(ui, &args.insert_after)?; - rebase_revisions_after( - ui, - command.settings(), - &mut workspace_command, - &after_commits, - &target_commits, - &rebase_options, - )?; - } else if !args.insert_before.is_empty() { - let before_commits = - workspace_command.resolve_some_revsets_default_single(ui, &args.insert_before)?; - rebase_revisions_before( - ui, - command.settings(), - &mut workspace_command, - &before_commits, - &target_commits, - &rebase_options, - )?; - } else { - let new_parents = workspace_command - .resolve_some_revsets_default_single(ui, &args.destination)? - .into_iter() - .collect_vec(); - rebase_revisions( - ui, - command.settings(), - &mut workspace_command, - &new_parents, - &target_commits, - &rebase_options, - )?; - } + rebase_revisions( + ui, + command.settings(), + &mut workspace_command, + &args.revisions, + &args.destination, + &args.insert_after, + &args.insert_before, + &rebase_options, + )?; } else if !args.source.is_empty() { let new_parents = workspace_command .resolve_some_revsets_default_single(ui, &args.destination)? @@ -332,6 +288,52 @@ pub(crate) fn cmd_rebase( Ok(()) } +#[allow(clippy::too_many_arguments)] +fn rebase_revisions( + ui: &mut Ui, + settings: &UserSettings, + workspace_command: &mut WorkspaceCommandHelper, + revisions: &[RevisionArg], + destination: &[RevisionArg], + insert_after: &[RevisionArg], + insert_before: &[RevisionArg], + rebase_options: &RebaseOptions, +) -> Result<(), CommandError> { + let target_commits: Vec<_> = workspace_command + .parse_union_revsets(ui, revisions)? + .evaluate_to_commits()? + .try_collect()?; // in reverse topological order + workspace_command.check_rewritable(target_commits.iter().ids())?; + + let (new_parents, new_children) = compute_rebase_destination( + ui, + workspace_command, + destination, + insert_after, + insert_before, + )?; + if !destination.is_empty() && new_children.is_empty() { + for commit in &target_commits { + if new_parents.contains(commit) { + return Err(user_error(format!( + "Cannot rebase {} onto itself", + short_commit_hash(commit.id()), + ))); + } + } + } + rebase_revisions_transaction( + ui, + settings, + workspace_command, + &new_parents.iter().ids().cloned().collect_vec(), + &new_children, + &target_commits, + None, + rebase_options, + ) +} + fn rebase_branch( ui: &mut Ui, settings: &UserSettings, @@ -451,164 +453,74 @@ fn rebase_descendants_transaction( tx.finish(ui, tx_description) } -fn rebase_revisions( +/// Computes the new parents and children given the input arguments for +/// `destination`, `insert_after`, and `insert_before`. +fn compute_rebase_destination( ui: &mut Ui, - settings: &UserSettings, workspace_command: &mut WorkspaceCommandHelper, - new_parents: &[Commit], - target_commits: &[Commit], - rebase_options: &RebaseOptions, -) -> Result<(), CommandError> { - if target_commits.is_empty() { - return Ok(()); - } - - workspace_command.check_rewritable(target_commits.iter().ids())?; - for commit in target_commits { - if new_parents.contains(commit) { - return Err(user_error(format!( - "Cannot rebase {} onto itself", - short_commit_hash(commit.id()), - ))); + destination: &[RevisionArg], + insert_after: &[RevisionArg], + insert_before: &[RevisionArg], +) -> Result<(Vec, Vec), CommandError> { + let resolve_revisions = |revisions: &[RevisionArg]| -> Result, CommandError> { + if revisions.is_empty() { + Ok(vec![]) + } else { + Ok(workspace_command + .resolve_some_revsets_default_single(ui, revisions)? + .into_iter() + .collect_vec()) } + }; + let destination_commits = resolve_revisions(destination)?; + let after_commits = resolve_revisions(insert_after)?; + let before_commits = resolve_revisions(insert_before)?; + + let (new_parents, new_children) = if !after_commits.is_empty() && !before_commits.is_empty() { + (after_commits, before_commits) + } else if !after_commits.is_empty() { + let new_children: Vec<_> = + RevsetExpression::commits(after_commits.iter().ids().cloned().collect_vec()) + .children() + .evaluate_programmatic(workspace_command.repo().as_ref())? + .iter() + .commits(workspace_command.repo().store()) + .try_collect()?; + + (after_commits, new_children) + } else if !before_commits.is_empty() { + // Not using `RevsetExpression::parents` here to persist the order of parents + // specified in `before_commits`. + let new_parent_ids = before_commits + .iter() + .flat_map(|commit| commit.parent_ids().iter().cloned().collect_vec()) + .unique() + .collect_vec(); + let new_parents: Vec<_> = new_parent_ids + .iter() + .map(|commit_id| workspace_command.repo().store().get_commit(commit_id)) + .try_collect()?; + + (new_parents, before_commits) + } else { + (destination_commits, vec![]) + }; + + if !new_children.is_empty() { + workspace_command.check_rewritable(new_children.iter().ids())?; + ensure_no_commit_loop( + workspace_command.repo().as_ref(), + &RevsetExpression::commits(new_children.iter().ids().cloned().collect_vec()), + &RevsetExpression::commits(new_parents.iter().ids().cloned().collect_vec()), + )?; } - move_commits_transaction( - ui, - settings, - workspace_command, - &new_parents.iter().ids().cloned().collect_vec(), - &[], - target_commits, - None, - rebase_options, - ) -} - -fn rebase_revisions_after( - ui: &mut Ui, - settings: &UserSettings, - workspace_command: &mut WorkspaceCommandHelper, - after_commits: &IndexSet, - target_commits: &[Commit], - rebase_options: &RebaseOptions, -) -> Result<(), CommandError> { - workspace_command.check_rewritable(target_commits.iter().ids())?; - - let after_commit_ids = after_commits.iter().ids().cloned().collect_vec(); - let new_parents_expression = RevsetExpression::commits(after_commit_ids.clone()); - let new_children_expression = new_parents_expression.children(); - - ensure_no_commit_loop( - workspace_command.repo().as_ref(), - &new_children_expression, - &new_parents_expression, - )?; - - let new_parent_ids = after_commit_ids; - let new_children: Vec<_> = new_children_expression - .evaluate_programmatic(workspace_command.repo().as_ref())? - .iter() - .commits(workspace_command.repo().store()) - .try_collect()?; - workspace_command.check_rewritable(new_children.iter().ids())?; - - move_commits_transaction( - ui, - settings, - workspace_command, - &new_parent_ids, - &new_children, - target_commits, - None, - rebase_options, - ) -} - -fn rebase_revisions_before( - ui: &mut Ui, - settings: &UserSettings, - workspace_command: &mut WorkspaceCommandHelper, - before_commits: &IndexSet, - target_commits: &[Commit], - rebase_options: &RebaseOptions, -) -> Result<(), CommandError> { - workspace_command.check_rewritable(target_commits.iter().ids())?; - let before_commit_ids = before_commits.iter().ids().cloned().collect_vec(); - workspace_command.check_rewritable(&before_commit_ids)?; - - let new_children_expression = RevsetExpression::commits(before_commit_ids); - let new_parents_expression = new_children_expression.parents(); - - ensure_no_commit_loop( - workspace_command.repo().as_ref(), - &new_children_expression, - &new_parents_expression, - )?; - - // Not using `new_parents_expression` here to persist the order of parents - // specified in `before_commits`. - let new_parent_ids: IndexSet<_> = before_commits - .iter() - .flat_map(|commit| commit.parent_ids().iter().cloned().collect_vec()) - .collect(); - let new_parent_ids = new_parent_ids.into_iter().collect_vec(); - let new_children = before_commits.iter().cloned().collect_vec(); - - move_commits_transaction( - ui, - settings, - workspace_command, - &new_parent_ids, - &new_children, - target_commits, - None, - rebase_options, - ) + Ok((new_parents, new_children)) } +/// Creates a transaction for rebasing revisions. #[allow(clippy::too_many_arguments)] -fn rebase_revisions_after_before( - ui: &mut Ui, - settings: &UserSettings, - workspace_command: &mut WorkspaceCommandHelper, - after_commits: &IndexSet, - before_commits: &IndexSet, - target_commits: &[Commit], - rebase_options: &RebaseOptions, -) -> Result<(), CommandError> { - workspace_command.check_rewritable(target_commits.iter().ids())?; - let before_commit_ids = before_commits.iter().ids().cloned().collect_vec(); - workspace_command.check_rewritable(&before_commit_ids)?; - - let after_commit_ids = after_commits.iter().ids().cloned().collect_vec(); - let new_children_expression = RevsetExpression::commits(before_commit_ids); - let new_parents_expression = RevsetExpression::commits(after_commit_ids.clone()); - - ensure_no_commit_loop( - workspace_command.repo().as_ref(), - &new_children_expression, - &new_parents_expression, - )?; - - let new_parent_ids = after_commit_ids; - let new_children = before_commits.iter().cloned().collect_vec(); - - move_commits_transaction( - ui, - settings, - workspace_command, - &new_parent_ids, - &new_children, - target_commits, - None, - rebase_options, - ) -} - -/// Wraps `move_commits` in a transaction. -#[allow(clippy::too_many_arguments)] -fn move_commits_transaction( +fn rebase_revisions_transaction( ui: &mut Ui, settings: &UserSettings, workspace_command: &mut WorkspaceCommandHelper,