tree: flatten TreeMergeError into BackendError

cli/src/command_error.rs

@@ -30,7 +30,6 @@ use jj_lib::revset::{
     RevsetEvaluationError, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError,
 };
 use jj_lib::signing::SignInitError;
-use jj_lib::tree::TreeMergeError;
 use jj_lib::working_copy::{ResetError, SnapshotError, WorkingCopyStateError};
 use jj_lib::workspace::WorkspaceInitError;
 use thiserror::Error;
@@ -306,16 +305,6 @@ want this file to be snapshotted. Otherwise add it to your `.gitignore` file."#,
     }
 }
 
-impl From<TreeMergeError> for CommandError {
-    fn from(err: TreeMergeError) -> Self {
-        let kind = match &err {
-            TreeMergeError::BackendError(BackendError::Unsupported(_)) => CommandErrorKind::User,
-            _ => CommandErrorKind::Internal,
-        };
-        CommandError::with_message(kind, "Merge failed", err)
-    }
-}
-
 impl From<OpStoreError> for CommandError {
     fn from(err: OpStoreError) -> Self {
         internal_error_with_message("Failed to load an operation", err)

cli/tests/test_abandon_command.rs

@@ -316,8 +316,7 @@ fn test_bug_2600_rootcommit_special_case() {
     // Now, the test
     let stderr = test_env.jj_cmd_failure(&repo_path, &["abandon", "base"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Merge failed
-    Caused by: The Git backend does not support creating merge commits with the root commit as one of the parents.
+    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,8 +515,7 @@ fn test_rebase_multiple_destinations() {
         &["rebase", "-r", "a", "-d", "b", "-d", "root()"],
     );
     insta::assert_snapshot!(stderr, @r###"
-    Error: Merge failed
-    Caused by: The Git backend does not support creating merge commits with the root commit as one of the parents.
+    Error: The Git backend does not support creating merge commits with the root commit as one of the parents.
     "###);
 }
 

lib/src/merged_tree.rs

@@ -31,7 +31,7 @@ use crate::matchers::{EverythingMatcher, Matcher};
 use crate::merge::{Merge, MergeBuilder, MergedTreeValue};
 use crate::repo_path::{RepoPath, RepoPathBuf, RepoPathComponent, RepoPathComponentsIter};
 use crate::store::Store;
-use crate::tree::{try_resolve_file_conflict, Tree, TreeMergeError};
+use crate::tree::{try_resolve_file_conflict, Tree};
 use crate::tree_builder::TreeBuilder;
 use crate::{backend, tree};
 
@@ -184,7 +184,7 @@ impl MergedTree {
     /// automatically resolved and leaving the rest unresolved. The returned
     /// conflict will either be resolved or have the same number of sides as
     /// the input.
-    pub fn resolve(&self) -> Result<Merge<Tree>, TreeMergeError> {
+    pub fn resolve(&self) -> BackendResult<Merge<Tree>> {
         match self {
             MergedTree::Legacy(tree) => Ok(Merge::resolved(tree.clone())),
             MergedTree::Merge(trees) => merge_trees(trees),
@@ -362,19 +362,15 @@ impl MergedTree {
     }
 
     /// Merges this tree with `other`, using `base` as base.
-    pub fn merge(
-        &self,
-        base: &MergedTree,
-        other: &MergedTree,
-    ) -> Result<MergedTree, TreeMergeError> {
+    pub fn merge(&self, base: &MergedTree, other: &MergedTree) -> BackendResult<MergedTree> {
         if let (MergedTree::Legacy(this), MergedTree::Legacy(base), MergedTree::Legacy(other)) =
             (self, base, other)
         {
             let merged_tree = tree::merge_trees(this, base, other)?;
             Ok(MergedTree::legacy(merged_tree))
         } else {
             // Convert legacy trees to merged trees and unwrap to `Merge<Tree>`
-            let to_merge = |tree: &MergedTree| -> Result<Merge<Tree>, TreeMergeError> {
+            let to_merge = |tree: &MergedTree| -> BackendResult<Merge<Tree>> {
                 match tree {
                     MergedTree::Legacy(tree) => {
                         let MergedTree::Merge(tree) = MergedTree::from_legacy_tree(tree.clone())?
@@ -475,7 +471,7 @@ fn trees_value<'a>(trees: &'a Merge<Tree>, basename: &RepoPathComponent) -> Merg
     MergedTreeVal::Conflict(value.map(|x| x.cloned()))
 }
 
-fn merge_trees(merge: &Merge<Tree>) -> Result<Merge<Tree>, TreeMergeError> {
+fn merge_trees(merge: &Merge<Tree>) -> BackendResult<Merge<Tree>> {
     if let Some(tree) = merge.resolve_trivial() {
         return Ok(Merge::resolved(tree.clone()));
     }
@@ -529,7 +525,7 @@ fn merge_tree_values(
     store: &Arc<Store>,
     path: &RepoPath,
     values: MergedTreeValue,
-) -> Result<MergedTreeValue, TreeMergeError> {
+) -> BackendResult<MergedTreeValue> {
     if let Some(resolved) = values.resolve_trivial() {
         return Ok(Merge::resolved(resolved.clone()));
     }

lib/src/repo.rs

@@ -57,7 +57,6 @@ use crate::simple_op_store::SimpleOpStore;
 use crate::store::Store;
 use crate::submodule_store::SubmoduleStore;
 use crate::transaction::Transaction;
-use crate::tree::TreeMergeError;
 use crate::view::View;
 use crate::{backend, dag_walk, op_store, revset};
 
@@ -580,7 +579,7 @@ pub fn read_store_type_compat(
 #[derive(Debug, Error)]
 pub enum RepoLoaderError {
     #[error(transparent)]
-    TreeMerge(#[from] TreeMergeError),
+    Backend(#[from] BackendError),
     #[error(transparent)]
     OpHeadResolution(#[from] OpHeadResolutionError),
     #[error(transparent)]
@@ -962,7 +961,7 @@ impl MutableRepo {
         &'repo mut self,
         settings: &'settings UserSettings,
         options: RebaseOptions,
-    ) -> Result<Option<DescendantRebaser<'settings, 'repo>>, TreeMergeError> {
+    ) -> BackendResult<Option<DescendantRebaser<'settings, 'repo>>> {
         if !self.has_rewrites() {
             // Optimization
             return Ok(None);
@@ -980,7 +979,7 @@ impl MutableRepo {
         &mut self,
         settings: &UserSettings,
         options: RebaseOptions,
-    ) -> Result<usize, TreeMergeError> {
+    ) -> BackendResult<usize> {
         let result = self
             .rebase_descendants_return_rebaser(settings, options)?
             .map_or(0, |rebaser| rebaser.into_map().len());
@@ -1005,7 +1004,7 @@ impl MutableRepo {
         &mut self,
         settings: &UserSettings,
         options: RebaseOptions,
-    ) -> Result<HashMap<CommitId, CommitId>, TreeMergeError> {
+    ) -> BackendResult<HashMap<CommitId, CommitId>> {
         let result = Ok(self
             // We do not set RebaseOptions here, since this function does not currently return
             // enough information to describe the results of a rebase if some commits got
@@ -1016,14 +1015,14 @@ impl MutableRepo {
         result
     }
 
-    pub fn rebase_descendants(&mut self, settings: &UserSettings) -> Result<usize, TreeMergeError> {
+    pub fn rebase_descendants(&mut self, settings: &UserSettings) -> BackendResult<usize> {
         self.rebase_descendants_with_options(settings, Default::default())
     }
 
     pub fn rebase_descendants_return_map(
         &mut self,
         settings: &UserSettings,
-    ) -> Result<HashMap<CommitId, CommitId>, TreeMergeError> {
+    ) -> BackendResult<HashMap<CommitId, CommitId>> {
         self.rebase_descendants_with_options_return_map(settings, Default::default())
     }
 

lib/src/rewrite.rs

@@ -35,13 +35,9 @@ use crate::repo_path::RepoPath;
 use crate::revset::{RevsetExpression, RevsetIteratorExt};
 use crate::settings::UserSettings;
 use crate::store::Store;
-use crate::tree::TreeMergeError;
 
 #[instrument(skip(repo))]
-pub fn merge_commit_trees(
-    repo: &dyn Repo,
-    commits: &[Commit],
-) -> Result<MergedTree, TreeMergeError> {
+pub fn merge_commit_trees(repo: &dyn Repo, commits: &[Commit]) -> BackendResult<MergedTree> {
     merge_commit_trees_without_repo(repo.store(), repo.index(), commits)
 }
 
@@ -50,7 +46,7 @@ pub fn merge_commit_trees_without_repo(
     store: &Arc<Store>,
     index: &dyn Index,
     commits: &[Commit],
-) -> Result<MergedTree, TreeMergeError> {
+) -> BackendResult<MergedTree> {
     if commits.is_empty() {
         Ok(store.get_root_tree(&store.empty_merged_tree_id())?)
     } else {
@@ -104,7 +100,7 @@ pub fn rebase_commit(
     mut_repo: &mut MutableRepo,
     old_commit: &Commit,
     new_parents: &[Commit],
-) -> Result<Commit, TreeMergeError> {
+) -> BackendResult<Commit> {
     let rebased_commit = rebase_commit_with_options(
         settings,
         mut_repo,
@@ -129,7 +125,7 @@ pub fn rebase_commit_with_options(
     old_commit: &Commit,
     new_parents: &[Commit],
     options: &RebaseOptions,
-) -> Result<RebasedCommit, TreeMergeError> {
+) -> BackendResult<RebasedCommit> {
     // If specified, don't create commit where one parent is an ancestor of another.
     let simplified_new_parents;
     let new_parents = if options.simplify_ancestor_merge {
@@ -213,7 +209,7 @@ pub fn rebase_to_dest_parent(
     repo: &dyn Repo,
     source: &Commit,
     destination: &Commit,
-) -> Result<MergedTree, TreeMergeError> {
+) -> BackendResult<MergedTree> {
     if source.parent_ids() == destination.parent_ids() {
         Ok(source.tree()?)
     } else {
@@ -230,7 +226,7 @@ pub fn back_out_commit(
     mut_repo: &mut MutableRepo,
     old_commit: &Commit,
     new_parents: &[Commit],
-) -> Result<Commit, TreeMergeError> {
+) -> BackendResult<Commit> {
     let old_base_tree = merge_commit_trees(mut_repo, &old_commit.parents())?;
     let new_base_tree = merge_commit_trees(mut_repo, new_parents)?;
     let old_tree = old_commit.tree()?;
@@ -240,10 +236,10 @@ pub fn back_out_commit(
         .map(|commit| commit.id().clone())
         .collect();
     // TODO: i18n the description based on repo language
-    Ok(mut_repo
+    mut_repo
         .new_commit(settings, new_parent_ids, new_tree.id())
         .set_description(format!("backout of commit {}", &old_commit.id().hex()))
-        .write()?)
+        .write()
 }
 
 #[derive(Clone, Default, PartialEq, Eq, Debug)]
@@ -439,7 +435,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         Ok(())
     }
 
-    fn rebase_one(&mut self, old_commit: Commit) -> Result<(), TreeMergeError> {
+    fn rebase_one(&mut self, old_commit: Commit) -> BackendResult<()> {
         let old_commit_id = old_commit.id().clone();
         assert!(!self.mut_repo.parent_mapping.contains_key(&old_commit_id));
         let old_parent_ids = old_commit.parent_ids();
@@ -528,7 +524,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         self.mut_repo.set_view(view);
     }
 
-    pub fn rebase_all(&mut self) -> Result<(), TreeMergeError> {
+    pub fn rebase_all(&mut self) -> BackendResult<()> {
         while let Some(old_commit) = self.to_visit.pop() {
             self.rebase_one(old_commit)?;
         }

lib/src/tree.rs

@@ -20,11 +20,11 @@ use std::io::Read;
 use std::sync::Arc;
 
 use itertools::Itertools;
-use thiserror::Error;
 use tracing::instrument;
 
 use crate::backend::{
-    BackendError, ConflictId, TreeEntriesNonRecursiveIterator, TreeEntry, TreeId, TreeValue,
+    BackendError, BackendResult, ConflictId, TreeEntriesNonRecursiveIterator, TreeEntry, TreeId,
+    TreeValue,
 };
 use crate::files::MergeResult;
 use crate::matchers::{EverythingMatcher, Matcher};
@@ -34,12 +34,6 @@ use crate::repo_path::{RepoPath, RepoPathBuf, RepoPathComponent, RepoPathCompone
 use crate::store::Store;
 use crate::{backend, files};
 
-#[derive(Debug, Error)]
-pub enum TreeMergeError {
-    #[error(transparent)]
-    BackendError(#[from] BackendError),
-}
-
 #[derive(Clone)]
 pub struct Tree {
     store: Arc<Store>,
@@ -281,11 +275,7 @@ impl<'trees> Iterator for TreeEntryDiffIterator<'trees> {
     }
 }
 
-pub fn merge_trees(
-    side1_tree: &Tree,
-    base_tree: &Tree,
-    side2_tree: &Tree,
-) -> Result<Tree, TreeMergeError> {
+pub fn merge_trees(side1_tree: &Tree, base_tree: &Tree, side2_tree: &Tree) -> BackendResult<Tree> {
     let store = base_tree.store();
     let dir = base_tree.dir();
     assert_eq!(side1_tree.dir(), dir);
@@ -313,7 +303,7 @@ pub fn merge_trees(
             new_tree.set_or_remove(basename, new_value);
         }
     }
-    Ok(store.write_tree(dir, new_tree)?)
+    store.write_tree(dir, new_tree)
 }
 
 /// Returns `Some(TreeId)` if this is a directory or missing. If it's missing,
@@ -336,7 +326,7 @@ fn merge_tree_value(
     maybe_base: Option<&TreeValue>,
     maybe_side1: Option<&TreeValue>,
     maybe_side2: Option<&TreeValue>,
-) -> Result<Option<TreeValue>, TreeMergeError> {
+) -> BackendResult<Option<TreeValue>> {
     // Resolve non-trivial conflicts:
     //   * resolve tree conflicts by recursing
     //   * try to resolve file conflicts by merging the file contents
@@ -399,7 +389,7 @@ pub fn try_resolve_file_conflict(
     store: &Store,
     filename: &RepoPath,
     conflict: &MergedTreeValue,
-) -> Result<Option<TreeValue>, TreeMergeError> {
+) -> BackendResult<Option<TreeValue>> {
     // If there are any non-file or any missing parts in the conflict, we can't
     // merge it. We check early so we don't waste time reading file contents if
     // we can't merge them anyway. At the same time we determine whether the
@@ -438,7 +428,7 @@ pub fn try_resolve_file_conflict(
     let file_id_conflict = file_id_conflict.simplify();
 
     let contents: Merge<Vec<u8>> =
-        file_id_conflict.try_map(|&file_id| -> Result<Vec<u8>, TreeMergeError> {
+        file_id_conflict.try_map(|&file_id| -> BackendResult<Vec<u8>> {
             let mut content = vec![];
             store
                 .read_file(filename, file_id)?