From 8b0161ede463c4303bf8126a388f943ce50a13b8 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 21 May 2024 20:00:56 +0800 Subject: [PATCH 1/3] test: add a test case for removing symlink dir Signed-off-by: hi-rustin --- crates/cargo-util/src/paths.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index aed6ce3b1b4..fe48dd6b159 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -908,4 +908,27 @@ mod tests { ); } } + + #[test] + #[cfg(windows)] + fn test_remove_symlink_dir() { + use super::*; + use std::fs; + use std::os::windows::fs::symlink_dir; + + let tmpdir = tempfile::tempdir().unwrap(); + let dir_path = tmpdir.path().join("testdir"); + let symlink_path = tmpdir.path().join("symlink"); + + fs::create_dir(&dir_path).unwrap(); + + symlink_dir(&dir_path, &symlink_path).expect("failed to create symlink"); + + assert!(symlink_path.exists()); + + assert!(remove_file(symlink_path.clone()).is_err()); + + assert!(symlink_path.exists()); + assert!(dir_path.exists()); + } } From 0e1b115ccd7ef0a5ebac487ff0953a0b1d631d0a Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 21 May 2024 20:16:07 +0800 Subject: [PATCH 2/3] test: add case for the symlink file Signed-off-by: hi-rustin --- crates/cargo-util/src/paths.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index fe48dd6b159..be4aef207c2 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -931,4 +931,27 @@ mod tests { assert!(symlink_path.exists()); assert!(dir_path.exists()); } + + #[test] + #[cfg(windows)] + fn test_remove_symlink_file() { + use super::*; + use std::fs; + use std::os::windows::fs::symlink_file; + + let tmpdir = tempfile::tempdir().unwrap(); + let file_path = tmpdir.path().join("testfile"); + let symlink_path = tmpdir.path().join("symlink"); + + fs::write(&file_path, b"test").unwrap(); + + symlink_file(&file_path, &symlink_path).expect("failed to create symlink"); + + assert!(symlink_path.exists()); + + assert!(remove_file(symlink_path.clone()).is_ok()); + + assert!(!symlink_path.exists()); + assert!(file_path.exists()); + } } From 40ff7be1ad10d1947e22dfeb0f9fa8d2c26025a1 Mon Sep 17 00:00:00 2001 From: hi-rustin Date: Tue, 21 May 2024 20:06:35 +0800 Subject: [PATCH 3/3] fix: remove symlink on Windows Signed-off-by: hi-rustin --- crates/cargo-util/src/paths.rs | 57 +++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/crates/cargo-util/src/paths.rs b/crates/cargo-util/src/paths.rs index be4aef207c2..59e812f3aea 100644 --- a/crates/cargo-util/src/paths.rs +++ b/crates/cargo-util/src/paths.rs @@ -517,24 +517,57 @@ fn _remove_dir(p: &Path) -> Result<()> { /// /// If the file is readonly, this will attempt to change the permissions to /// force the file to be deleted. +/// On Windows, if the file is a symlink to a directory, this will attempt to remove +/// the symlink itself. pub fn remove_file>(p: P) -> Result<()> { _remove_file(p.as_ref()) } fn _remove_file(p: &Path) -> Result<()> { - let mut err = match fs::remove_file(p) { - Ok(()) => return Ok(()), - Err(e) => e, - }; - - if err.kind() == io::ErrorKind::PermissionDenied && set_not_readonly(p).unwrap_or(false) { - match fs::remove_file(p) { - Ok(()) => return Ok(()), - Err(e) => err = e, + // For Windows, we need to check if the file is a symlink to a directory + // and remove the symlink itself by calling `remove_dir` instead of + // `remove_file`. + #[cfg(target_os = "windows")] + { + use std::os::windows::fs::FileTypeExt; + let metadata = symlink_metadata(p)?; + let file_type = metadata.file_type(); + if file_type.is_symlink_dir() { + return remove_symlink_dir_with_permission_check(p); } } - Err(err).with_context(|| format!("failed to remove file `{}`", p.display())) + remove_file_with_permission_check(p) +} + +#[cfg(target_os = "windows")] +fn remove_symlink_dir_with_permission_check(p: &Path) -> Result<()> { + remove_with_permission_check(fs::remove_dir, p) + .with_context(|| format!("failed to remove symlink dir `{}`", p.display())) +} + +fn remove_file_with_permission_check(p: &Path) -> Result<()> { + remove_with_permission_check(fs::remove_file, p) + .with_context(|| format!("failed to remove file `{}`", p.display())) +} + +fn remove_with_permission_check(remove_func: F, p: P) -> io::Result<()> +where + F: Fn(P) -> io::Result<()>, + P: AsRef + Clone, +{ + match remove_func(p.clone()) { + Ok(()) => Ok(()), + Err(e) => { + if e.kind() == io::ErrorKind::PermissionDenied + && set_not_readonly(p.as_ref()).unwrap_or(false) + { + remove_func(p) + } else { + Err(e) + } + } + } } fn set_not_readonly(p: &Path) -> io::Result { @@ -926,9 +959,9 @@ mod tests { assert!(symlink_path.exists()); - assert!(remove_file(symlink_path.clone()).is_err()); + assert!(remove_file(symlink_path.clone()).is_ok()); - assert!(symlink_path.exists()); + assert!(!symlink_path.exists()); assert!(dir_path.exists()); }