From 08b5b66ad41146491339efff5670cba4a81e56f4 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Fri, 29 Mar 2024 20:17:45 +0900 Subject: [PATCH] cli: let backend decide whether merge with root can be performed 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. --- cli/src/cli_util.rs | 15 --------------- cli/src/commands/new.rs | 11 +++++++---- cli/src/commands/rebase.rs | 9 +++++---- cli/src/commands/workspace.rs | 5 +++-- cli/tests/test_new_command.rs | 2 +- cli/tests/test_rebase_command.rs | 3 ++- 6 files changed, 18 insertions(+), 27 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 8ba78e29f..be61f4ad1 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -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, 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], diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 54b5ee8a2..de7d670f9 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -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; @@ -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; diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index cdf13fbc2..af3991b71 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -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}; @@ -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` diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs index d2a4328f4..916028124 100644 --- a/cli/src/commands/workspace.rs +++ b/cli/src/commands/workspace.rs @@ -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}; @@ -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() }; diff --git a/cli/tests/test_new_command.rs b/cli/tests/test_new_command.rs index 1d30be8e3..55462dc56 100644 --- a/cli/tests/test_new_command.rs +++ b/cli/tests/test_new_command.rs @@ -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. "###); } diff --git a/cli/tests/test_rebase_command.rs b/cli/tests/test_rebase_command.rs index 4f2d3b307..8d22cdaf7 100644 --- a/cli/tests/test_rebase_command.rs +++ b/cli/tests/test_rebase_command.rs @@ -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. "###); }