Merge pull request #1315 from rivy/fix.rm

fix ~ rm: fix dir-type symlink removal on windows

src/rm/rm.rs

@@ -163,6 +163,8 @@ fn remove(files: Vec<String>, options: Options) -> bool {
             Ok(metadata) => {
                 if metadata.is_dir() {
                     handle_dir(file, &options)
+                } else if is_symlink_dir(&metadata) {
+                    remove_dir(file, &options)
                 } else {
                     remove_file(file, &options)
                 }
@@ -305,3 +307,20 @@ fn prompt(msg: &str) -> bool {
         _ => false,
     }
 }
+
+#[cfg(not(windows))]
+fn is_symlink_dir(_metadata: &fs::Metadata) -> bool {
+    false
+}
+
+#[cfg(windows)]
+use std::os::windows::prelude::MetadataExt;
+
+#[cfg(windows)]
+fn is_symlink_dir(metadata: &fs::Metadata) -> bool {
+    use std::os::raw::c_ulong;
+    pub type DWORD = c_ulong;
+    pub const FILE_ATTRIBUTE_DIRECTORY: DWORD = 0x10;
+
+    metadata.file_type().is_symlink() && ((metadata.file_attributes() & FILE_ATTRIBUTE_DIRECTORY) != 0)
+}

tests/common/util.rs

@@ -5,9 +5,9 @@ use std::env;
 use std::fs::{self, File, OpenOptions};
 use std::io::{Read, Result, Write};
 #[cfg(unix)]
-use std::os::unix::fs::symlink as symlink_file;
+use std::os::unix::fs::{symlink as symlink_dir, symlink as symlink_file};
 #[cfg(windows)]
-use std::os::windows::fs::symlink_file;
+use std::os::windows::fs::{symlink_dir, symlink_file};
 use std::path::{Path, PathBuf};
 use std::process::{Child, Command, Stdio};
 use std::str::from_utf8;
@@ -279,7 +279,7 @@ impl AtPath {
         File::create(&self.plus(file)).unwrap();
     }
 
-    pub fn symlink(&self, src: &str, dst: &str) {
+    pub fn symlink_file(&self, src: &str, dst: &str) {
         log_info(
             "symlink",
             &format!("{},{}", self.plus_as_string(src), self.plus_as_string(dst)),
@@ -287,6 +287,14 @@ impl AtPath {
         symlink_file(&self.plus(src), &self.plus(dst)).unwrap();
     }
 
+    pub fn symlink_dir(&self, src: &str, dst: &str) {
+        log_info(
+            "symlink",
+            &format!("{},{}", self.plus_as_string(src), self.plus_as_string(dst)),
+        );
+        symlink_dir(&self.plus(src), &self.plus(dst)).unwrap();
+    }
+
     pub fn is_symlink(&self, path: &str) -> bool {
         log_info("is_symlink", self.plus_as_string(path));
         match fs::symlink_metadata(&self.plus(path)) {

tests/test_chgrp.rs

@@ -67,7 +67,7 @@ fn test_preserve_root_symlink() {
                "..//../../..//../..//../../../../../../../../",
                ".//../../../../../../..//../../../../../../../"] {
         let (at, mut ucmd) = at_and_ucmd!();
-        at.symlink(d, file);
+        at.symlink_file(d, file);
         ucmd.arg("--preserve-root")
             .arg("-HR")
             .arg("bin").arg(file)
@@ -76,7 +76,7 @@ fn test_preserve_root_symlink() {
     }
 
     let (at, mut ucmd) = at_and_ucmd!();
-    at.symlink("///usr", file);
+    at.symlink_file("///usr", file);
     ucmd.arg("--preserve-root")
         .arg("-HR")
         .arg("bin").arg(format!(".//{}/..//..//../../", file))
@@ -84,7 +84,7 @@ fn test_preserve_root_symlink() {
         .stderr_is("chgrp: it is dangerous to operate recursively on '/'\nchgrp: use --no-preserve-root to override this failsafe");
 
     let (at, mut ucmd) = at_and_ucmd!();
-    at.symlink("/", "/tmp/__root__");
+    at.symlink_file("/", "/tmp/__root__");
     ucmd.arg("--preserve-root")
         .arg("-R")
         .arg("bin").arg("/tmp/__root__/.")

tests/test_ln.rs

@@ -87,7 +87,7 @@ fn test_symlink_overwrite_force() {
     let link = "test_symlink_overwrite_force_link";
 
     // Create symlink
-    at.symlink(file_a, link);
+    at.symlink_file(file_a, link);
     assert!(at.is_symlink(link));
     assert_eq!(at.resolve_link(link), file_a);
 
@@ -130,7 +130,7 @@ fn test_symlink_simple_backup() {
     let link = "test_symlink_simple_backup_link";
 
     at.touch(file);
-    at.symlink(file, link);
+    at.symlink_file(file, link);
     assert!(at.file_exists(file));
     assert!(at.is_symlink(link));
     assert_eq!(at.resolve_link(link), file);
@@ -155,7 +155,7 @@ fn test_symlink_custom_backup_suffix() {
     let suffix = "super-suffix-of-the-century";
 
     at.touch(file);
-    at.symlink(file, link);
+    at.symlink_file(file, link);
     assert!(at.file_exists(file));
     assert!(at.is_symlink(link));
     assert_eq!(at.resolve_link(link), file);
@@ -179,7 +179,7 @@ fn test_symlink_backup_numbering() {
     let link = "test_symlink_backup_numbering_link";
 
     at.touch(file);
-    at.symlink(file, link);
+    at.symlink_file(file, link);
     assert!(at.file_exists(file));
     assert!(at.is_symlink(link));
     assert_eq!(at.resolve_link(link), file);
@@ -205,13 +205,13 @@ fn test_symlink_existing_backup() {
 
     // Create symlink and verify
     at.touch(file);
-    at.symlink(file, link);
+    at.symlink_file(file, link);
     assert!(at.file_exists(file));
     assert!(at.is_symlink(link));
     assert_eq!(at.resolve_link(link), file);
 
     // Create backup symlink and verify
-    at.symlink(file, link_backup);
+    at.symlink_file(file, link_backup);
     assert!(at.file_exists(file));
     assert!(at.is_symlink(link_backup));
     assert_eq!(at.resolve_link(link_backup), file);

tests/test_rm.rs

@@ -144,7 +144,7 @@ fn test_rm_dir_symlink() {
     let link = "test_rm_dir_symlink_link";
 
     at.mkdir(dir);
-    at.symlink(dir, link);
+    at.symlink_dir(dir, link);
 
     ucmd.arg(link).succeeds();
 }
@@ -154,7 +154,7 @@ fn test_rm_invalid_symlink() {
     let (at, mut ucmd) = at_and_ucmd!();
     let link = "test_rm_invalid_symlink";
 
-    at.symlink(link, link);
+    at.symlink_file(link, link);
 
     ucmd.arg(link).succeeds();
 }

tests/test_touch.rs

@@ -231,7 +231,7 @@ fn test_touch_no_dereference() {
 
     at.touch(file_a);
     set_file_times(&at, file_a, start_of_year, start_of_year);
-    at.symlink(file_a, file_b);
+    at.symlink_file(file_a, file_b);
     assert!(at.file_exists(file_a));
     assert!(at.is_symlink(file_b));