Skip to content

Commit

Permalink
rebase: add --insert-after and --insert-before options
Browse files Browse the repository at this point in the history
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed Mar 30, 2024
1 parent 53d7958 commit 1a4f7df
Show file tree
Hide file tree
Showing 3 changed files with 597 additions and 14 deletions.
332 changes: 324 additions & 8 deletions cli/src/commands/rebase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

use std::collections::HashMap;
use std::io::Write;
use std::rc::Rc;
use std::sync::Arc;

use clap::ArgGroup;
Expand Down Expand Up @@ -123,6 +124,7 @@ use crate::ui::Ui;
#[derive(clap::Args, Clone, Debug)]
#[command(verbatim_doc_comment)]
#[command(group(ArgGroup::new("to_rebase").args(&["branch", "source", "revision"])))]
#[command(group(ArgGroup::new("order").args(&["destination", "insert_after", "insert_before"]).required(true)))]
pub(crate) struct RebaseArgs {
/// Rebase the whole branch relative to destination's ancestors (can be
/// repeated)
Expand Down Expand Up @@ -155,8 +157,32 @@ pub(crate) struct RebaseArgs {
revision: Option<RevisionArg>,
/// The revision(s) to rebase onto (can be repeated to create a merge
/// commit)
#[arg(long, short, required = true)]
#[arg(long, short)]
destination: Vec<RevisionArg>,
/// The revision(s) to insert after (can be repeated to create a merge
/// commit)
///
/// Only works with `-r`.
#[arg(
long,
short = 'A',
visible_alias = "after",
conflicts_with = "source",
conflicts_with = "branch"
)]
insert_after: Vec<RevisionArg>,
/// The revision(s) to insert before (can be repeated to create a merge
/// commit)
///
/// Only works with `-r`.
#[arg(
long,
short = 'B',
visible_alias = "before",
conflicts_with = "source",
conflicts_with = "branch"
)]
insert_before: Vec<RevisionArg>,

/// If true, when rebasing would produce an empty commit, the commit is
/// abandoned. It will not be abandoned if it was already empty before the
Expand Down Expand Up @@ -211,13 +237,45 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
EmptyBehaviour::Keep,
"clap should forbid `-r --skip-empty`"
);
rebase_revision(
ui,
command.settings(),
&mut workspace_command,
&new_parents,
rev_str,
)?;
if !args.insert_after.is_empty() {
// TODO: Use `IndexSet`?
let after_commits = resolve_multiple_nonempty_revsets_default_single(
&workspace_command,
&args.insert_after,
)?
.into_iter()
.collect_vec();
rebase_revision_after(
ui,
command.settings(),
&mut workspace_command,
after_commits,
rev_str,
)?;
} else if !args.insert_before.is_empty() {
// TODO: Use `IndexSet`?
let before_commits = resolve_multiple_nonempty_revsets_default_single(
&workspace_command,
&args.insert_before,
)?
.into_iter()
.collect_vec();
rebase_revision_before(
ui,
command.settings(),
&mut workspace_command,
before_commits,
rev_str,
)?;
} else {
rebase_revision(
ui,
command.settings(),
&mut workspace_command,
&new_parents,
rev_str,
)?;
}
} else if !args.source.is_empty() {
let source_commits =
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.source)?;
Expand Down Expand Up @@ -417,6 +475,242 @@ fn rebase_revision(
}
}

fn rebase_revision_after(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
after_commits: Vec<Commit>,
rev_str: &str,
) -> Result<(), CommandError> {
let old_commit = workspace_command.resolve_single_rev(rev_str)?;
workspace_command.check_rewritable([&old_commit])?;

let after_commit_ids = after_commits
.iter()
.map(|commit| commit.id().clone())
.collect();
let new_parents_expression = RevsetExpression::commits(after_commit_ids);
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_parents = after_commits;

let new_children: Vec<Commit> = new_children_expression
.evaluate_programmatic(workspace_command.repo().as_ref())?
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;
workspace_command.check_rewritable(&new_children)?;

let (new_parents, new_children_with_parents) = move_revision_compute_parent_children(
workspace_command,
&old_commit,
&new_parents_expression,
new_parents,
new_children,
)?;

move_commit(
ui,
settings,
workspace_command,
&new_parents,
&new_children_with_parents,
old_commit,
)
}

