ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

working_copy: take Tree, not CommitId, as argument to check_out()

Browse source 
We no longer need the commit ID, so we shouldn't make the callers pass
it. This lets us simplify several tests, because they no longer to
create commits just to check out a tree in the working copy.
This commit is contained in:
Martin von Zweigbergk 2022-02-12 23:46:38 -08:00
parent 315e5e87a3
commit 03e6b8c0e6
4 changed files with 50 additions and 101 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
lib/src/working_copy.rs
View file

@ -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<CheckoutStats, CheckoutError> {
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)
}

67
lib/tests/test_working_copy.rs
View file

@ -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)

77
lib/tests/test_working_copy_concurrent.rs
View file

@ -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);

2
src/commands.rs
View file

@ -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 {}: {}",