repo: add MutableRepo::new_commit() returning CommitBuilder

Since `CommitBuilder` now has a reference to `MutableRepo`, it's
convenient to create instances of it by calling a method on
`MutableRepo`.

lib/src/commit_builder.rs

@@ -20,6 +20,7 @@ use crate::commit::Commit;
 use crate::repo::MutableRepo;
 use crate::settings::UserSettings;
 
+#[must_use]
 pub struct CommitBuilder<'repo> {
     mut_repo: &'repo mut MutableRepo,
     commit: backend::Commit,

lib/src/repo.rs

@@ -24,7 +24,7 @@ use once_cell::sync::OnceCell;
 use thiserror::Error;
 
 use self::dirty_cell::DirtyCell;
-use crate::backend::{Backend, BackendError, BackendResult, ChangeId, CommitId};
+use crate::backend::{Backend, BackendError, BackendResult, ChangeId, CommitId, TreeId};
 use crate::commit::Commit;
 use crate::commit_builder::CommitBuilder;
 use crate::dag_walk::topo_order_reverse;
@@ -617,6 +617,15 @@ impl MutableRepo {
         (self.index, self.view.into_inner())
     }
 
+    pub fn new_commit(
+        &mut self,
+        settings: &UserSettings,
+        parents: Vec<CommitId>,
+        tree_id: TreeId,
+    ) -> CommitBuilder {
+        CommitBuilder::for_new_commit(self, settings, parents, tree_id)
+    }
+
     pub fn write_commit(&mut self, commit: backend::Commit) -> BackendResult<Commit> {
         let commit = self.store().write_commit(commit)?;
         self.add_head(&commit);
@@ -707,13 +716,13 @@ impl MutableRepo {
         commit: &Commit,
     ) -> BackendResult<Commit> {
         self.leave_commit(&workspace_id);
-        let wc_commit = CommitBuilder::for_new_commit(
-            self,
-            settings,
-            vec![commit.id().clone()],
-            commit.tree_id().clone(),
-        )
-        .write()?;
+        let wc_commit = self
+            .new_commit(
+                settings,
+                vec![commit.id().clone()],
+                commit.tree_id().clone(),
+            )
+            .write()?;
         self.set_wc_commit(workspace_id, wc_commit.id().clone())
             .unwrap();
         Ok(wc_commit)

lib/src/rewrite.rs

@@ -104,7 +104,8 @@ pub fn back_out_commit(
         .map(|commit| commit.id().clone())
         .collect();
     // TODO: i18n the description based on repo language
-    CommitBuilder::for_new_commit(mut_repo, settings, new_parent_ids, new_tree_id)
+    mut_repo
+        .new_commit(settings, new_parent_ids, new_tree_id)
         .set_description(format!("backout of commit {}", &old_commit.id().hex()))
         .write()
 }
@@ -351,13 +352,13 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
         let new_wc_commit = if edit {
             new_commit
         } else {
-            CommitBuilder::for_new_commit(
-                self.mut_repo,
-                self.settings,
-                vec![new_commit.id().clone()],
-                new_commit.tree_id().clone(),
-            )
-            .write()?
+            self.mut_repo
+                .new_commit(
+                    self.settings,
+                    vec![new_commit.id().clone()],
+                    new_commit.tree_id().clone(),
+                )
+                .write()?
         };
         for workspace_id in workspaces_to_update.into_iter() {
             self.mut_repo.edit(workspace_id, &new_wc_commit).unwrap();

lib/tests/test_commit_builder.rs

@@ -39,14 +39,15 @@ fn test_initial(use_git: bool) {
     );
 
     let mut tx = repo.start_transaction(&settings, "test");
-    let commit = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![store.root_commit_id().clone()],
-        tree.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit = tx
+        .mut_repo()
+        .new_commit(
+            &settings,
+            vec![store.root_commit_id().clone()],
+            tree.id().clone(),
+        )
+        .write()
+        .unwrap();
     tx.commit();
 
     assert_eq!(commit.parents(), vec![store.root_commit()]);
@@ -88,14 +89,15 @@ fn test_rewrite(use_git: bool) {
     );
 
     let mut tx = repo.start_transaction(&settings, "test");
-    let initial_commit = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![store.root_commit_id().clone()],
-        initial_tree.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let initial_commit = tx
+        .mut_repo()
+        .new_commit(
+            &settings,
+            vec![store.root_commit_id().clone()],
+            initial_tree.id().clone(),
+        )
+        .write()
+        .unwrap();
     let repo = tx.commit();
 
     let rewritten_tree = testutils::create_tree(
@@ -171,14 +173,15 @@ fn test_rewrite_update_missing_user(use_git: bool) {
     let repo = &test_repo.repo;
 
     let mut tx = repo.start_transaction(&missing_user_settings, "test");
-    let initial_commit = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &missing_user_settings,
-        vec![repo.store().root_commit_id().clone()],
-        repo.store().empty_tree_id().clone(),
-    )
-    .write()
-    .unwrap();
+    let initial_commit = tx
+        .mut_repo()
+        .new_commit(
+            &missing_user_settings,
+            vec![repo.store().root_commit_id().clone()],
+            repo.store().empty_tree_id().clone(),
+        )
+        .write()
+        .unwrap();
     assert_eq!(initial_commit.author().name, "(no name configured)");
     assert_eq!(initial_commit.author().email, "(no email configured)");
     assert_eq!(initial_commit.committer().name, "(no name configured)");
@@ -226,14 +229,14 @@ fn test_commit_builder_descendants(use_git: bool) {
 
     // Test with for_new_commit()
     let mut tx = repo.start_transaction(&settings, "test");
-    CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![store.root_commit_id().clone()],
-        store.empty_tree_id().clone(),
-    )
-    .write()
-    .unwrap();
+    tx.mut_repo()
+        .new_commit(
+            &settings,
+            vec![store.root_commit_id().clone()],
+            store.empty_tree_id().clone(),
+        )
+        .write()
+        .unwrap();
     let mut rebaser = tx.mut_repo().create_descendant_rebaser(&settings);
     assert!(rebaser.rebase_next().unwrap().is_none());
 

lib/tests/test_index.rs

@@ -23,7 +23,6 @@ use jujutsu_lib::settings::UserSettings;
 use test_case::test_case;
 use testutils::{create_random_commit, write_random_commit, CommitGraphBuilder, TestRepo};
 
-#[must_use]
 fn child_commit<'repo>(
     mut_repo: &'repo mut MutableRepo,
     settings: &UserSettings,

lib/tests/test_merge_trees.rs

@@ -573,41 +573,33 @@ fn test_simplify_conflict_after_resolving_parent(use_git: bool) {
     let path = RepoPath::from_internal_string("dir/file");
     let mut tx = repo.start_transaction(&settings, "test");
     let tree_a = testutils::create_tree(repo, &[(&path, "abc\ndef\nghi\n")]);
-    let commit_a = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![repo.store().root_commit_id().clone()],
-        tree_a.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_a = tx
+        .mut_repo()
+        .new_commit(
+            &settings,
+            vec![repo.store().root_commit_id().clone()],
+            tree_a.id().clone(),
+        )
+        .write()
+        .unwrap();
     let tree_b = testutils::create_tree(repo, &[(&path, "Abc\ndef\nghi\n")]);
-    let commit_b = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![commit_a.id().clone()],
-        tree_b.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_b = tx
+        .mut_repo()
+        .new_commit(&settings, vec![commit_a.id().clone()], tree_b.id().clone())
+        .write()
+        .unwrap();
     let tree_c = testutils::create_tree(repo, &[(&path, "Abc\ndef\nGhi\n")]);
-    let commit_c = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![commit_b.id().clone()],
-        tree_c.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_c = tx
+        .mut_repo()
+        .new_commit(&settings, vec![commit_b.id().clone()], tree_c.id().clone())
+        .write()
+        .unwrap();
     let tree_d = testutils::create_tree(repo, &[(&path, "abC\ndef\nghi\n")]);
-    let commit_d = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![commit_a.id().clone()],
-        tree_d.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_d = tx
+        .mut_repo()
+        .new_commit(&settings, vec![commit_a.id().clone()], tree_d.id().clone())
+        .write()
+        .unwrap();
 
     let commit_b2 = rebase_commit(&settings, tx.mut_repo(), &commit_b, &[commit_d]).unwrap();
     let commit_c2 =

lib/tests/test_mut_repo.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use jujutsu_lib::commit_builder::CommitBuilder;
 use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
 use maplit::hashset;
 use test_case::test_case;
@@ -102,14 +101,14 @@ fn test_checkout_previous_empty(use_git: bool) {
 
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
-    let old_checkout = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![repo.store().root_commit_id().clone()],
-        repo.store().empty_tree_id().clone(),
-    )
-    .write()
-    .unwrap();
+    let old_checkout = mut_repo
+        .new_commit(
+            &settings,
+            vec![repo.store().root_commit_id().clone()],
+            repo.store().empty_tree_id().clone(),
+        )
+        .write()
+        .unwrap();
     let ws_id = WorkspaceId::default();
     mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
     let repo = tx.commit();
@@ -133,15 +132,15 @@ fn test_checkout_previous_empty_with_description(use_git: bool) {
 
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
-    let old_checkout = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![repo.store().root_commit_id().clone()],
-        repo.store().empty_tree_id().clone(),
-    )
-    .set_description("not empty")
-    .write()
-    .unwrap();
+    let old_checkout = mut_repo
+        .new_commit(
+            &settings,
+            vec![repo.store().root_commit_id().clone()],
+            repo.store().empty_tree_id().clone(),
+        )
+        .set_description("not empty")
+        .write()
+        .unwrap();
     let ws_id = WorkspaceId::default();
     mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
     let repo = tx.commit();
@@ -165,22 +164,22 @@ fn test_checkout_previous_empty_non_head(use_git: bool) {
 
     let mut tx = repo.start_transaction(&settings, "test");
     let mut_repo = tx.mut_repo();
-    let old_checkout = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![repo.store().root_commit_id().clone()],
-        repo.store().empty_tree_id().clone(),
-    )
-    .write()
-    .unwrap();
-    let old_child = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![old_checkout.id().clone()],
-        old_checkout.tree_id().clone(),
-    )
-    .write()
-    .unwrap();
+    let old_checkout = mut_repo
+        .new_commit(
+            &settings,
+            vec![repo.store().root_commit_id().clone()],
+            repo.store().empty_tree_id().clone(),
+        )
+        .write()
+        .unwrap();
+    let old_child = mut_repo
+        .new_commit(
+            &settings,
+            vec![old_checkout.id().clone()],
+            old_checkout.tree_id().clone(),
+        )
+        .write()
+        .unwrap();
     let ws_id = WorkspaceId::default();
     mut_repo.edit(ws_id.clone(), &old_checkout).unwrap();
     let repo = tx.commit();

lib/tests/test_revset.rs

@@ -15,7 +15,6 @@
 use std::path::Path;
 
 use jujutsu_lib::backend::{CommitId, MillisSinceEpoch, Signature, Timestamp};
-use jujutsu_lib::commit_builder::CommitBuilder;
 use jujutsu_lib::git;
 use jujutsu_lib::matchers::{FilesMatcher, Matcher};
 use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
@@ -63,17 +62,17 @@ fn test_resolve_symbol_commit_id() {
 
     let mut commits = vec![];
     for i in &[1, 167, 895] {
-        let commit = CommitBuilder::for_new_commit(
-            mut_repo,
-            &settings,
-            vec![repo.store().root_commit_id().clone()],
-            repo.store().empty_tree_id().clone(),
-        )
-        .set_description(format!("test {i}"))
-        .set_author(signature.clone())
-        .set_committer(signature.clone())
-        .write()
-        .unwrap();
+        let commit = mut_repo
+            .new_commit(
+                &settings,
+                vec![repo.store().root_commit_id().clone()],
+                repo.store().empty_tree_id().clone(),
+            )
+            .set_description(format!("test {i}"))
+            .set_author(signature.clone())
+            .set_committer(signature.clone())
+            .write()
+            .unwrap();
         commits.push(commit);
     }
     let repo = tx.commit();
@@ -1927,38 +1926,26 @@ fn test_filter_by_diff(use_git: bool) {
             // added_modified_removed,
         ],
     );
-    let commit1 = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![repo.store().root_commit_id().clone()],
-        tree1.id().clone(),
-    )
-    .write()
-    .unwrap();
-    let commit2 = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![commit1.id().clone()],
-        tree2.id().clone(),
-    )
-    .write()
-    .unwrap();
-    let commit3 = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![commit2.id().clone()],
-        tree3.id().clone(),
-    )
-    .write()
-    .unwrap();
-    let commit4 = CommitBuilder::for_new_commit(
-        mut_repo,
-        &settings,
-        vec![commit3.id().clone()],
-        tree3.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit1 = mut_repo
+        .new_commit(
+            &settings,
+            vec![repo.store().root_commit_id().clone()],
+            tree1.id().clone(),
+        )
+        .write()
+        .unwrap();
+    let commit2 = mut_repo
+        .new_commit(&settings, vec![commit1.id().clone()], tree2.id().clone())
+        .write()
+        .unwrap();
+    let commit3 = mut_repo
+        .new_commit(&settings, vec![commit2.id().clone()], tree3.id().clone())
+        .write()
+        .unwrap();
+    let commit4 = mut_repo
+        .new_commit(&settings, vec![commit3.id().clone()], tree3.id().clone())
+        .write()
+        .unwrap();
 
     // matcher API:
     let resolve = |file_path: &RepoPath| -> Vec<CommitId> {

lib/tests/test_rewrite.rs

@@ -852,44 +852,36 @@ fn test_rebase_descendants_contents(use_git: bool) {
     let mut tx = repo.start_transaction(&settings, "test");
     let path1 = RepoPath::from_internal_string("file1");
     let tree1 = testutils::create_tree(repo, &[(&path1, "content")]);
-    let commit_a = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![repo.store().root_commit_id().clone()],
-        tree1.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_a = tx
+        .mut_repo()
+        .new_commit(
+            &settings,
+            vec![repo.store().root_commit_id().clone()],
+            tree1.id().clone(),
+        )
+        .write()
+        .unwrap();
     let path2 = RepoPath::from_internal_string("file2");
     let tree2 = testutils::create_tree(repo, &[(&path2, "content")]);
-    let commit_b = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![commit_a.id().clone()],
-        tree2.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_b = tx
+        .mut_repo()
+        .new_commit(&settings, vec![commit_a.id().clone()], tree2.id().clone())
+        .write()
+        .unwrap();
     let path3 = RepoPath::from_internal_string("file3");
     let tree3 = testutils::create_tree(repo, &[(&path3, "content")]);
-    let commit_c = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![commit_b.id().clone()],
-        tree3.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_c = tx
+        .mut_repo()
+        .new_commit(&settings, vec![commit_b.id().clone()], tree3.id().clone())
+        .write()
+        .unwrap();
     let path4 = RepoPath::from_internal_string("file4");
     let tree4 = testutils::create_tree(repo, &[(&path4, "content")]);
-    let commit_d = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        &settings,
-        vec![commit_a.id().clone()],
-        tree4.id().clone(),
-    )
-    .write()
-    .unwrap();
+    let commit_d = tx
+        .mut_repo()
+        .new_commit(&settings, vec![commit_a.id().clone()], tree4.id().clone())
+        .write()
+        .unwrap();
 
     let mut rebaser = DescendantRebaser::new(
         &settings,

lib/testutils/src/lib.rs

@@ -208,20 +208,19 @@ pub fn create_random_tree(repo: &ReadonlyRepo) -> TreeId {
     tree_builder.write_tree()
 }
 
-#[must_use]
 pub fn create_random_commit<'repo>(
     mut_repo: &'repo mut MutableRepo,
     settings: &UserSettings,
 ) -> CommitBuilder<'repo> {
     let tree_id = create_random_tree(mut_repo.base_repo());
     let number = rand::random::<u32>();
-    CommitBuilder::for_new_commit(
-        mut_repo,
-        settings,
-        vec![mut_repo.store().root_commit_id().clone()],
-        tree_id,
-    )
-    .set_description(format!("random commit {number}"))
+    mut_repo
+        .new_commit(
+            settings,
+            vec![mut_repo.store().root_commit_id().clone()],
+            tree_id,
+        )
+        .set_description(format!("random commit {number}"))
 }
 
 pub fn write_random_commit(mut_repo: &mut MutableRepo, settings: &UserSettings) -> Commit {

src/commands.rs

@@ -1182,13 +1182,14 @@ fn cmd_checkout(
     let workspace_id = workspace_command.workspace_id();
     let mut tx =
         workspace_command.start_transaction(&format!("check out commit {}", target.id().hex()));
-    let commit_builder = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        ui.settings(),
-        vec![target.id().clone()],
-        target.tree_id().clone(),
-    )
-    .set_description(&args.message);
+    let commit_builder = tx
+        .mut_repo()
+        .new_commit(
+            ui.settings(),
+            vec![target.id().clone()],
+            target.tree_id().clone(),
+        )
+        .set_description(&args.message);
     let new_commit = commit_builder.write()?;
     tx.mut_repo().edit(workspace_id, &new_commit).unwrap();
     workspace_command.finish_transaction(ui, tx)?;
@@ -1920,13 +1921,14 @@ fn cmd_commit(ui: &mut Ui, command: &CommandHelper, args: &CommitArgs) -> Result
         .view()
         .workspaces_for_wc_commit_id(commit.id());
     if !workspace_ids.is_empty() {
-        let new_checkout = CommitBuilder::for_new_commit(
-            tx.mut_repo(),
-            ui.settings(),
-            vec![new_commit.id().clone()],
-            new_commit.tree_id().clone(),
-        )
-        .write()?;
+        let new_checkout = tx
+            .mut_repo()
+            .new_commit(
+                ui.settings(),
+                vec![new_commit.id().clone()],
+                new_commit.tree_id().clone(),
+            )
+            .write()?;
         for workspace_id in workspace_ids {
             tx.mut_repo().edit(workspace_id, &new_checkout).unwrap();
         }
@@ -2033,14 +2035,11 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
     let parent_ids = commits.iter().map(|c| c.id().clone()).collect();
     let mut tx = workspace_command.start_transaction("new empty commit");
     let merged_tree = merge_commit_trees(workspace_command.repo().as_repo_ref(), &commits);
-    let new_commit = CommitBuilder::for_new_commit(
-        tx.mut_repo(),
-        ui.settings(),
-        parent_ids,
-        merged_tree.id().clone(),
-    )
-    .set_description(&args.message)
-    .write()?;
+    let new_commit = tx
+        .mut_repo()
+        .new_commit(ui.settings(), parent_ids, merged_tree.id().clone())
+        .set_description(&args.message)
+        .write()?;
     let workspace_id = workspace_command.workspace_id();
     tx.mut_repo().edit(workspace_id, &new_commit).unwrap();
     workspace_command.finish_transaction(ui, tx)?;