mv: fix error when moving directory to itself (#3809)

src/uu/mv/src/mv.rs

@@ -302,20 +302,6 @@ fn exec(files: &[OsString], b: &Behavior) -> UResult<()> {
             let target_dir = paths.last().unwrap();
             let sources = &paths[..paths.len() - 1];
 
-            // Check if we have mv dir1 dir2 dir2
-            // And generate an error if this is the case
-            if sources.contains(target_dir) {
-                return Err(USimpleError::new(
-                    1,
-                    format!(
-                        "cannot move {} to a subdirectory of itself, '{}/{}'",
-                        target_dir.quote(),
-                        target_dir.display(),
-                        target_dir.display()
-                    ),
-                ));
-            }
-
             move_files_into_dir(sources, target_dir, b)
         }
     }
@@ -326,6 +312,10 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR
         return Err(MvError::NotADirectory(target_dir.quote().to_string()).into());
     }
 
+    let canonized_target_dir = target_dir
+        .canonicalize()
+        .unwrap_or_else(|_| target_dir.to_path_buf());
+
     for sourcepath in files.iter() {
         let targetpath = match sourcepath.file_name() {
             Some(name) => target_dir.join(name),
@@ -334,6 +324,29 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UR
                 continue;
             }
         };
+
+        // Check if we have mv dir1 dir2 dir2
+        // And generate an error if this is the case
+        if let Ok(canonized_source) = sourcepath.canonicalize() {
+            if canonized_source == canonized_target_dir {
+                // User tried to move directory to itself, warning is shown
+                // and process of moving files is continued.
+                show!(USimpleError::new(
+                    1,
+                    format!(
+                        "cannot move '{}' to a subdirectory of itself, '{}/{}'",
+                        sourcepath.display(),
+                        target_dir.display(),
+                        canonized_target_dir.components().last().map_or_else(
+                            || target_dir.display().to_string(),
+                            |dir| { PathBuf::from(dir.as_os_str()).display().to_string() }
+                        )
+                    )
+                ));
+                continue;
+            }
+        }
+
         show_if_err!(
             rename(sourcepath, &targetpath, b).map_err_context(|| format!(
                 "cannot move {} to {}",

tests/by-util/test_mv.rs

@@ -855,6 +855,31 @@ fn test_mv_info_self() {
         .stderr_contains("mv: cannot move 'dir2' to a subdirectory of itself, 'dir2/dir2'");
 }
 
+#[test]
+fn test_mv_into_self_data() {
+    let scene = TestScenario::new(util_name!());
+
+    let at = &scene.fixtures;
+    let sub_dir = "sub_folder";
+    let file1 = "t1.test";
+    let file2 = "sub_folder/t2.test";
+
+    let file1_result_location = "sub_folder/t1.test";
+
+    at.mkdir(sub_dir);
+    at.touch(file1);
+    at.touch(file2);
+
+    let result = scene.ucmd().arg(file1).arg(sub_dir).arg(sub_dir).run();
+
+    // sub_dir exists, file1 has been moved, file2 still exists.
+    result.code_is(1);
+
+    assert!(at.dir_exists(sub_dir));
+    assert!(at.file_exists(file1_result_location));
+    assert!(at.file_exists(file2));
+    assert!(!at.file_exists(file1));
+}
 // Todo:
 
 // $ at.touch a b