fix(mv): Allow move across file systems (#1524)

Co-authored-by: Arni Dagur <arni@dagur.eu>

Cargo.lock

@@ -574,6 +574,11 @@ dependencies = [
  "uucore 0.0.2 (git+https://github.com/uutils/uucore/?tag=0.0.2)",
 ]
 
+[[package]]
+name = "fs_extra"
+version = "1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "fuchsia-cprng"
 version = "0.1.1"
@@ -910,6 +915,7 @@ dependencies = [
 name = "mv"
 version = "0.0.1"
 dependencies = [
+ "fs_extra 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)",
  "uucore 0.0.2 (git+https://github.com/uutils/uucore/?tag=0.0.2)",
 ]
@@ -2237,6 +2243,7 @@ dependencies = [
 "checksum fake-simd 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "e88a8acf291dafb59c2d96e8f59828f3838bb1a70398823ade51a84de6a6deed"
 "checksum filetime 0.2.5 (registry+https://github.com/rust-lang/crates.io-index)" = "2f8c63033fcba1f51ef744505b3cad42510432b904c062afa67ad7ece008429d"
 "checksum fnv 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2fad85553e09a6f881f739c29f0b00b0f01357c743266d478b68951ce23285f3"
+"checksum fs_extra 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5f2a4a2034423744d2cc7ca2068453168dcdb82c438419e639a26bd87839c674"
 "checksum fuchsia-cprng 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a06f77d526c1a601b7c4cdd98f54b5eaabffc14d5f2f0296febdc7f357c6d3ba"
 "checksum generic-array 0.8.3 (registry+https://github.com/rust-lang/crates.io-index)" = "fceb69994e330afed50c93524be68c42fa898c2d9fd4ee8da03bd7363acd26f2"
 "checksum getopts 0.2.19 (registry+https://github.com/rust-lang/crates.io-index)" = "72327b15c228bfe31f1390f93dd5e9279587f0463836393c9df719ce62a3e450"

src/uu/mv/Cargo.toml

@@ -12,6 +12,7 @@ path = "src/mv.rs"
 [dependencies]
 getopts = "0.2.18"
 uucore = "0.0.2"
+fs_extra = "1.1.0"
 
 [[bin]]
 name = "mv"

src/uu/mv/src/mv.rs

@@ -9,6 +9,7 @@
 // that was distributed with this source code.
 //
 
+extern crate fs_extra;
 extern crate getopts;
 
 #[macro_use]
@@ -16,9 +17,15 @@ extern crate uucore;
 
 use std::env;
 use std::fs;
-use std::io::{stdin, Result};
+use std::io::{self, stdin};
+#[cfg(unix)]
+use std::os::unix;
+#[cfg(windows)]
+use std::os::windows;
 use std::path::{Path, PathBuf};
 
+use fs_extra::dir::{move_dir, CopyOptions as DirCopyOptions};
+
 static NAME: &str = "mv";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 
@@ -237,7 +244,11 @@ fn exec(files: &[PathBuf], b: Behaviour) -> i32 {
         2 => {
             let source = &files[0];
             let target = &files[1];
-            if !source.exists() {
+            // Here we use the `symlink_metadata()` method instead of `exists()`,
+            // since it handles dangling symlinks correctly. The method gives an
+            // `Ok()` results unless the source does not exist, or the user
+            // lacks permission to access metadata.
+            if source.symlink_metadata().is_err() {
                 show_error!(
                     "cannot stat ‘{}’: No such file or directory",
                     source.display()
@@ -339,7 +350,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behaviour) -
     }
 }
 
-fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> {
+fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> io::Result<()> {
     let mut backup_path = None;
 
     if to.exists() {
@@ -360,8 +371,8 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> {
             BackupMode::NumberedBackup => Some(numbered_backup_path(to)),
             BackupMode::ExistingBackup => Some(existing_backup_path(to, &b.suffix)),
         };
-        if let Some(ref p) = backup_path {
+        if let Some(ref backup_path) = backup_path {
-            fs::rename(to, p)?;
+            rename_with_fallback(to, backup_path)?;
         }
 
         if b.update && fs::metadata(from)?.modified()? <= fs::metadata(to)?.modified()? {
@@ -376,15 +387,12 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> {
             if is_empty_dir(to) {
                 fs::remove_dir(to)?
             } else {
-                return Err(std::io::Error::new(
+                return Err(io::Error::new(io::ErrorKind::Other, "Directory not empty"));
-                    std::io::ErrorKind::Other,
-                    "Directory not empty",
-                ));
             }
         }
     }
 
-    fs::rename(from, to)?;
+    rename_with_fallback(from, to)?;
 
     if b.verbose {
         print!("‘{}’ -> ‘{}’", from.display(), to.display());
@@ -396,6 +404,75 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behaviour) -> Result<()> {
     Ok(())
 }
 
+/// A wrapper around `fs::rename`, so that if it fails, we try falling back on
+/// copying and removing.
+fn rename_with_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> {
+    if fs::rename(from, to).is_err() {
+        // Get metadata without following symlinks
+        let metadata = from.symlink_metadata()?;
+        let file_type = metadata.file_type();
+
+        if file_type.is_symlink() {
+            rename_symlink_fallback(&from, &to)?;
+        } else if file_type.is_dir() {
+            // We remove the destination directory if it exists to match the
+            // behaviour of `fs::rename`. As far as I can tell, `fs_extra`'s
+            // `move_dir` would otherwise behave differently.
+            if to.exists() {
+                fs::remove_dir_all(to)?;
+            }
+            let options = DirCopyOptions {
+                // From the `fs_extra` documentation:
+                // "Recursively copy a directory with a new name or place it
+                // inside the destination. (same behaviors like cp -r in Unix)"
+                copy_inside: true,
+                ..DirCopyOptions::new()
+            };
+            if let Err(err) = move_dir(from, to, &options) {
+                return Err(io::Error::new(io::ErrorKind::Other, format!("{:?}", err)));
+            }
+        } else {
+            fs::copy(from, to).and_then(|_| fs::remove_file(from))?;
+        }
+    }
+    Ok(())
+}
+
+/// Move the given symlink to the given destination. On Windows, dangling
+/// symlinks return an error.
+#[inline]
+fn rename_symlink_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> {
+    let path_symlink_points_to = fs::read_link(from)?;
+    #[cfg(unix)]
+    {
+        unix::fs::symlink(&path_symlink_points_to, &to).and_then(|_| fs::remove_file(&from))?;
+    }
+    #[cfg(windows)]
+    {
+        if path_symlink_points_to.exists() {
+            if path_symlink_points_to.is_dir() {
+                windows::fs::symlink_dir(&path_symlink_points_to, &to)?;
+            } else {
+                windows::fs::symlink_file(&path_symlink_points_to, &to)?;
+            }
+            fs::remove_file(&from)?;
+        } else {
+            return Err(io::Error::new(
+                io::ErrorKind::NotFound,
+                "can't determine symlink type, since it is dangling",
+            ));
+        }
+    }
+    #[cfg(not(any(windows, unix)))]
+    {
+        return Err(io::Error::new(
+            io::ErrorKind::Other,
+            "your operating system does not support symlinks",
+        ));
+    }
+    Ok(())
+}
+
 fn read_yes() -> bool {
     let mut s = String::new();
     match stdin().read_line(&mut s) {