cli: let backend decide whether merge with root can be performed

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<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],

cli/src/command_error.rs

@@ -72,6 +72,14 @@ impl CommandError {
         }
     }
 
+    pub fn with_message(
+        kind: CommandErrorKind,
+        message: impl Into<String>,
+        source: impl Into<Box<dyn error::Error + Send + Sync>>,
+    ) -> Self {
+        Self::new(kind, ErrorWithMessage::new(message, source))
+    }
+
     /// Returns error with the given plain-text `hint` attached.
     pub fn hinted(mut self, hint: impl Into<String>) -> Self {
         self.add_hint(hint);
@@ -146,7 +154,7 @@ pub fn user_error_with_message(
     message: impl Into<String>,
     source: impl Into<Box<dyn error::Error + Send + Sync>>,
 ) -> CommandError {
-    user_error(ErrorWithMessage::new(message, source))
+    CommandError::with_message(CommandErrorKind::User, message, source)
 }
 
 pub fn config_error(err: impl Into<Box<dyn error::Error + Send + Sync>>) -> CommandError {
@@ -157,7 +165,7 @@ pub fn config_error_with_message(
     message: impl Into<String>,
     source: impl Into<Box<dyn error::Error + Send + Sync>>,
 ) -> CommandError {
-    config_error(ErrorWithMessage::new(message, source))
+    CommandError::with_message(CommandErrorKind::Config, message, source)
 }
 
 pub fn cli_error(err: impl Into<Box<dyn error::Error + Send + Sync>>) -> CommandError {
@@ -172,7 +180,7 @@ pub fn internal_error_with_message(
     message: impl Into<String>,
     source: impl Into<Box<dyn error::Error + Send + Sync>>,
 ) -> CommandError {
-    internal_error(ErrorWithMessage::new(message, source))
+    CommandError::with_message(CommandErrorKind::Internal, message, source)
 }
 
 fn format_similarity_hint<S: AsRef<str>>(candidates: &[S]) -> Option<String> {
@@ -230,7 +238,10 @@ impl From<CheckOutCommitError> for CommandError {
 
 impl From<BackendError> for CommandError {
     fn from(err: BackendError) -> Self {
-        internal_error_with_message("Unexpected error from backend", err)
+        match &err {
+            BackendError::Unsupported(_) => user_error(err),
+            _ => internal_error_with_message("Unexpected error from backend", err),
+        }
     }
 }
 
@@ -297,7 +308,11 @@ want this file to be snapshotted. Otherwise add it to your `.gitignore` file."#,
 
 impl From<TreeMergeError> for CommandError {
     fn from(err: TreeMergeError) -> Self {
-        internal_error_with_message("Merge failed", err)
+        let kind = match &err {
+            TreeMergeError::BackendError(BackendError::Unsupported(_)) => CommandErrorKind::User,
+            _ => CommandErrorKind::Internal,
+        };
+        CommandError::with_message(kind, "Merge failed", err)
     }
 }
 

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;

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`

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()
     };

cli/tests/test_abandon_command.rs

@@ -314,12 +314,10 @@ fn test_bug_2600_rootcommit_special_case() {
     "###);
 
     // Now, the test
-    let stderr = test_env.jj_cmd_internal_error(&repo_path, &["abandon", "base"]);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["abandon", "base"]);
     insta::assert_snapshot!(stderr, @r###"
-    Internal error: Merge failed
-    Caused by:
-    1: Backend error
-    2: The Git backend does not support creating merge commits with the root commit as one of the parents.
+    Error: Merge failed
+    Caused by: The Git backend does not support creating merge commits with the root commit as one of the parents.
     "###);
 }
 

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.
     "###);
 }
 

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.
     "###);
 }
 

lib/src/backend.rs

@@ -200,6 +200,10 @@ pub enum BackendError {
     },
     #[error(transparent)]
     Other(Box<dyn std::error::Error + Send + Sync>),
+    /// A valid operation attempted, but failed because it isn't supported by
+    /// the particular backend.
+    #[error("{0}")]
+    Unsupported(String),
 }
 
 pub type BackendResult<T> = Result<T, BackendError>;

lib/src/git_backend.rs

@@ -1121,10 +1121,10 @@ impl Backend for GitBackend {
                 // there are no other parents since Git cannot represent a merge between a root
                 // commit and another commit.
                 if contents.parents.len() > 1 {
-                    return Err(BackendError::Other(
+                    return Err(BackendError::Unsupported(
                         "The Git backend does not support creating merge commits with the root \
                          commit as one of the parents."
-                            .into(),
+                            .to_owned(),
                     ));
                 }
             } else {
@@ -1702,7 +1702,7 @@ mod tests {
         commit.parents = vec![first_id, backend.root_commit_id().clone()];
         assert_matches!(
             backend.write_commit(commit, None),
-            Err(BackendError::Other(err)) if err.to_string().contains("root commit")
+            Err(BackendError::Unsupported(message)) if message.contains("root commit")
         );
     }
 

lib/src/tree.rs

@@ -41,7 +41,7 @@ pub enum TreeMergeError {
         source: std::io::Error,
         file_id: FileId,
     },
-    #[error("Backend error")]
+    #[error(transparent)]
     BackendError(#[from] BackendError),
 }