From c08adba424a4c019e5e2bfd052b6c329341e3700 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 21 Nov 2021 14:39:34 -0800 Subject: [PATCH] repo: remove working_copy(), only used in tests This is another step towards removing coupling between the repo and the working copy, so we can have multiple working copies for a single repo (#13). --- lib/src/repo.rs | 8 --- lib/tests/test_working_copy.rs | 66 +++++++++++------------ lib/tests/test_working_copy_concurrent.rs | 53 +++++++++++------- 3 files changed, 65 insertions(+), 62 deletions(-) diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 51c0942e3..72fd8fd24 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -307,14 +307,6 @@ impl ReadonlyRepo { self.index() } - pub fn working_copy(&self) -> WorkingCopy { - WorkingCopy::load( - self.store.clone(), - self.wc_path.clone(), - self.repo_path.join("working_copy"), - ) - } - pub fn store(&self) -> &Arc { &self.store } diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 1c27c2943..0b749853a 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -35,10 +35,10 @@ use test_case::test_case; fn test_root(use_git: bool) { // Test that the working copy is clean and empty after init. let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); let locked_wc = wc.write_tree(); let new_tree_id = locked_wc.new_tree_id(); locked_wc.discard(); @@ -55,7 +55,7 @@ fn test_checkout_file_transitions(use_git: bool) { // additions and removals as well. let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; let store = repo.store().clone(); @@ -215,7 +215,7 @@ fn test_checkout_file_transitions(use_git: bool) { .write_to_repo(tx.mut_repo()); tx.commit(); - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); wc.check_out(left_commit).unwrap(); wc.check_out(right_commit.clone()).unwrap(); @@ -293,7 +293,7 @@ fn test_checkout_file_transitions(use_git: bool) { #[test] fn test_untrack() { let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, false); + let mut test_workspace = testutils::init_repo(&settings, false); let repo = &test_workspace.repo; let wanted_path = RepoPath::from_internal_string("wanted"); @@ -318,7 +318,7 @@ fn test_untrack() { ) .write_to_repo(tx.mut_repo()); let repo = tx.commit(); - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); wc.check_out(initial_commit.clone()).unwrap(); // Now we untrack the file called "unwanted" @@ -357,12 +357,12 @@ fn test_commit_racy_timestamps(use_git: bool) { // millisecond as the updated working copy state. let _home_dir = testutils::new_user_home(); let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; let file_path = repo.working_copy_path().join("file"); let mut previous_tree_id = repo.store().empty_tree_id().clone(); - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); for i in 0..100 { { let mut file = OpenOptions::new() @@ -388,9 +388,9 @@ fn test_gitignores(use_git: bool) { let _home_dir = testutils::new_user_home(); let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; - let workspace_root = &test_workspace.workspace.workspace_root(); + let workspace_root = test_workspace.workspace.workspace_root().clone(); let gitignore_path = RepoPath::from_internal_string(".gitignore"); let added_path = RepoPath::from_internal_string("added"); @@ -400,13 +400,13 @@ fn test_gitignores(use_git: bool) { let subdir_modified_path = RepoPath::from_internal_string("dir/modified"); let subdir_ignored_path = RepoPath::from_internal_string("dir/ignored"); - testutils::write_working_copy_file(workspace_root, &gitignore_path, "ignored\n"); - testutils::write_working_copy_file(workspace_root, &modified_path, "1"); - testutils::write_working_copy_file(workspace_root, &removed_path, "1"); + testutils::write_working_copy_file(&workspace_root, &gitignore_path, "ignored\n"); + testutils::write_working_copy_file(&workspace_root, &modified_path, "1"); + testutils::write_working_copy_file(&workspace_root, &removed_path, "1"); std::fs::create_dir(workspace_root.join("dir")).unwrap(); - testutils::write_working_copy_file(workspace_root, &subdir_modified_path, "1"); + testutils::write_working_copy_file(&workspace_root, &subdir_modified_path, "1"); - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); let locked_wc = wc.write_tree(); let new_tree_id1 = locked_wc.new_tree_id(); locked_wc.discard(); @@ -426,16 +426,16 @@ fn test_gitignores(use_git: bool) { ); testutils::write_working_copy_file( - workspace_root, + &workspace_root, &gitignore_path, "ignored\nmodified\nremoved\n", ); - testutils::write_working_copy_file(workspace_root, &added_path, "2"); - testutils::write_working_copy_file(workspace_root, &modified_path, "2"); - std::fs::remove_file(removed_path.to_fs_path(workspace_root)).unwrap(); - testutils::write_working_copy_file(workspace_root, &ignored_path, "2"); - testutils::write_working_copy_file(workspace_root, &subdir_modified_path, "2"); - testutils::write_working_copy_file(workspace_root, &subdir_ignored_path, "2"); + testutils::write_working_copy_file(&workspace_root, &added_path, "2"); + testutils::write_working_copy_file(&workspace_root, &modified_path, "2"); + std::fs::remove_file(removed_path.to_fs_path(&workspace_root)).unwrap(); + testutils::write_working_copy_file(&workspace_root, &ignored_path, "2"); + testutils::write_working_copy_file(&workspace_root, &subdir_modified_path, "2"); + testutils::write_working_copy_file(&workspace_root, &subdir_ignored_path, "2"); let locked_wc = wc.write_tree(); let new_tree_id2 = locked_wc.new_tree_id(); @@ -464,7 +464,7 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) { let _home_dir = testutils::new_user_home(); let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; let workspace_root = test_workspace.workspace.workspace_root(); @@ -490,7 +490,7 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) { // Now check out the commit that adds the file "modified" with contents // "contents". The exiting contents ("garbage") should be replaced in the // working copy. - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); wc.check_out(commit).unwrap(); // Check that the new contents are in the working copy @@ -522,7 +522,7 @@ fn test_gitignores_ignored_directory_already_tracked(use_git: bool) { let _home_dir = testutils::new_user_home(); let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; // Add a .gitignore file saying to ignore the directory "ignored/" @@ -548,7 +548,7 @@ fn test_gitignores_ignored_directory_already_tracked(use_git: bool) { let repo = tx.commit(); // Check out the commit with the file in ignored/ - let mut wc = repo.working_copy(); + let wc = test_workspace.workspace.working_copy_mut(); wc.check_out(commit).unwrap(); // Check that the file is still in the tree created by committing the working @@ -571,22 +571,20 @@ fn test_dotgit_ignored(use_git: bool) { let _home_dir = testutils::new_user_home(); let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&settings, use_git); let repo = &test_workspace.repo; - let workspace_root = test_workspace.workspace.workspace_root(); - - let mut wc = repo.working_copy(); + let workspace_root = test_workspace.workspace.workspace_root().clone(); // Test with a .git/ directory (with a file in, since we don't write empty // trees) let dotgit_path = repo.working_copy_path().join(".git"); std::fs::create_dir(&dotgit_path).unwrap(); testutils::write_working_copy_file( - workspace_root, + &workspace_root, &RepoPath::from_internal_string(".git/file"), "contents", ); - let locked_working_copy = wc.write_tree(); + let locked_working_copy = test_workspace.workspace.working_copy_mut().write_tree(); let new_tree_id = locked_working_copy.new_tree_id(); assert_eq!(new_tree_id, *repo.store().empty_tree_id()); locked_working_copy.discard(); @@ -594,11 +592,11 @@ fn test_dotgit_ignored(use_git: bool) { // Test with a .git file testutils::write_working_copy_file( - workspace_root, + &workspace_root, &RepoPath::from_internal_string(".git"), "contents", ); - let locked_working_copy = wc.write_tree(); + let locked_working_copy = test_workspace.workspace.working_copy_mut().write_tree(); let new_tree_id = locked_working_copy.new_tree_id(); assert_eq!(new_tree_id, *repo.store().empty_tree_id()); locked_working_copy.discard(); diff --git a/lib/tests/test_working_copy_concurrent.rs b/lib/tests/test_working_copy_concurrent.rs index 37a81dd13..5faa3f575 100644 --- a/lib/tests/test_working_copy_concurrent.rs +++ b/lib/tests/test_working_copy_concurrent.rs @@ -17,10 +17,10 @@ use std::collections::HashSet; use std::thread; use jujutsu_lib::commit_builder::CommitBuilder; -use jujutsu_lib::repo::ReadonlyRepo; use jujutsu_lib::repo_path::RepoPath; use jujutsu_lib::testutils; use jujutsu_lib::working_copy::CheckoutError; +use jujutsu_lib::workspace::Workspace; use test_case::test_case; #[test_case(false ; "local backend")] @@ -29,28 +29,33 @@ fn test_concurrent_checkout(use_git: bool) { // Test that we error out if a concurrent checkout is detected (i.e. if the // current checkout changed on disk after we read it). let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); - let repo1 = &test_workspace.repo; + let mut test_workspace1 = testutils::init_repo(&settings, use_git); + 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) + 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) + 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) + let commit3 = testutils::create_random_commit(&settings, &repo1) .set_open(true) .write_to_repo(tx1.mut_repo()); tx1.commit(); // Check out commit1 - let mut wc1 = repo1.working_copy(); + let wc1 = test_workspace1.workspace.working_copy_mut(); wc1.check_out(commit1).unwrap(); - // Check out commit2 from another process (simulated by another repo instance) - let repo2 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()).unwrap(); - repo2.working_copy().check_out(commit2.clone()).unwrap(); + // Check out commit2 from another process (simulated by another workspace + // instance) + let mut workspace2 = Workspace::load(&settings, workspace1_root.clone()).unwrap(); + workspace2 + .working_copy_mut() + .check_out(commit2.clone()) + .unwrap(); // Checking out another commit (via the first repo instance) should now fail. assert_eq!( @@ -59,9 +64,9 @@ fn test_concurrent_checkout(use_git: bool) { ); // Check that the commit2 is still checked out on disk. - let repo3 = ReadonlyRepo::load(&settings, repo1.working_copy_path().clone()).unwrap(); + let workspace3 = Workspace::load(&settings, workspace1_root).unwrap(); assert_eq!( - repo3.working_copy().current_tree_id(), + workspace3.working_copy().current_tree_id(), commit2.tree().id().clone() ); } @@ -72,9 +77,10 @@ fn test_checkout_parallel(use_git: bool) { // Test that concurrent checkouts by different processes (simulated by using // different repo instances) is safe. let settings = testutils::user_settings(); - let test_workspace = testutils::init_repo(&settings, use_git); + let mut test_workspace = testutils::init_repo(&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(); @@ -100,19 +106,26 @@ fn test_checkout_parallel(use_git: bool) { .set_open(true) .write_to_repo(tx.mut_repo()); tx.commit(); - repo.working_copy().check_out(commit).unwrap(); + test_workspace + .workspace + .working_copy_mut() + .check_out(commit) + .unwrap(); let mut threads = vec![]; for commit_id in &commit_ids { let tree_ids = tree_ids.clone(); let commit_id = commit_id.clone(); let settings = settings.clone(); - let working_copy_path = repo.working_copy_path().clone(); + let workspace_root = workspace_root.clone(); let handle = thread::spawn(move || { - let repo = ReadonlyRepo::load(&settings, working_copy_path).unwrap(); - let mut wc = repo.working_copy(); - let commit = repo.store().get_commit(&commit_id).unwrap(); - let stats = wc.check_out(commit).unwrap(); + let mut workspace = Workspace::load(&settings, workspace_root).unwrap(); + let commit = workspace + .repo_loader() + .store() + .get_commit(&commit_id) + .unwrap(); + let stats = workspace.working_copy_mut().check_out(commit).unwrap(); assert_eq!(stats.updated_files, 0); assert_eq!(stats.added_files, 1); assert_eq!(stats.removed_files, 1); @@ -120,7 +133,7 @@ fn test_checkout_parallel(use_git: bool) { // different tree than the one we just checked out, but since // write_tree() should take the same lock as check_out(), write_tree() // should never produce a different tree. - let locked_wc = wc.write_tree(); + let locked_wc = workspace.working_copy_mut().write_tree(); let new_tree_id = locked_wc.new_tree_id(); locked_wc.discard(); assert!(tree_ids.contains(&new_tree_id));