From 20e9d29c4bf7264c0184f0dc4393f059ba6ea62b Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 11 Sep 2021 09:45:06 -0700 Subject: [PATCH] CommitBuilder: remove `write_to_new_transaction()`, which was only used in tests --- lib/src/commit_builder.rs | 9 +-------- lib/tests/test_bad_locking.rs | 17 ++++++++++++----- lib/tests/test_commit_builder.rs | 13 +++++++++---- lib/tests/test_commit_concurrent.rs | 10 ++++++---- lib/tests/test_git.rs | 19 +++++++++++-------- lib/tests/test_index.rs | 18 +++++++++--------- lib/tests/test_operations.rs | 5 +++-- lib/tests/test_working_copy.rs | 10 +++++++--- lib/tests/test_working_copy_concurrent.rs | 12 +++++++----- 9 files changed, 65 insertions(+), 48 deletions(-) diff --git a/lib/src/commit_builder.rs b/lib/src/commit_builder.rs index 7fcf0c935..2971c79df 100644 --- a/lib/src/commit_builder.rs +++ b/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, 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()) { diff --git a/lib/tests/test_bad_locking.rs b/lib/tests/test_bad_locking.rs index aa5821ed9..ae8c7c04e 100644 --- a/lib/tests/test_bad_locking.rs +++ b/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"); - let repo = repo.reload(); + .write_to_repo(tx.mut_repo()); + 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 diff --git a/lib/tests/test_commit_builder.rs b/lib/tests/test_commit_builder.rs index 53b48cf96..266daa7ee 100644 --- a/lib/tests/test_commit_builder.rs +++ b/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"); - let repo = repo.reload(); + .write_to_repo(tx.mut_repo()); + 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(), diff --git a/lib/tests/test_commit_concurrent.rs b/lib/tests/test_commit_concurrent.rs index 0209a3e18..e7e225033 100644 --- a/lib/tests/test_commit_concurrent.rs +++ b/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) - .write_to_new_transaction(&repo, "test"); + let mut tx = repo.start_transaction("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) - .write_to_new_transaction(&repo, "test"); + let mut tx = repo.start_transaction("test"); + testutils::create_random_commit(&settings, &repo).write_to_repo(tx.mut_repo()); + tx.commit(); }); threads.push(handle); } diff --git a/lib/tests/test_git.rs b/lib/tests/test_git.rs index 86aa42a03..b52a21358 100644 --- a/lib/tests/test_git.rs +++ b/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"); - let jj_repo = jj_repo.reload(); + .write_to_repo(tx.mut_repo()); + 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) - .write_to_new_transaction(&setup.jj_repo, "test"); - setup.jj_repo = setup.jj_repo.reload(); + let mut tx = setup.jj_repo.start_transaction("test"); + let new_commit = + 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) - .write_to_new_transaction(&setup.jj_repo, "test"); - setup.jj_repo = setup.jj_repo.reload(); + let mut tx = setup.jj_repo.start_transaction("test"); + let new_commit = + 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, diff --git a/lib/tests/test_index.rs b/lib/tests/test_index.rs index ce0cab3e6..1712c70a2 100644 --- a/lib/tests/test_index.rs +++ b/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 = - child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test"); - repo = repo.reload(); + let mut tx = repo.start_transaction("test"); + let commit_a = child_commit(&settings, &repo, &root_commit).write_to_repo(tx.mut_repo()); + 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 = - child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test"); - repo = repo.reload(); + let mut tx = repo.start_transaction("test"); + let commit_a = child_commit(&settings, &repo, &root_commit).write_to_repo(tx.mut_repo()); + 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 = - child_commit(&settings, &repo, &root_commit).write_to_new_transaction(&repo, "test"); - repo = repo.reload(); + let mut tx = repo.start_transaction("test"); + let commit_a = child_commit(&settings, &repo, &root_commit).write_to_repo(tx.mut_repo()); + repo = tx.commit(); assert!(repo.index().has_id(commit_a.id())); assert_eq!(repo.index().num_commits(), 2 + 1); diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index 7acceb891..9d9ea38b0 100644 --- a/lib/tests/test_operations.rs +++ b/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"); - let repo = repo.reload(); + .write_to_repo(tx.mut_repo()); + let repo = tx.commit(); let mut tx1 = repo.start_transaction("transaction 1"); let mut_repo1 = tx1.mut_repo(); diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index d8f99de79..08d651175 100644 --- a/lib/tests/test_working_copy.rs +++ b/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(); diff --git a/lib/tests/test_working_copy_concurrent.rs b/lib/tests/test_working_copy_concurrent.rs index 3c68d3175..b19c2aa36 100644 --- a/lib/tests/test_working_copy_concurrent.rs +++ b/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());