Merge pull request #4999 from sylvestre/mv-backup-src

mv: add the check with --b=simple and when the source is a backup

src/uu/mv/src/mv.rs

@@ -22,7 +22,7 @@ use std::os::unix;
 #[cfg(windows)]
 use std::os::windows;
 use std::path::{Path, PathBuf};
-use uucore::backup_control::{self, BackupMode};
+use uucore::backup_control::{self, source_is_target_backup, BackupMode};
 use uucore::display::Quotable;
 use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError, UUsageError};
 use uucore::fs::{are_hardlinks_or_one_way_symlink_to_same_file, are_hardlinks_to_same_file};
@@ -251,6 +251,17 @@ fn parse_paths(files: &[OsString], b: &Behavior) -> Vec<PathBuf> {
 }
 
 fn handle_two_paths(source: &Path, target: &Path, b: &Behavior) -> UResult<()> {
+    if b.backup == BackupMode::SimpleBackup && source_is_target_backup(source, target, &b.suffix) {
+        return Err(io::Error::new(
+            io::ErrorKind::NotFound,
+            format!(
+                "backing up {} might destroy source;  {} not moved",
+                target.quote(),
+                source.quote()
+            ),
+        )
+        .into());
+    }
     if source.symlink_metadata().is_err() {
         return Err(MvError::NoSuchFile(source.quote().to_string()).into());
     }

src/uucore/src/lib/mods/backup_control.rs

@@ -438,6 +438,32 @@ fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf {
     }
 }
 
+/// Returns true if the source file is likely to be the simple backup file for the target file.
+///
+/// # Arguments
+///
+/// * `source` - A Path reference that holds the source (backup) file path.
+/// * `target` - A Path reference that holds the target file path.
+/// * `suffix` - Str that holds the backup suffix.
+///
+/// # Examples
+///
+/// ```
+/// use std::path::Path;
+/// use uucore::backup_control::source_is_target_backup;
+/// let source = Path::new("data.txt~");
+/// let target = Path::new("data.txt");
+/// let suffix = String::from("~");
+///
+/// assert_eq!(source_is_target_backup(&source, &target, &suffix), true);
+/// ```
+///
+pub fn source_is_target_backup(source: &Path, target: &Path, suffix: &str) -> bool {
+    let source_filename = source.to_string_lossy();
+    let target_backup_filename = format!("{}{suffix}", target.to_string_lossy());
+    source_filename == target_backup_filename
+}
+
 //
 // Tests for this module
 //
@@ -626,4 +652,30 @@ mod tests {
         let result = determine_backup_suffix(&matches);
         assert_eq!(result, "-v");
     }
+    #[test]
+    fn test_source_is_target_backup() {
+        let source = Path::new("data.txt.bak");
+        let target = Path::new("data.txt");
+        let suffix = String::from(".bak");
+
+        assert!(source_is_target_backup(&source, &target, &suffix));
+    }
+
+    #[test]
+    fn test_source_is_not_target_backup() {
+        let source = Path::new("data.txt");
+        let target = Path::new("backup.txt");
+        let suffix = String::from(".bak");
+
+        assert!(!source_is_target_backup(&source, &target, &suffix));
+    }
+
+    #[test]
+    fn test_source_is_target_backup_with_tilde_suffix() {
+        let source = Path::new("example~");
+        let target = Path::new("example");
+        let suffix = String::from("~");
+
+        assert!(source_is_target_backup(&source, &target, &suffix));
+    }
 }

tests/by-util/test_mv.rs

@@ -510,6 +510,22 @@ fn test_mv_same_hardlink_backup_simple() {
         .succeeds();
 }
 
+#[test]
+#[cfg(all(unix, not(target_os = "android")))]
+fn test_mv_same_hardlink_backup_simple_destroy() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let file_a = "test_mv_same_file_a~";
+    let file_b = "test_mv_same_file_a";
+    at.touch(file_a);
+    at.touch(file_b);
+
+    ucmd.arg(file_a)
+        .arg(file_b)
+        .arg("--b=simple")
+        .fails()
+        .stderr_contains("backing up 'test_mv_same_file_a' might destroy source");
+}
+
 #[test]
 fn test_mv_same_file_not_dot_dir() {
     let (at, mut ucmd) = at_and_ucmd!();