mirror of
https://github.com/martinvonz/jj.git
synced 2025-02-01 00:50:57 +00:00
local_working_copy: detect error of file removal earlier
This should be safer than relying on file open error. It's scary to continue processing if the file was a symlink. I'll add a few more sanity checks to remove_old_file(), so it's extracted as a function.
This commit is contained in:
parent
1c30f3b3e8
commit
8540536ea2
2 changed files with 40 additions and 1 deletions
|
@ -466,6 +466,18 @@ fn create_parent_dirs(
|
|||
Ok(false)
|
||||
}
|
||||
|
||||
/// Removes existing file named `disk_path` if any.
|
||||
fn remove_old_file(disk_path: &Path) -> Result<(), CheckoutError> {
|
||||
match fs::remove_file(disk_path) {
|
||||
Ok(()) => Ok(()),
|
||||
Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(()),
|
||||
Err(err) => Err(CheckoutError::Other {
|
||||
message: format!("Failed to remove file {}", disk_path.display()),
|
||||
err: err.into(),
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn mtime_from_metadata(metadata: &Metadata) -> MillisSinceEpoch {
|
||||
let time = metadata
|
||||
.modified()
|
||||
|
@ -1415,7 +1427,7 @@ impl TreeState {
|
|||
let disk_path = path.to_fs_path(&self.working_copy_path)?;
|
||||
|
||||
if present_before {
|
||||
fs::remove_file(&disk_path).ok();
|
||||
remove_old_file(&disk_path)?;
|
||||
} else if disk_path.exists() {
|
||||
changed_file_states.push((path, FileState::placeholder()));
|
||||
stats.skipped_files += 1;
|
||||
|
|
|
@ -1184,6 +1184,33 @@ fn test_gitsubmodule() {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_check_out_existing_file_cannot_be_removed() {
|
||||
let settings = testutils::user_settings();
|
||||
let mut test_workspace = TestWorkspace::init(&settings);
|
||||
let repo = &test_workspace.repo;
|
||||
let workspace_root = test_workspace.workspace.workspace_root().to_owned();
|
||||
|
||||
let file_path = RepoPath::from_internal_string("file");
|
||||
let tree1 = create_tree(repo, &[(file_path, "0")]);
|
||||
let tree2 = create_tree(repo, &[(file_path, "1")]);
|
||||
let commit1 = commit_with_tree(repo.store(), tree1.id());
|
||||
let commit2 = commit_with_tree(repo.store(), tree2.id());
|
||||
|
||||
let ws = &mut test_workspace.workspace;
|
||||
ws.check_out(repo.op_id().clone(), None, &commit1).unwrap();
|
||||
|
||||
// Trigger IO error by replacing file with directory.
|
||||
std::fs::remove_file(file_path.to_fs_path_unchecked(&workspace_root)).unwrap();
|
||||
std::fs::create_dir(file_path.to_fs_path_unchecked(&workspace_root)).unwrap();
|
||||
|
||||
let result = ws.check_out(repo.op_id().clone(), None, &commit2);
|
||||
assert_matches!(
|
||||
result,
|
||||
Err(CheckoutError::Other { message, .. }) if message.contains("Failed to remove")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_existing_directory_symlink() {
|
||||
let settings = testutils::user_settings();
|
||||
|
|
Loading…
Reference in a new issue