cleanup: propagate errors from Commit::parents()

I updated callers to also propagate the error where it was trivial.

cli/src/cli_util.rs

@@ -1238,7 +1238,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
                 write!(formatter, "Working copy now at: ")?;
                 formatter.with_label("working_copy", |fmt| template.format(new_commit, fmt))?;
                 writeln!(formatter)?;
-                for parent in new_commit.parents() {
+                for parent in new_commit.parents()? {
                     //                "Working copy now at: "
                     write!(formatter, "Parent commit      : ")?;
                     template.format(&parent, formatter.as_mut())?;

cli/src/commands/commit.rs

@@ -61,7 +61,8 @@ pub(crate) fn cmd_commit(
     let diff_selector =
         workspace_command.diff_selector(ui, args.tool.as_deref(), args.interactive)?;
     let mut tx = workspace_command.start_transaction();
-    let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;
+    let parents = commit.parents()?;
+    let base_tree = merge_commit_trees(tx.repo(), &parents)?;
     let instructions = format!(
         "\
 You are splitting the working-copy commit: {}

cli/src/commands/diff.rs

@@ -72,7 +72,7 @@ pub(crate) fn cmd_diff(
     } else {
         let commit = workspace_command
             .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
-        let parents = commit.parents();
+        let parents = commit.parents()?;
         from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &parents)?;
         to_tree = commit.tree()?
     }

cli/src/commands/diffedit.rs

@@ -81,7 +81,7 @@ pub(crate) fn cmd_diffedit(
     } else {
         target_commit = workspace_command
             .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
-        base_commits = target_commit.parents();
+        base_commits = target_commit.parents()?;
         diff_description = "The diff initially shows the commit's changes.".to_string();
     };
     workspace_command.check_rewritable([target_commit.id()])?;

cli/src/commands/rebase.rs

@@ -394,7 +394,7 @@ fn rebase_descendants_transaction(
     workspace_command.check_rewritable(old_commits.iter().ids())?;
     let (skipped_commits, old_commits) = old_commits
         .iter()
-        .partition::<Vec<_>, _>(|commit| commit.parents() == new_parents);
+        .partition::<Vec<_>, _>(|commit| commit.parents().unwrap() == new_parents);
     let num_skipped_rebases = skipped_commits.len();
     if num_skipped_rebases > 0 {
         writeln!(

cli/src/commands/restore.rs

@@ -94,7 +94,8 @@ pub(crate) fn cmd_restore(
     } else {
         to_commit = workspace_command
             .resolve_single_rev(args.changes_in.as_ref().unwrap_or(&RevisionArg::AT))?;
-        from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &to_commit.parents())?;
+        let to_commit_parents = to_commit.parents()?;
+        from_tree = merge_commit_trees(workspace_command.repo().as_ref(), &to_commit_parents)?;
     }
     workspace_command.check_rewritable([to_commit.id()])?;
 

cli/src/commands/split.rs

@@ -76,7 +76,8 @@ pub(crate) fn cmd_split(
     )?;
     let mut tx = workspace_command.start_transaction();
     let end_tree = commit.tree()?;
-    let base_tree = merge_commit_trees(tx.repo(), &commit.parents())?;
+    let parents = commit.parents()?;
+    let base_tree = merge_commit_trees(tx.repo(), &parents)?;
     let instructions = format!(
         "\
 You are splitting a commit into two: {}

cli/src/commands/squash.rs

@@ -108,7 +108,7 @@ pub(crate) fn cmd_squash(
     } else {
         let source = workspace_command
             .resolve_single_rev(args.revision.as_ref().unwrap_or(&RevisionArg::AT))?;
-        let mut parents = source.parents();
+        let mut parents = source.parents()?;
         if parents.len() != 1 {
             return Err(user_error("Cannot squash merge commits"));
         }
@@ -191,7 +191,8 @@ pub fn move_diff(
     }
     let mut source_commits = vec![];
     for source in sources {
-        let parent_tree = merge_commit_trees(tx.repo(), &source.parents())?;
+        let source_parents = source.parents()?;
+        let parent_tree = merge_commit_trees(tx.repo(), &source_parents)?;
         let source_tree = source.tree()?;
         let instructions = format!(
             "\

cli/src/commands/status.rs

@@ -59,7 +59,8 @@ pub(crate) fn cmd_status(
     let formatter = formatter.as_mut();
 
     if let Some(wc_commit) = &maybe_wc_commit {
-        let parent_tree = merge_commit_trees(repo.as_ref(), &wc_commit.parents())?;
+        let wc_commit_parents = wc_commit.parents()?;
+        let parent_tree = merge_commit_trees(repo.as_ref(), &wc_commit_parents)?;
         let tree = wc_commit.tree()?;
         if tree.id() == parent_tree.id() {
             writeln!(formatter, "The working copy is clean")?;
@@ -87,7 +88,7 @@ pub(crate) fn cmd_status(
         write!(formatter, "Working copy : ")?;
         formatter.with_label("working_copy", |fmt| template.format(wc_commit, fmt))?;
         writeln!(formatter)?;
-        for parent in wc_commit.parents() {
+        for parent in wc_commit.parents()? {
             write!(formatter, "Parent commit: ")?;
             template.format(&parent, formatter)?;
             writeln!(formatter)?;

cli/src/commands/unsquash.rs

@@ -58,18 +58,20 @@ pub(crate) fn cmd_unsquash(
     let mut workspace_command = command.workspace_helper(ui)?;
     let commit = workspace_command.resolve_single_rev(&args.revision)?;
     workspace_command.check_rewritable([commit.id()])?;
-    if commit.parent_ids().len() > 1 {
+    let mut parents = commit.parents()?;
+    if parents.len() > 1 {
         return Err(user_error("Cannot unsquash merge commits"));
     }
-    let parent = commit.parents().pop().unwrap();
+    let parent = parents.pop().unwrap();
     workspace_command.check_rewritable([parent.id()])?;
     let interactive_editor = if args.tool.is_some() || args.interactive {
         Some(workspace_command.diff_editor(ui, args.tool.as_deref())?)
     } else {
         None
     };
     let mut tx = workspace_command.start_transaction();
-    let parent_base_tree = merge_commit_trees(tx.repo(), &parent.parents())?;
+    let grandparents = parent.parents()?;
+    let parent_base_tree = merge_commit_trees(tx.repo(), &grandparents)?;
     let new_parent_tree_id;
     if let Some(diff_editor) = &interactive_editor {
         let instructions = format!(

cli/src/commands/workspace.rs

@@ -198,7 +198,7 @@ fn cmd_workspace_add(
             .view()
             .get_wc_commit_id(old_workspace_command.workspace_id())
         {
-            tx.repo().store().get_commit(old_wc_commit_id)?.parents()
+            tx.repo().store().get_commit(old_wc_commit_id)?.parents()?
         } else {
             vec![tx.repo().store().root_commit()]
         }

cli/src/commit_templater.rs

@@ -467,7 +467,7 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
         "parents",
         |_language, _build_ctx, self_property, function| {
             template_parser::expect_no_arguments(function)?;
-            let out_property = self_property.map(|commit| commit.parents());
+            let out_property = self_property.map(|commit| commit.parents().unwrap());
             Ok(L::wrap_commit_list(out_property))
         },
     );
@@ -646,10 +646,11 @@ fn builtin_commit_methods<'repo>() -> CommitTemplateBuildMethodFnMap<'repo, Comm
         template_parser::expect_no_arguments(function)?;
         let repo = language.repo;
         let out_property = self_property.and_then(|commit| {
-            if let [parent] = &commit.parents()[..] {
+            let parents = commit.parents()?;
+            if let [parent] = &parents[..] {
                 return Ok(parent.tree_id() == commit.tree_id());
             }
-            let parent_tree = rewrite::merge_commit_trees(repo, &commit.parents())?;
+            let parent_tree = rewrite::merge_commit_trees(repo, &parents)?;
             Ok(*commit.tree_id() == parent_tree.id())
         });
         Ok(L::wrap_boolean(out_property))

cli/src/diff_util.rs

@@ -237,7 +237,7 @@ pub fn show_patch(
     matcher: &dyn Matcher,
     formats: &[DiffFormat],
 ) -> Result<(), CommandError> {
-    let parents = commit.parents();
+    let parents = commit.parents()?;
     let from_tree = rewrite::merge_commit_trees(workspace_command.repo().as_ref(), &parents)?;
     let to_tree = commit.tree()?;
     show_diff(

lib/src/commit.rs

@@ -80,11 +80,11 @@ impl Commit {
         &self.data.parents
     }
 
-    pub fn parents(&self) -> Vec<Commit> {
+    pub fn parents(&self) -> BackendResult<Vec<Commit>> {
         self.data
             .parents
             .iter()
-            .map(|id| self.store.get_commit(id).unwrap())
+            .map(|id| self.store.get_commit(id))
             .collect()
     }
 
@@ -138,13 +138,13 @@ impl Commit {
 
     /// A commit is discardable if it has one parent, no change from its
     /// parent, and an empty description.
-    pub fn is_discardable(&self) -> bool {
+    pub fn is_discardable(&self) -> BackendResult<bool> {
         if self.description().is_empty() {
-            if let [parent_commit] = &*self.parents() {
-                return self.tree_id() == parent_commit.tree_id();
+            if let [parent_commit] = &*self.parents()? {
+                return Ok(self.tree_id() == parent_commit.tree_id());
             }
         }
-        false
+        Ok(false)
     }
 
     /// A quick way to just check if a signature is present.

lib/src/default_index/revset_engine.rs

@@ -1060,7 +1060,7 @@ fn has_diff_from_parent(
     matcher: &dyn Matcher,
 ) -> bool {
     let commit = store.get_commit(&entry.commit_id()).unwrap();
-    let parents = commit.parents();
+    let parents = commit.parents().unwrap();
     if let [parent] = parents.as_slice() {
         // Fast path: no need to load the root tree
         let unchanged = commit.tree_id() == parent.tree_id();

lib/src/repo.rs

@@ -1137,7 +1137,7 @@ impl MutableRepo {
             |commit| -> Vec<BackendResult<Commit>> {
                 visited.insert(commit.id().clone());
                 let mut dependents = vec![];
-                for parent in commit.parents() {
+                for parent in commit.parents().unwrap() {
                     if let Some((_, targets)) = self.parent_mapping.get(parent.id()) {
                         for target in targets {
                             if to_visit_set.contains(target) && !visited.contains(target) {
@@ -1327,7 +1327,7 @@ impl MutableRepo {
                 .store()
                 .get_commit(&wc_commit_id)
                 .map_err(EditCommitError::WorkingCopyCommitNotFound)?;
-            if wc_commit.is_discardable()
+            if wc_commit.is_discardable()?
                 && self
                     .view
                     .with_ref(|v| local_branch_target_ids(v).all(|id| id != wc_commit.id()))
@@ -1734,6 +1734,8 @@ pub enum EditCommitError {
     WorkingCopyCommitNotFound(#[source] BackendError),
     #[error("Cannot rewrite the root commit")]
     RewriteRootCommit,
+    #[error(transparent)]
+    BackendError(#[from] BackendError),
 }
 
 /// Error from attempts to check out a commit

lib/src/rewrite.rs

@@ -199,7 +199,7 @@ impl<'repo> CommitRewriter<'repo> {
         settings: &UserSettings,
         empty: EmptyBehaviour,
     ) -> BackendResult<Option<CommitBuilder<'repo>>> {
-        let old_parents = self.old_commit.parents();
+        let old_parents = self.old_commit.parents()?;
         let old_parent_trees = old_parents
             .iter()
             .map(|parent| parent.tree_id().clone())
@@ -306,8 +306,10 @@ pub fn rebase_to_dest_parent(
     if source.parent_ids() == destination.parent_ids() {
         Ok(source.tree()?)
     } else {
-        let destination_parent_tree = merge_commit_trees(repo, &destination.parents())?;
-        let source_parent_tree = merge_commit_trees(repo, &source.parents())?;
+        let destination_parents = destination.parents()?;
+        let destination_parent_tree = merge_commit_trees(repo, &destination_parents)?;
+        let source_parents = source.parents()?;
+        let source_parent_tree = merge_commit_trees(repo, &source_parents)?;
         let source_tree = source.tree()?;
         let rebased_tree = destination_parent_tree.merge(&source_parent_tree, &source_tree)?;
         Ok(rebased_tree)
@@ -320,7 +322,8 @@ pub fn back_out_commit(
     old_commit: &Commit,
     new_parents: &[Commit],
 ) -> BackendResult<Commit> {
-    let old_base_tree = merge_commit_trees(mut_repo, &old_commit.parents())?;
+    let old_parents = old_commit.parents()?;
+    let old_base_tree = merge_commit_trees(mut_repo, &old_parents)?;
     let new_base_tree = merge_commit_trees(mut_repo, new_parents)?;
     let old_tree = old_commit.tree()?;
     let new_tree = new_base_tree.merge(&old_tree, &old_base_tree)?;

lib/tests/test_commit_builder.rs

@@ -77,7 +77,7 @@ fn test_initial(backend: TestRepoBackend) {
     let commit = builder.write().unwrap();
     tx.commit("test");
 
-    assert_eq!(commit.parents(), vec![store.root_commit()]);
+    assert_eq!(commit.parents().unwrap(), vec![store.root_commit()]);
     assert_eq!(commit.predecessors(), vec![]);
     assert_eq!(commit.description(), "description");
     assert_eq!(commit.author(), &author_signature);
@@ -152,7 +152,10 @@ fn test_rewrite(backend: TestRepoBackend) {
         .unwrap();
     tx.mut_repo().rebase_descendants(&settings).unwrap();
     tx.commit("test");
-    assert_eq!(rewritten_commit.parents(), vec![store.root_commit()]);
+    assert_eq!(
+        rewritten_commit.parents().unwrap(),
+        vec![store.root_commit()]
+    );
     assert_eq!(
         rewritten_commit.predecessors(),
         vec![initial_commit.clone()]

lib/tests/test_mut_repo.rs

@@ -56,8 +56,8 @@ fn test_checkout() {
         .check_out(ws_id.clone(), &settings, &wc_commit_parent)
         .unwrap();
     assert_eq!(wc_commit.tree_id(), wc_commit_parent.tree_id());
-    assert_eq!(wc_commit.parents().len(), 1);
-    assert_eq!(wc_commit.parents()[0].id(), wc_commit_parent.id());
+    assert_eq!(wc_commit.parent_ids().len(), 1);
+    assert_eq!(&wc_commit.parent_ids()[0], wc_commit_parent.id());
     let repo = tx.commit("test");
     assert_eq!(repo.view().get_wc_commit_id(&ws_id), Some(wc_commit.id()));
 }