CommitBuilder: remove write_to_new_transaction(), which was only used in tests

lib/src/commit_builder.rs

@@ -17,7 +17,7 @@ use std::sync::Arc;
 use uuid::Uuid;
 
 use crate::commit::Commit;
-use crate::repo::{MutableRepo, ReadonlyRepo};
+use crate::repo::MutableRepo;
 use crate::settings::UserSettings;
 use crate::store;
 use crate::store::{ChangeId, CommitId, Signature, Timestamp, TreeId};
@@ -154,13 +154,6 @@ impl CommitBuilder {
         self
     }
 
-    pub fn write_to_new_transaction(self, repo: &Arc<ReadonlyRepo>, description: &str) -> Commit {
-        let mut tx = repo.start_transaction(description);
-        let commit = self.write_to_repo(tx.mut_repo());
-        tx.commit();
-        commit
-    }
-
     pub fn write_to_repo(mut self, repo: &mut MutableRepo) -> Commit {
         let parents = &mut self.commit.parents;
         if parents.contains(self.store.root_commit_id()) {

lib/tests/test_bad_locking.rs

@@ -96,25 +96,31 @@ fn test_bad_locking_children(use_git: bool) {
     let settings = testutils::user_settings();
     let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
 
+    let mut tx = repo.start_transaction("test");
     let initial = testutils::create_random_commit(&settings, &repo)
         .set_parents(vec![repo.store().root_commit_id().clone()])
-        .write_to_new_transaction(&repo, "test");
+        .write_to_repo(tx.mut_repo());
+    tx.commit();
 
     // Simulate a write of a commit that happens on one machine
     let machine1_path = TempDir::new().unwrap().into_path();
     copy_directory(repo.working_copy_path(), &machine1_path);
     let machine1_repo = ReadonlyRepo::load(&settings, machine1_path).unwrap();
+    let mut machine1_tx = machine1_repo.start_transaction("test");
     let child1 = testutils::create_random_commit(&settings, &machine1_repo)
         .set_parents(vec![initial.id().clone()])
-        .write_to_new_transaction(&machine1_repo, "test");
+        .write_to_repo(machine1_tx.mut_repo());
+    machine1_tx.commit();
 
     // Simulate a write of a commit that happens on another machine
     let machine2_path = TempDir::new().unwrap().into_path();
     copy_directory(repo.working_copy_path(), &machine2_path);
     let machine2_repo = ReadonlyRepo::load(&settings, machine2_path).unwrap();
+    let mut machine2_tx = machine2_repo.start_transaction("test");
     let child2 = testutils::create_random_commit(&settings, &machine2_repo)
         .set_parents(vec![initial.id().clone()])
-        .write_to_new_transaction(&machine2_repo, "test");
+        .write_to_repo(machine2_tx.mut_repo());
+    machine2_tx.commit();
 
     // Simulate that the distributed file system now has received the changes from
     // both machines
@@ -142,10 +148,11 @@ fn test_bad_locking_interrupted(use_git: bool) {
     let settings = testutils::user_settings();
     let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
 
+    let mut tx = repo.start_transaction("test");
     let initial = testutils::create_random_commit(&settings, &repo)
         .set_parents(vec![repo.store().root_commit_id().clone()])
-        .write_to_new_transaction(&repo, "test");
+        .write_to_repo(tx.mut_repo());
-    let repo = repo.reload();
+    let repo = tx.commit();
 
     // Simulate a crash that resulted in the old op-head left in place. We simulate
     // it somewhat hackily by copying the .jj/op_heads/ directory before the

lib/tests/test_commit_builder.rs

@@ -37,9 +37,11 @@ fn test_initial(use_git: bool) {
         ],
     );
 
+    let mut tx = repo.start_transaction("test");
     let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone())
         .set_parents(vec![store.root_commit_id().clone()])
-        .write_to_new_transaction(&repo, "test");
+        .write_to_repo(tx.mut_repo());
+    tx.commit();
 
     assert_eq!(commit.parents(), vec![store.root_commit()]);
     assert_eq!(commit.predecessors(), vec![]);
@@ -79,11 +81,12 @@ fn test_rewrite(use_git: bool) {
         ],
     );
 
+    let mut tx = repo.start_transaction("test");
     let initial_commit =
         CommitBuilder::for_new_commit(&settings, &store, initial_tree.id().clone())
             .set_parents(vec![store.root_commit_id().clone()])
-            .write_to_new_transaction(&repo, "test");
+            .write_to_repo(tx.mut_repo());
-    let repo = repo.reload();
+    let repo = tx.commit();
 
     let rewritten_tree = testutils::create_tree(
         &repo,
@@ -99,10 +102,12 @@ fn test_rewrite(use_git: bool) {
         .set("user.email", "rewrite.user@example.com")
         .unwrap();
     let rewrite_settings = UserSettings::from_config(config);
+    let mut tx = repo.start_transaction("test");
     let rewritten_commit =
         CommitBuilder::for_rewrite_from(&rewrite_settings, &store, &initial_commit)
             .set_tree(rewritten_tree.id().clone())
-            .write_to_new_transaction(&repo, "test");
+            .write_to_repo(tx.mut_repo());
+    tx.commit();
     assert_eq!(rewritten_commit.parents(), vec![store.root_commit()]);
     assert_eq!(
         rewritten_commit.predecessors(),

lib/tests/test_commit_concurrent.rs

@@ -51,8 +51,9 @@ fn test_commit_parallel(use_git: bool) {
         let settings = settings.clone();
         let repo = repo.clone();
         let handle = thread::spawn(move || {
-            testutils::create_random_commit(&settings, &repo)
+            let mut tx = repo.start_transaction("test");
-                .write_to_new_transaction(&repo, "test");
+            testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
+            tx.commit();
         });
         threads.push(handle);
     }
@@ -83,8 +84,9 @@ fn test_commit_parallel_instances(use_git: bool) {
         let settings = settings.clone();
         let repo = ReadonlyRepo::load(&settings, repo.working_copy_path().clone()).unwrap();
         let handle = thread::spawn(move || {
-            testutils::create_random_commit(&settings, &repo)
+            let mut tx = repo.start_transaction("test");
-                .write_to_new_transaction(&repo, "test");
+            testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo());
+            tx.commit();
         });
         threads.push(handle);
     }

lib/tests/test_git.rs

@@ -336,10 +336,11 @@ fn set_up_push_repos(settings: &UserSettings, temp_dir: &TempDir) -> PushTestSet
     git2::Repository::clone(source_repo_dir.to_str().unwrap(), &clone_repo_dir).unwrap();
     std::fs::create_dir(&jj_repo_dir).unwrap();
     let jj_repo = ReadonlyRepo::init_external_git(settings, jj_repo_dir, clone_repo_dir).unwrap();
+    let mut tx = jj_repo.start_transaction("test");
     let new_commit = testutils::create_random_commit(settings, &jj_repo)
         .set_parents(vec![initial_commit_id])
-        .write_to_new_transaction(&jj_repo, "test");
+        .write_to_repo(tx.mut_repo());
-    let jj_repo = jj_repo.reload();
+    let jj_repo = tx.commit();
     PushTestSetup {
         source_repo_dir,
         jj_repo,
@@ -380,9 +381,10 @@ fn test_push_commit_not_fast_forward() {
     let settings = testutils::user_settings();
     let temp_dir = tempfile::tempdir().unwrap();
     let mut setup = set_up_push_repos(&settings, &temp_dir);
-    let new_commit = testutils::create_random_commit(&settings, &setup.jj_repo)
+    let mut tx = setup.jj_repo.start_transaction("test");
-        .write_to_new_transaction(&setup.jj_repo, "test");
+    let new_commit =
-    setup.jj_repo = setup.jj_repo.reload();
+        testutils::create_random_commit(&settings, &setup.jj_repo).write_to_repo(tx.mut_repo());
+    setup.jj_repo = tx.commit();
     let result = git::push_commit(
         &setup.jj_repo.store().git_repo().unwrap(),
         &new_commit,
@@ -398,9 +400,10 @@ fn test_push_commit_not_fast_forward_with_force() {
     let settings = testutils::user_settings();
     let temp_dir = tempfile::tempdir().unwrap();
     let mut setup = set_up_push_repos(&settings, &temp_dir);
-    let new_commit = testutils::create_random_commit(&settings, &setup.jj_repo)
+    let mut tx = setup.jj_repo.start_transaction("test");
-        .write_to_new_transaction(&setup.jj_repo, "test");
+    let new_commit =
-    setup.jj_repo = setup.jj_repo.reload();
+        testutils::create_random_commit(&settings, &setup.jj_repo).write_to_repo(tx.mut_repo());
+    setup.jj_repo = tx.commit();
     let result = git::push_commit(
         &setup.jj_repo.store().git_repo().unwrap(),
         &new_commit,

lib/tests/test_index.rs

@@ -300,9 +300,9 @@ fn test_index_commits_incremental(use_git: bool) {
     // o root
 
     let root_commit = repo.store().root_commit();
-    let commit_a =
+    let mut tx = repo.start_transaction("test");
-        child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test");
+    let commit_a = child_commit(&settings, &repo, &root_commit).write_to_repo(tx.mut_repo());
-    repo = repo.reload();
+    repo = tx.commit();
 
     let index = repo.index();
     // There should be the root commit and the working copy commit, plus
@@ -347,9 +347,9 @@ fn test_index_commits_incremental_empty_transaction(use_git: bool) {
     // o root
 
     let root_commit = repo.store().root_commit();
-    let commit_a =
+    let mut tx = repo.start_transaction("test");
-        child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test");
+    let commit_a = child_commit(&settings, &repo, &root_commit).write_to_repo(tx.mut_repo());
-    repo = repo.reload();
+    repo = tx.commit();
 
     let index = repo.index();
     // There should be the root commit and the working copy commit, plus
@@ -391,9 +391,9 @@ fn test_index_commits_incremental_already_indexed(use_git: bool) {
     // o root
 
     let root_commit = repo.store().root_commit();
-    let commit_a =
+    let mut tx = repo.start_transaction("test");
-        child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test");
+    let commit_a = child_commit(&settings, &repo, &root_commit).write_to_repo(tx.mut_repo());
-    repo = repo.reload();
+    repo = tx.commit();
 
     assert!(repo.index().has_id(commit_a.id()));
     assert_eq!(repo.index().num_commits(), 2 + 1);

lib/tests/test_operations.rs

@@ -133,10 +133,11 @@ fn test_isolation(use_git: bool) {
     let (_temp_dir, repo) = testutils::init_repo(&settings, use_git);
 
     let wc_id = repo.working_copy_locked().current_commit_id();
+    let mut tx = repo.start_transaction("test");
     let initial = testutils::create_random_commit(&settings, &repo)
         .set_parents(vec![repo.store().root_commit_id().clone()])
-        .write_to_new_transaction(&repo, "test");
+        .write_to_repo(tx.mut_repo());
-    let repo = repo.reload();
+    let repo = tx.commit();
 
     let mut tx1 = repo.start_transaction("transaction 1");
     let mut_repo1 = tx1.mut_repo();

lib/tests/test_working_copy.rs

@@ -121,10 +121,12 @@ fn test_checkout_file_transitions(use_git: bool) {
                 TreeValue::Tree(id)
             }
             Kind::GitSubmodule => {
+                let mut tx = repo.start_transaction("test");
                 let id = testutils::create_random_commit(settings, repo)
-                    .write_to_new_transaction(repo, "test")
+                    .write_to_repo(tx.mut_repo())
                     .id()
                     .clone();
+                tx.commit();
                 TreeValue::GitSubmodule(id)
             }
         };
@@ -157,14 +159,16 @@ fn test_checkout_file_transitions(use_git: bool) {
     let left_tree_id = left_tree_builder.write_tree();
     let right_tree_id = right_tree_builder.write_tree();
 
+    let mut tx = repo.start_transaction("test");
     let left_commit = CommitBuilder::for_new_commit(&settings, repo.store(), left_tree_id)
         .set_parents(vec![store.root_commit_id().clone()])
         .set_open(true)
-        .write_to_new_transaction(&repo, "test");
+        .write_to_repo(tx.mut_repo());
     let right_commit = CommitBuilder::for_new_commit(&settings, repo.store(), right_tree_id)
         .set_parents(vec![store.root_commit_id().clone()])
         .set_open(true)
-        .write_to_new_transaction(&repo, "test");
+        .write_to_repo(tx.mut_repo());
+    tx.commit();
 
     let owned_wc = repo.working_copy().clone();
     let wc = owned_wc.lock().unwrap();

lib/tests/test_working_copy_concurrent.rs

@@ -31,15 +31,17 @@ fn test_concurrent_checkout(use_git: bool) {
     let settings = testutils::user_settings();
     let (_temp_dir, repo1) = testutils::init_repo(&settings, use_git);
 
+    let mut tx1 = repo1.start_transaction("test");
     let commit1 = testutils::create_random_commit(&settings, &repo1)
         .set_open(true)
-        .write_to_new_transaction(&repo1, "test");
+        .write_to_repo(tx1.mut_repo());
     let commit2 = testutils::create_random_commit(&settings, &repo1)
         .set_open(true)
-        .write_to_new_transaction(&repo1, "test");
+        .write_to_repo(tx1.mut_repo());
     let commit3 = testutils::create_random_commit(&settings, &repo1)
         .set_open(true)
-        .write_to_new_transaction(&repo1, "test");
+        .write_to_repo(tx1.mut_repo());
+    tx1.commit();
 
     // Check out commit1
     let wc1 = repo1.working_copy_locked();
@@ -78,13 +80,14 @@ fn test_checkout_parallel(use_git: bool) {
     let num_threads = max(num_cpus::get(), 4);
     let mut tree_ids = HashSet::new();
     let mut commit_ids = vec![];
+    let mut tx = repo.start_transaction("test");
     for i in 0..num_threads {
         let path = RepoPath::from_internal_string(format!("file{}", i).as_str());
         let tree = testutils::create_tree(&repo, &[(&path, "contents")]);
         tree_ids.insert(tree.id().clone());
         let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone())
             .set_open(true)
-            .write_to_new_transaction(&repo, "test");
+            .write_to_repo(tx.mut_repo());
         commit_ids.push(commit.id().clone());
     }
 
@@ -94,7 +97,6 @@ fn test_checkout_parallel(use_git: bool) {
         &repo,
         &[(&RepoPath::from_internal_string("other file"), "contents")],
     );
-    let mut tx = repo.start_transaction("test");
     let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone())
         .set_open(true)
         .write_to_repo(tx.mut_repo());