diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 62fc6d273..53c561d44 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -176,6 +176,7 @@ fn sparse_patterns_from_proto(proto: &crate::protos::working_copy::TreeState) -> } fn create_parent_dirs(disk_path: &Path) -> Result<(), CheckoutError> { + // TODO: Check that we don't follow symlinks while creating parent dirs. fs::create_dir_all(disk_path.parent().unwrap()).map_err(|err| CheckoutError::IoError { message: format!( "Failed to create parent directories for {}", @@ -643,12 +644,9 @@ impl TreeState { executable: bool, ) -> Result { create_parent_dirs(disk_path)?; - // TODO: Check that we're not overwriting an un-ignored file here (which might - // be created by a concurrent process). let mut file = OpenOptions::new() .write(true) - .create(true) - .truncate(true) + .create_new(true) // Don't overwrite un-ignored file. Don't follow symlink. .open(disk_path) .map_err(|err| CheckoutError::IoError { message: format!("Failed to open file {} for writing", disk_path.display()), @@ -710,12 +708,9 @@ impl TreeState { ) -> Result { create_parent_dirs(disk_path)?; let conflict = self.store.read_conflict(path, id)?; - // TODO: Check that we're not overwriting an un-ignored file here (which might - // be created by a concurrent process). let mut file = OpenOptions::new() .write(true) - .create(true) - .truncate(true) + .create_new(true) // Don't overwrite un-ignored file. Don't follow symlink. .open(disk_path) .map_err(|err| CheckoutError::IoError { message: format!("Failed to open file {} for writing", disk_path.display()), diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index 315f839e7..ceb8b8444 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -554,9 +554,9 @@ fn test_gitignores(use_git: bool) { #[test_case(false ; "local backend")] #[test_case(true ; "git backend")] -fn test_gitignores_checkout_overwrites_ignored(use_git: bool) { - // Tests that a .gitignore'd file gets overwritten if check out a commit where - // the file is tracked. +fn test_gitignores_checkout_never_overwrites_ignored(use_git: bool) { + // Tests that a .gitignore'd file doesn't get overwritten if check out a commit + // where the file is tracked. let _home_dir = testutils::new_user_home(); let settings = testutils::user_settings(); @@ -579,27 +579,15 @@ fn test_gitignores_checkout_overwrites_ignored(use_git: bool) { let tree = repo.store().get_tree(&RepoPath::root(), &tree_id).unwrap(); // Now check out the tree that adds the file "modified" with contents - // "contents". The exiting contents ("garbage") should be replaced in the + // "contents". The exiting contents ("garbage") shouldn't be replaced in the // working copy. let wc = test_workspace.workspace.working_copy_mut(); - wc.check_out(repo.op_id().clone(), None, &tree).unwrap(); + assert!(wc.check_out(repo.op_id().clone(), None, &tree).is_err()); - // Check that the new contents are in the working copy + // Check that the old contents are in the working copy let path = workspace_root.join("modified"); assert!(path.is_file()); - assert_eq!(std::fs::read(&path).unwrap(), b"contents"); - - // Check that the file is in the tree created by snapshotting the working copy - let mut locked_wc = wc.start_mutation(); - let new_tree_id = locked_wc.snapshot(GitIgnoreFile::empty()).unwrap(); - locked_wc.discard(); - let new_tree = repo - .store() - .get_tree(&RepoPath::root(), &new_tree_id) - .unwrap(); - assert!(new_tree - .entry(&RepoPathComponent::from("modified")) - .is_some()); + assert_eq!(std::fs::read(&path).unwrap(), b"garbage"); } #[test_case(false ; "local backend")] diff --git a/lib/tests/test_working_copy_sparse.rs b/lib/tests/test_working_copy_sparse.rs index defa81f19..8fe73a549 100644 --- a/lib/tests/test_working_copy_sparse.rs +++ b/lib/tests/test_working_copy_sparse.rs @@ -183,11 +183,6 @@ fn test_sparse_commit() { let sparse_patterns = vec![dir1_path, dir2_path]; locked_wc.set_sparse_patterns(sparse_patterns).unwrap(); locked_wc.finish(repo.op_id().clone()); - // Write out a modified version of dir2/file1 again because it was overwritten - // when we added dir2/ to the sparse patterns. - // TODO: We shouldn't overwrite files when updating (there's already a TODO - // about that in `TreeState::write_file()`). - std::fs::write(dir2_file1_path.to_fs_path(&working_copy_path), "modified").unwrap(); // Create a tree from the working copy. Only dir1/file1 and dir2/file1 should be // updated in the tree.