diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index df087bd1c..862631418 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -34,7 +34,6 @@ use thiserror::Error; use crate::backend::{ BackendError, ConflictId, FileId, MillisSinceEpoch, SymlinkId, TreeId, TreeValue, }; -use crate::commit::Commit; use crate::conflicts::{materialize_conflict, update_conflict_from_content}; use crate::gitignore::GitIgnoreFile; use crate::lock::FileLock; @@ -853,7 +852,7 @@ impl WorkingCopy { &mut self, operation_id: OperationId, old_tree_id: Option<&TreeId>, - new_commit: Commit, + new_tree: &Tree, ) -> Result { let mut locked_wc = self.start_mutation(); // Check if the current checkout has changed on disk compared to what the caller @@ -865,7 +864,7 @@ impl WorkingCopy { return Err(CheckoutError::ConcurrentCheckout); } } - let stats = locked_wc.check_out(&new_commit.tree())?; + let stats = locked_wc.check_out(new_tree)?; locked_wc.finish(operation_id); Ok(stats) } diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 79cbffc5f..7a341a22d 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -20,7 +20,6 @@ use std::sync::Arc; use itertools::Itertools; use jujutsu_lib::backend::{Conflict, ConflictPart, TreeValue}; -use jujutsu_lib::commit_builder::CommitBuilder; use jujutsu_lib::op_store::WorkspaceId; use jujutsu_lib::repo::ReadonlyRepo; use jujutsu_lib::repo_path::{RepoPath, RepoPathComponent}; @@ -205,29 +204,20 @@ 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_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_repo(tx.mut_repo()); - let repo = tx.commit(); + let left_tree = store.get_tree(&RepoPath::root(), &left_tree_id).unwrap(); + let right_tree = store.get_tree(&RepoPath::root(), &right_tree_id).unwrap(); let wc = test_workspace.workspace.working_copy_mut(); - wc.check_out(repo.op_id().clone(), None, left_commit) + wc.check_out(repo.op_id().clone(), None, &left_tree) .unwrap(); - wc.check_out(repo.op_id().clone(), None, right_commit.clone()) + wc.check_out(repo.op_id().clone(), None, &right_tree) .unwrap(); // Check that the working copy is clean. let mut locked_wc = wc.start_mutation(); let new_tree_id = locked_wc.write_tree(); locked_wc.discard(); - assert_eq!(&new_tree_id, right_commit.tree().id()); + assert_eq!(new_tree_id, right_tree_id); for (_left_kind, right_kind, path) in &files { let wc_path = workspace_root.join(path); @@ -309,21 +299,9 @@ fn test_reset() { repo, &[(&gitignore_path, "ignored\n"), (&ignored_path, "code")], ); - let mut tx = repo.start_transaction("test"); - let store = repo.store(); - let root_commit = store.root_commit_id(); - let commit_with_file = CommitBuilder::for_open_commit( - &settings, - store, - root_commit.clone(), - tree_with_file.id().clone(), - ) - .write_to_repo(tx.mut_repo()); - let repo = tx.commit(); - test_workspace.repo = repo.clone(); let wc = test_workspace.workspace.working_copy_mut(); - wc.check_out(repo.op_id().clone(), None, commit_with_file) + wc.check_out(repo.op_id().clone(), None, &tree_with_file) .unwrap(); // Test the setup: the file should exist on disk and in the tree state. @@ -382,18 +360,13 @@ fn test_checkout_discard() { let file1_path = RepoPath::from_internal_string("file1"); let file2_path = RepoPath::from_internal_string("file2"); - let mut tx = repo.start_transaction("test"); let store = repo.store(); let tree1 = testutils::create_tree(&repo, &[(&file1_path, "contents")]); let tree2 = testutils::create_tree(&repo, &[(&file2_path, "contents")]); - let commit1 = CommitBuilder::for_new_commit(&settings, store, tree1.id().clone()) - .write_to_repo(tx.mut_repo()); - let repo = tx.commit(); - test_workspace.repo = repo.clone(); let wc = test_workspace.workspace.working_copy_mut(); let state_path = wc.state_path().to_path_buf(); - wc.check_out(repo.op_id().clone(), None, commit1).unwrap(); + wc.check_out(repo.op_id().clone(), None, &tree1).unwrap(); // Test the setup: the file should exist on disk and in the tree state. assert!(file1_path.to_fs_path(&workspace_root).is_file()); @@ -545,24 +518,19 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) { let modified_path = RepoPath::from_internal_string("modified"); testutils::write_working_copy_file(&workspace_root, &modified_path, "garbage"); - // Create a commit that adds the same file but with different contents - let mut tx = repo.start_transaction("test"); + // Create a tree that adds the same file but with different contents let mut tree_builder = repo .store() .tree_builder(repo.store().empty_tree_id().clone()); testutils::write_normal_file(&mut tree_builder, &modified_path, "contents"); let tree_id = tree_builder.write_tree(); - let commit = CommitBuilder::for_new_commit(&settings, repo.store(), tree_id) - .set_open(true) - .set_description("add file".to_string()) - .write_to_repo(tx.mut_repo()); - let repo = tx.commit(); + let tree = repo.store().get_tree(&RepoPath::root(), &tree_id).unwrap(); - // Now check out the commit that adds the file "modified" with contents + // Now check out the tree that adds the file "modified" with contents // "contents". The exiting contents ("garbage") should be replaced in the // working copy. let wc = test_workspace.workspace.working_copy_mut(); - wc.check_out(repo.op_id().clone(), None, commit).unwrap(); + wc.check_out(repo.op_id().clone(), None, &tree).unwrap(); // Check that the new contents are in the working copy let path = workspace_root.join("modified"); @@ -605,22 +573,17 @@ fn test_gitignores_ignored_directory_already_tracked(use_git: bool) { ); let file_path = RepoPath::from_internal_string("ignored/file"); - // Create a commit that adds a file in the ignored directory - let mut tx = repo.start_transaction("test"); + // Create a tree that adds a file in the ignored directory let mut tree_builder = repo .store() .tree_builder(repo.store().empty_tree_id().clone()); testutils::write_normal_file(&mut tree_builder, &file_path, "contents"); let tree_id = tree_builder.write_tree(); - let commit = CommitBuilder::for_new_commit(&settings, repo.store(), tree_id) - .set_open(true) - .set_description("add ignored file".to_string()) - .write_to_repo(tx.mut_repo()); - let repo = tx.commit(); + let tree = repo.store().get_tree(&RepoPath::root(), &tree_id).unwrap(); - // Check out the commit with the file in ignored/ + // Check out the tree with the file in ignored/ let wc = test_workspace.workspace.working_copy_mut(); - wc.check_out(repo.op_id().clone(), None, commit).unwrap(); + wc.check_out(repo.op_id().clone(), None, &tree).unwrap(); // Check that the file is still in the tree created by committing the working // copy (that it didn't get removed because the directory is ignored) diff --git a/lib/tests/test_working_copy_concurrent.rs b/lib/tests/test_working_copy_concurrent.rs index 7b2d2fd3b..7a38ffb53 100644 --- a/lib/tests/test_working_copy_concurrent.rs +++ b/lib/tests/test_working_copy_concurrent.rs @@ -13,10 +13,8 @@ // limitations under the License. use std::cmp::max; -use std::collections::HashSet; use std::thread; -use jujutsu_lib::commit_builder::CommitBuilder; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::testutils; use jujutsu_lib::working_copy::CheckoutError; @@ -33,44 +31,44 @@ fn test_concurrent_checkout(use_git: bool) { let repo1 = test_workspace1.repo.clone(); let workspace1_root = test_workspace1.workspace.workspace_root().clone(); - let mut tx1 = repo1.start_transaction("test"); - let commit1 = testutils::create_random_commit(&settings, &repo1) - .set_open(true) - .write_to_repo(tx1.mut_repo()); - let commit2 = testutils::create_random_commit(&settings, &repo1) - .set_open(true) - .write_to_repo(tx1.mut_repo()); - let commit3 = testutils::create_random_commit(&settings, &repo1) - .set_open(true) - .write_to_repo(tx1.mut_repo()); - tx1.commit(); + let tree_id1 = testutils::create_random_tree(&repo1); + let tree_id2 = testutils::create_random_tree(&repo1); + let tree_id3 = testutils::create_random_tree(&repo1); + let tree1 = repo1 + .store() + .get_tree(&RepoPath::root(), &tree_id1) + .unwrap(); + let tree2 = repo1 + .store() + .get_tree(&RepoPath::root(), &tree_id2) + .unwrap(); + let tree3 = repo1 + .store() + .get_tree(&RepoPath::root(), &tree_id3) + .unwrap(); - // Check out commit1 + // Check out tree1 let wc1 = test_workspace1.workspace.working_copy_mut(); - let tree_id1 = commit1.tree_id().clone(); // The operation ID is not correct, but that doesn't matter for this test - wc1.check_out(repo1.op_id().clone(), None, commit1).unwrap(); + wc1.check_out(repo1.op_id().clone(), None, &tree1).unwrap(); - // Check out commit2 from another process (simulated by another workspace + // Check out tree2 from another process (simulated by another workspace // instance) let mut workspace2 = Workspace::load(&settings, workspace1_root.clone()).unwrap(); workspace2 .working_copy_mut() - .check_out(repo1.op_id().clone(), Some(&tree_id1), commit2.clone()) + .check_out(repo1.op_id().clone(), Some(&tree_id1), &tree2) .unwrap(); - // Checking out another commit (via the first repo instance) should now fail. + // Checking out another tree (via the first repo instance) should now fail. assert_eq!( - wc1.check_out(repo1.op_id().clone(), Some(&tree_id1), commit3), + wc1.check_out(repo1.op_id().clone(), Some(&tree_id1), &tree3), Err(CheckoutError::ConcurrentCheckout) ); - // Check that the commit2 is still checked out on disk. + // Check that the tree2 is still checked out on disk. let workspace3 = Workspace::load(&settings, workspace1_root).unwrap(); - assert_eq!( - workspace3.working_copy().current_tree_id(), - commit2.tree().id().clone() - ); + assert_eq!(workspace3.working_copy().current_tree_id(), tree_id2); } #[test_case(false ; "local backend")] @@ -81,57 +79,46 @@ fn test_checkout_parallel(use_git: bool) { let settings = testutils::user_settings(); let mut test_workspace = testutils::init_workspace(&settings, use_git); let repo = &test_workspace.repo; - let store = repo.store(); let workspace_root = test_workspace.workspace.workspace_root().clone(); 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"); + let mut tree_ids = vec![]; 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_repo(tx.mut_repo()); - commit_ids.push(commit.id().clone()); + tree_ids.push(tree.id().clone()); } - // Create another commit just so we can test the update stats reliably from the + // Create another tree just so we can test the update stats reliably from the // first update let tree = testutils::create_tree( repo, &[(&RepoPath::from_internal_string("other file"), "contents")], ); - let commit = CommitBuilder::for_new_commit(&settings, store, tree.id().clone()) - .set_open(true) - .write_to_repo(tx.mut_repo()); - let repo = tx.commit(); test_workspace .workspace .working_copy_mut() - .check_out(repo.op_id().clone(), None, commit) + .check_out(repo.op_id().clone(), None, &tree) .unwrap(); let mut threads = vec![]; - for commit_id in &commit_ids { + for tree_id in &tree_ids { let op_id = repo.op_id().clone(); let tree_ids = tree_ids.clone(); - let commit_id = commit_id.clone(); + let tree_id = tree_id.clone(); let settings = settings.clone(); let workspace_root = workspace_root.clone(); let handle = thread::spawn(move || { let mut workspace = Workspace::load(&settings, workspace_root).unwrap(); - let commit = workspace + let tree = workspace .repo_loader() .store() - .get_commit(&commit_id) + .get_tree(&RepoPath::root(), &tree_id) .unwrap(); // The operation ID is not correct, but that doesn't matter for this test let stats = workspace .working_copy_mut() - .check_out(op_id, None, commit) + .check_out(op_id, None, &tree) .unwrap(); assert_eq!(stats.updated_files, 0); assert_eq!(stats.added_files, 1); diff --git a/src/commands.rs b/src/commands.rs index a40e778fc..fb1c855ae 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -795,7 +795,7 @@ fn update_working_copy( // TODO: CheckoutError::ConcurrentCheckout should probably just result in a // warning for most commands (but be an error for the checkout command) let stats = wc - .check_out(repo.op_id().clone(), old_tree_id, new_commit.clone()) + .check_out(repo.op_id().clone(), old_tree_id, &new_commit.tree()) .map_err(|err| { CommandError::InternalError(format!( "Failed to check out commit {}: {}",