Skip to content

Commit

Permalink
cli: let backend decide whether merge with root can be performed
Browse files Browse the repository at this point in the history
One less CLI revset helper. It might look odd that "jj rebase" says "Merge
failed" whereas "jj new" doesn't, but that depends on where the BackendError
is detected.
  • Loading branch information
yuja committed Mar 30, 2024
1 parent f20004f commit 08b5b66
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 27 deletions.
15 changes: 0 additions & 15 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1651,21 +1651,6 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> {
Ok(())
}

/// Resolves revsets into revisions for use; useful for rebases or operations
/// that take multiple parents.
pub fn resolve_all_revs(
workspace_command: &WorkspaceCommandHelper,
revisions: &[RevisionArg],
) -> Result<IndexSet<Commit>, CommandError> {
let commits = resolve_multiple_nonempty_revsets_default_single(workspace_command, revisions)?;
let root_commit_id = workspace_command.repo().store().root_commit_id();
if commits.len() >= 2 && commits.iter().any(|c| c.id() == root_commit_id) {
Err(user_error("Cannot merge with root revision"))
} else {
Ok(commits)
}
}

pub fn resolve_multiple_nonempty_revsets_default_single(
workspace_command: &WorkspaceCommandHelper,
revisions: &[RevisionArg],
Expand Down
11 changes: 7 additions & 4 deletions cli/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
use jj_lib::rewrite::{merge_commit_trees, rebase_commit};
use tracing::instrument;

use crate::cli_util::{self, short_commit_hash, CommandHelper, RevisionArg};
use crate::cli_util::{
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper, RevisionArg,
};
use crate::command_error::{user_error, CommandError};
use crate::description_util::join_message_paragraphs;
use crate::ui::Ui;
Expand Down Expand Up @@ -98,9 +100,10 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
!args.revisions.is_empty(),
"expected a non-empty list from clap"
);
let target_commits = cli_util::resolve_all_revs(&workspace_command, &args.revisions)?
.into_iter()
.collect_vec();
let target_commits =
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.revisions)?
.into_iter()
.collect_vec();
let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec();
let mut tx = workspace_command.start_transaction();
let mut num_rebased;
Expand Down
9 changes: 5 additions & 4 deletions cli/src/commands/rebase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use jj_lib::settings::UserSettings;
use tracing::instrument;

use crate::cli_util::{
self, resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper,
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper,
RevisionArg, WorkspaceCommandHelper,
};
use crate::command_error::{user_error, CommandError};
Expand Down Expand Up @@ -191,9 +191,10 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
simplify_ancestor_merge: false,
};
let mut workspace_command = command.workspace_helper(ui)?;
let new_parents = cli_util::resolve_all_revs(&workspace_command, &args.destination)?
.into_iter()
.collect_vec();
let new_parents =
resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.destination)?
.into_iter()
.collect_vec();
if let Some(rev_str) = &args.revision {
assert_eq!(
// In principle, `-r --skip-empty` could mean to abandon the `-r`
Expand Down
5 changes: 3 additions & 2 deletions cli/src/commands/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ use jj_lib::workspace::Workspace;
use tracing::instrument;

use crate::cli_util::{
self, check_stale_working_copy, print_checkout_stats, short_commit_hash, CommandHelper,
check_stale_working_copy, print_checkout_stats,
resolve_multiple_nonempty_revsets_default_single, short_commit_hash, CommandHelper,
RevisionArg, WorkingCopyFreshness, WorkspaceCommandHelper,
};
use crate::command_error::{internal_error_with_message, user_error, CommandError};
Expand Down Expand Up @@ -202,7 +203,7 @@ fn cmd_workspace_add(
vec![tx.repo().store().root_commit()]
}
} else {
cli_util::resolve_all_revs(&old_workspace_command, &args.revision)?
resolve_multiple_nonempty_revsets_default_single(&old_workspace_command, &args.revision)?
.into_iter()
.collect_vec()
};
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_new_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ fn test_new_merge() {
// merge with root
let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "@", "root()"]);
insta::assert_snapshot!(stderr, @r###"
Error: Cannot merge with root revision
Error: The Git backend does not support creating merge commits with the root commit as one of the parents.
"###);
}

Expand Down
3 changes: 2 additions & 1 deletion cli/tests/test_rebase_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,8 @@ fn test_rebase_multiple_destinations() {
&["rebase", "-r", "a", "-d", "b", "-d", "root()"],
);
insta::assert_snapshot!(stderr, @r###"
Error: Cannot merge with root revision
Error: Merge failed
Caused by: The Git backend does not support creating merge commits with the root commit as one of the parents.
"###);
}

Expand Down

0 comments on commit 08b5b66

Please sign in to comment.