From f13f15e057433b21e28a2d1d3c92200de8a0e203 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 25 Aug 2021 13:31:54 -0700 Subject: [PATCH] cli: add `jj rebase -s` and make `jj rebase -r` rebase descendants onto parents I think it makes sense to have a version of rebase that rebases the descendants of the rebased commit onto the parents of the rebased commit. Let's make `jj rebase -r` do just that. Let's also add `jj rebase -s` (matching Mercurial's `hg rebase -s`) for rebasing a commit and its descendants onto another commit. Since both flavors of the command now explicitly rebase the descendants (just to different destinations), I also made the command not evolve orphans afterwards. That would have made sense regardless of this commit. --- README.md | 12 ++++---- src/commands.rs | 74 +++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 71 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index ad58394f9..e13422cfd 100644 --- a/README.md +++ b/README.md @@ -314,8 +314,8 @@ We now have a few commits, where A, B1, and B2 modify the same file, while C modifies a different file. We checked out A in order to simplify the next steps. Let's now rebase B2 directly onto A: ```shell script -$ jj rebase -r 5548374c0794 -d cf49e6bec410 -Rebased 1 descendant commits +$ jj rebase -s 5548374c0794 -d cf49e6bec410 +Rebased 2 commits $ jj l o 66274d5a7d2d 8e6178b84ffb martinvonz@google.com 2021-05-26 12:39:35.000 -07:00 conflict | C @@ -330,8 +330,8 @@ o 661432c51c08 cf49e6bec410 martinvonz@google.com 2021-05-26 12:39:12.000 -07:00 ``` There are several things worth noting here. First, the `jj rebase` command said -"Rebased 1 descendant commits". That's because we asked it to rebase commit B2, -but commit C was on top of it, so it rebased that commit as well. Second, +"Rebased 2 commits". That's because we asked it to rebase commit B2 with the +`-s` option, which also rebases descendants (commit C in this case). Second, because B2 modified the same file (and word) as B1, rebasing it resulted in conflicts, as the `jj l` output indicates. Third, the conflicts did not prevent the rebase from completing successfully, nor did it prevent C from getting @@ -395,8 +395,8 @@ o 1e6dd15305a3 martinvonz@ 2021-05-26 12:52:39.374 -07:00 - 2021-05-26 | check out commit 0c305a9e6b274bc09b2bca85635299dcfdc6811c | args: jj co 0c305a9e6b27 o 401652a2f61e martinvonz@ 2021-05-26 12:44:51.872 -07:00 - 2021-05-26 12:44:51.882 -07:00 -| rebase commit de5690380f40f3f7fc6b7d66d43a4f68ee606228 -| args: jj rebase -r de5690380f40 -d 661432c51c08 +| rebase commit de5690380f40f3f7fc6b7d66d43a4f68ee606228 and descendants +| args: jj rebase -s de5690380f40 -d 661432c51c08 [many more lines] ``` diff --git a/src/commands.rs b/src/commands.rs index 6be9eeba8..61e504688 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -50,7 +50,7 @@ use jujutsu_lib::repo::{ }; use jujutsu_lib::revset::{RevsetError, RevsetExpression, RevsetParseError}; use jujutsu_lib::revset_graph_iterator::RevsetGraphEdgeType; -use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit}; +use jujutsu_lib::rewrite::{back_out_commit, merge_commit_trees, rebase_commit, DescendantRebaser}; use jujutsu_lib::settings::UserSettings; use jujutsu_lib::store::{CommitId, StoreError, Timestamp, TreeValue}; use jujutsu_lib::store_wrapper::StoreWrapper; @@ -868,7 +868,20 @@ fn get_app<'a, 'b>() -> App<'a, 'b> { .arg(message_arg()); let rebase_command = SubCommand::with_name("rebase") .about("Move a commit to a different parent") - .arg(rev_arg()) + .arg( + Arg::with_name("revision") + .long("revision") + .short("r") + .takes_value(true), + ) + .arg( + Arg::with_name("source") + .long("source") + .short("s") + .takes_value(true) + .required(false) + .multiple(false), + ) .arg( Arg::with_name("destination") .long("destination") @@ -2229,18 +2242,61 @@ fn cmd_rebase( command: &CommandHelper, sub_matches: &ArgMatches, ) -> Result<(), CommandError> { - let mut repo_command = command.repo_helper(ui)?; - let commit_to_rebase = repo_command.resolve_revision_arg(ui, sub_matches)?; - repo_command.check_rewriteable(&commit_to_rebase)?; + let mut repo_command = command.repo_helper(ui)?.evolve_orphans(false); let mut parents = vec![]; for revision_str in sub_matches.values_of("destination").unwrap() { let destination = repo_command.resolve_single_rev(ui, revision_str)?; parents.push(destination); } - let mut tx = - repo_command.start_transaction(&format!("rebase commit {}", commit_to_rebase.id().hex())); - rebase_commit(ui.settings(), tx.mut_repo(), &commit_to_rebase, &parents); - repo_command.finish_transaction(ui, tx)?; + // TODO: Unless we want to allow both --revision and --source, is it better to + // replace --source by --rebase-descendants? + if sub_matches.is_present("revision") && sub_matches.is_present("source") { + return Err(CommandError::UserError(String::from( + "--revision cannot be used with --source", + ))); + } + if let Some(source_str) = sub_matches.value_of("source") { + let old_commit = repo_command.resolve_single_rev(ui, source_str)?; + let mut tx = repo_command.start_transaction(&format!( + "rebase commit {} and descendants", + old_commit.id().hex() + )); + repo_command.check_rewriteable(&old_commit)?; + let new_commit = rebase_commit(ui.settings(), tx.mut_repo(), &old_commit, &parents); + let mut rebaser = DescendantRebaser::new( + ui.settings(), + tx.mut_repo(), + old_commit.id().clone(), + vec![new_commit.id().clone()], + ); + rebaser.rebase_all(); + let num_rebased = rebaser.rebased().len() + 1; + writeln!(ui, "Rebased {} commits", num_rebased)?; + repo_command.finish_transaction(ui, tx)?; + } else { + let old_commit = + repo_command.resolve_single_rev(ui, sub_matches.value_of("revision").unwrap_or("@"))?; + let mut tx = + repo_command.start_transaction(&format!("rebase commit {}", old_commit.id().hex())); + repo_command.check_rewriteable(&old_commit)?; + rebase_commit(ui.settings(), tx.mut_repo(), &old_commit, &parents); + let mut rebaser = DescendantRebaser::new( + ui.settings(), + tx.mut_repo(), + old_commit.id().clone(), + old_commit.parent_ids(), + ); + rebaser.rebase_all(); + let num_rebased = rebaser.rebased().len(); + if num_rebased != 0 { + writeln!( + ui, + "Also rebased {} descendant commits onto parent of rebased commit", + num_rebased + )?; + } + repo_command.finish_transaction(ui, tx)?; + } Ok(()) }