fn rebase_revision_before(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
before_commits: Vec<Commit>,
rev_str: &str,
) -> Result<(), CommandError> {
let old_commit = workspace_command.resolve_single_rev(rev_str)?;
workspace_command.check_rewritable([&old_commit])?;
workspace_command.check_rewritable(&before_commits)?;

let before_commit_ids = before_commits.iter().map(|c| c.id().clone()).collect_vec();
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,
)?;

let new_parents: Vec<Commit> = new_parents_expression
// Exclude parents that are ancestors of each other.
.minus(&new_parents_expression.parents().ancestors())
.clone()
.evaluate_programmatic(workspace_command.repo().as_ref())?
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;
let new_children = before_commits;

let (new_parents, new_children_with_parents) = move_revision_compute_parent_children(
workspace_command,
&old_commit,
&new_parents_expression,
new_parents,
new_children,
)?;

move_commit(
ui,
settings,
workspace_command,
&new_parents,
&new_children_with_parents,
old_commit,
)
}

type NewParentsAndChildren = (Vec<Commit>, Vec<(Commit, Vec<Commit>)>);

fn move_revision_compute_parent_children(
workspace_command: &mut WorkspaceCommandHelper,
old_commit: &Commit,
new_parents_expression: &Rc<RevsetExpression>,
new_parents: Vec<Commit>,
new_children: Vec<Commit>,
) -> Result<NewParentsAndChildren, CommandError> {
let new_parents: Vec<_> = new_parents
.into_iter()
.flat_map(|c| {
if &c == old_commit {
old_commit.parents().clone()
} else {
[c].to_vec()
}
})
.collect();

let old_commit_expression = RevsetExpression::commit(old_commit.id().clone());
let old_commit_children: Vec<Commit> = old_commit_expression
.children()
.evaluate_programmatic(workspace_command.repo().as_ref())?
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;

let new_children: Vec<_> = new_children
.into_iter()
.flat_map(|c| {
if &c == old_commit {
old_commit_children.clone()
} else {
[c].to_vec()
}
})
.collect();

let new_children_to_rebase: Vec<_> = new_children
.into_iter()
.map(|child_commit| {
let new_child_parents_expression =
RevsetExpression::commits(child_commit.parent_ids().to_owned())
.minus(new_parents_expression)
.minus(&old_commit_expression);

let new_child_parents: Vec<Commit> = new_child_parents_expression
.evaluate_programmatic(workspace_command.repo().as_ref())
.unwrap()
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;

Ok::<_, BackendError>((child_commit, new_child_parents))
})
.try_collect()?;

Ok((new_parents, new_children_to_rebase))
}

