From 75b7f768eae75d9368c10e96ebfbdccc39d7a206 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 24 May 2020 12:49:56 +0200 Subject: [PATCH] fix(mv): Allow move across file systems (#1524) Co-authored-by: Arni Dagur --- Cargo.lock | 7 ++++ src/uu/mv/Cargo.toml | 1 + src/uu/mv/src/mv.rs | 97 +++++++++++++++++++++++++++++++++++++++----- 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a10cc4336..b7b5fe2d1 100644 --- a/Cargo.lock +++ b/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" diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index 4432c17b6..03afd1a0c 100644 --- a/src/uu/mv/Cargo.toml +++ b/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" diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 807acedee..af0878ea9 100644 --- a/src/uu/mv/src/mv.rs +++ b/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 { - fs::rename(to, p)?; + if let Some(ref backup_path) = backup_path { + 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( - std::io::ErrorKind::Other, - "Directory not empty", - )); + return Err(io::Error::new(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) {