/// Moves `commit` from its current location to a new location, given by the set
/// of `new_parents` and `new_children_with_parents`.
fn move_commit(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
new_parents: &[Commit],
new_children_with_parents: &[(Commit, Vec<Commit>)],
commit: Commit,
) -> Result<(), CommandError> {
let mut tx = workspace_command.start_transaction();
// Extract `commit` from its previous location by rebasing its children
// onto its parents.
let (rebased_commit_ids, mut num_rebased_descendants) =
extract_commit(&mut tx, settings, &commit)?;

// We now update `new_parents` to account for the rebase of all of
// `commit`'s descendants. Even if some of the original `new_parents`
// were descendants of `old_commit`, this will no longer be the case after
// the update.
//
// See comment in `rebase_revision` for the bug in this code.
let new_parents: Vec<_> = new_parents
.iter()
.map(|new_parent| get_possibly_rewritten_commit(&mut tx, &rebased_commit_ids, new_parent))
.try_collect()?;

// Finally, it's safe to rebase `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
// have been abandoned.
let new_commit = if commit.parents() == new_parents {
write!(ui.stderr(), "Skipping rebase of commit ")?;
tx.write_commit_summary(ui.stderr_formatter().as_mut(), &commit)?;
writeln!(ui.stderr())?;
commit.clone()
} else {
let new_commit = rebase_commit(settings, tx.mut_repo(), &commit, &new_parents)?;
num_rebased_descendants += 1;
new_commit
};

// Now, rebase all the new children onto the newly rebased commit.
// TODO: Make this no-op if the new children already have the required parents.
let new_children_with_parents: Vec<_> = new_children_with_parents
.iter()
.map(|(commit, parents)| {
// The child commit itself could have been rewritten.
let commit = get_possibly_rewritten_commit(&mut tx, &rebased_commit_ids, commit)?;

let mut new_parents: Vec<_> = parents
.iter()
.map(|parent| get_possibly_rewritten_commit(&mut tx, &rebased_commit_ids, parent))
.try_collect()?;
new_parents.push(new_commit.clone());

Ok::<_, BackendError>((commit, new_parents))
})
.try_collect()?;

let (_, new_num_rebased_descendants) =
rebase_onto_new_parents(&mut tx, settings, &new_children_with_parents)?;

// TODO: the number of descendants isn't quite right. Do we care if counts are
// repeated?
let num_rebased_descendants = num_rebased_descendants + new_num_rebased_descendants;
if num_rebased_descendants > 0 {
writeln!(ui.stderr(), "Rebased {num_rebased_descendants} commits")?;
}
if tx.mut_repo().has_changes() {
tx.finish(ui, format!("move commit {}", commit.id().hex()))
} else {
Ok(()) // Do not print "Nothing changed."
}
}

/// Extracts `commit` from the graph by rebasing its children on its parents.
/// This assumes that it can be rewritten.
fn extract_commit(
Expand Down Expand Up @@ -518,6 +812,28 @@ fn get_possibly_rewritten_commit(
})
}

/// Ensure that there is no possible cycle between the potential children and
/// parents of a rebased commit.
fn ensure_no_commit_loop(
repo: &ReadonlyRepo,
children_expression: &Rc<RevsetExpression>,
parents_expression: &Rc<RevsetExpression>,
) -> Result<(), CommandError> {
if let Some(commit_id) = children_expression
.dag_range_to(parents_expression)
.evaluate_programmatic(repo)?
.iter()
.next()
{
return Err(user_error(format!(
"Refusing to create a loop: commit {} would be both an ancestor and a descendant of \
the rebased commit",
short_commit_hash(&commit_id),
)));
}
Ok(())
}

fn check_rebase_destinations(
repo: &Arc<ReadonlyRepo>,
new_parents: &[Commit],
Expand Down
4 changes: 3 additions & 1 deletion cli/tests/[email protected]
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1477,14 +1477,16 @@ J J
If a working-copy commit gets abandoned, it will be given a new, empty
commit. This is true in general; it is not specific to this command.
**Usage:** `jj rebase [OPTIONS] --destination <DESTINATION>`
**Usage:** `jj rebase [OPTIONS] <--destination <DESTINATION>|--insert-after <INSERT_AFTER>|--insert-before <INSERT_BEFORE>>`
###### **Options:**
* `-b`, `--branch <BRANCH>` — Rebase the whole branch relative to destination's ancestors (can be repeated)
* `-s`, `--source <SOURCE>` — Rebase specified revision(s) together their tree of descendants (can be repeated)
* `-r`, `--revision <REVISION>` — Rebase only this revision, rebasing descendants onto this revision's parent(s)
* `-d`, `--destination <DESTINATION>` — The revision(s) to rebase onto (can be repeated to create a merge commit)
* `-A`, `--insert-after <INSERT_AFTER>` — The revision(s) to insert after (can be repeated to create a merge commit)
* `-B`, `--insert-before <INSERT_BEFORE>` — The revision(s) to insert before (can be repeated to create a merge commit)
* `--skip-empty` — If true, when rebasing would produce an empty commit, the commit is abandoned. It will not be abandoned if it was already empty before the rebase. Will never skip merge commits with multiple non-empty parents
Possible values: `true`, `false`
Expand Down
Loading

0 comments on commit 1a4f7df

Please sign in to comment.