Merge pull request #4831 from sylvestre/mv-hardlink

mv: 'mv source hardlink' should fail

src/uu/mv/Cargo.toml

@@ -18,7 +18,7 @@ path = "src/mv.rs"
 clap = { workspace=true }
 fs_extra = { workspace=true }
 indicatif = { workspace=true }
-uucore = { workspace=true }
+uucore = { workspace=true, features=["fs"] }
 
 [[bin]]
 name = "mv"

src/uu/mv/src/mv.rs

@@ -25,6 +25,7 @@ use std::path::{Path, PathBuf};
 use uucore::backup_control::{self, BackupMode};
 use uucore::display::Quotable;
 use uucore::error::{set_exit_code, FromIo, UError, UResult, USimpleError, UUsageError};
+use uucore::fs::are_hardlinks_to_same_file;
 use uucore::update_control::{self, UpdateMode};
 use uucore::{format_usage, help_about, help_section, help_usage, prompt_yes, show};
 
@@ -237,93 +238,90 @@ fn determine_overwrite_mode(matches: &ArgMatches) -> OverwriteMode {
     }
 }
 
-fn exec(files: &[OsString], b: &Behavior) -> UResult<()> {
-    let paths: Vec<PathBuf> = {
-        let paths = files.iter().map(Path::new);
+fn parse_paths(files: &[OsString], b: &Behavior) -> Vec<PathBuf> {
+    let paths = files.iter().map(Path::new);
 
-        // Strip slashes from path, if strip opt present
-        if b.strip_slashes {
-            paths
-                .map(|p| p.components().as_path().to_owned())
-                .collect::<Vec<PathBuf>>()
+    if b.strip_slashes {
+        paths
+            .map(|p| p.components().as_path().to_owned())
+            .collect::<Vec<PathBuf>>()
+    } else {
+        paths.map(|p| p.to_owned()).collect::<Vec<PathBuf>>()
+    }
+}
+
+fn handle_two_paths(source: &Path, target: &Path, b: &Behavior) -> UResult<()> {
+    if source.symlink_metadata().is_err() {
+        return Err(MvError::NoSuchFile(source.quote().to_string()).into());
+    }
+
+    if (source.eq(target) || are_hardlinks_to_same_file(source, target))
+        && b.backup != BackupMode::SimpleBackup
+    {
+        if source.eq(Path::new(".")) || source.ends_with("/.") || source.is_file() {
+            return Err(
+                MvError::SameFile(source.quote().to_string(), target.quote().to_string()).into(),
+            );
         } else {
-            paths.map(|p| p.to_owned()).collect::<Vec<PathBuf>>()
+            return Err(MvError::SelfSubdirectory(source.display().to_string()).into());
         }
-    };
+    }
+
+    if target.is_dir() {
+        if b.no_target_dir {
+            if source.is_dir() {
+                rename(source, target, b, None).map_err_context(|| {
+                    format!("cannot move {} to {}", source.quote(), target.quote())
+                })
+            } else {
+                Err(MvError::DirectoryToNonDirectory(target.quote().to_string()).into())
+            }
+        } else {
+            move_files_into_dir(&[source.to_path_buf()], target, b)
+        }
+    } else if target.exists() && source.is_dir() {
+        match b.overwrite {
+            OverwriteMode::NoClobber => return Ok(()),
+            OverwriteMode::Interactive => {
+                if !prompt_yes!("overwrite {}? ", target.quote()) {
+                    return Err(io::Error::new(io::ErrorKind::Other, "").into());
+                }
+            }
+            OverwriteMode::Force => {}
+        };
+        Err(MvError::NonDirectoryToDirectory(
+            source.quote().to_string(),
+            target.quote().to_string(),
+        )
+        .into())
+    } else {
+        rename(source, target, b, None).map_err(|e| USimpleError::new(1, format!("{e}")))
+    }
+}
+
+fn handle_multiple_paths(paths: &[PathBuf], b: &Behavior) -> UResult<()> {
+    if b.no_target_dir {
+        return Err(UUsageError::new(
+            1,
+            format!("mv: extra operand {}", paths[2].quote()),
+        ));
+    }
+    let target_dir = paths.last().unwrap();
+    let sources = &paths[..paths.len() - 1];
+
+    move_files_into_dir(sources, target_dir, b)
+}
+
+fn exec(files: &[OsString], b: &Behavior) -> UResult<()> {
+    let paths = parse_paths(files, b);
 
     if let Some(ref name) = b.target_dir {
         return move_files_into_dir(&paths, &PathBuf::from(name), b);
     }
+
     match paths.len() {
-        /* case 0/1 are not possible thanks to clap */
-        2 => {
-            let source = &paths[0];
-            let target = &paths[1];
-            // 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() {
-                return Err(MvError::NoSuchFile(source.quote().to_string()).into());
-            }
-
-            // GNU semantics are: if the source and target are the same, no move occurs and we print an error
-            if source.eq(target) {
-                // Done to match GNU semantics for the dot file
-                if source.eq(Path::new(".")) || source.ends_with("/.") || source.is_file() {
-                    return Err(MvError::SameFile(
-                        source.quote().to_string(),
-                        target.quote().to_string(),
-                    )
-                    .into());
-                } else {
-                    return Err(MvError::SelfSubdirectory(source.display().to_string()).into());
-                }
-            }
-
-            if target.is_dir() {
-                if b.no_target_dir {
-                    if source.is_dir() {
-                        rename(source, target, b, None).map_err_context(|| {
-                            format!("cannot move {} to {}", source.quote(), target.quote())
-                        })
-                    } else {
-                        Err(MvError::DirectoryToNonDirectory(target.quote().to_string()).into())
-                    }
-                } else {
-                    move_files_into_dir(&[source.clone()], target, b)
-                }
-            } else if target.exists() && source.is_dir() {
-                match b.overwrite {
-                    OverwriteMode::NoClobber => return Ok(()),
-                    OverwriteMode::Interactive => {
-                        if !prompt_yes!("overwrite {}? ", target.quote()) {
-                            return Err(io::Error::new(io::ErrorKind::Other, "").into());
-                        }
-                    }
-                    OverwriteMode::Force => {}
-                };
-                Err(MvError::NonDirectoryToDirectory(
-                    source.quote().to_string(),
-                    target.quote().to_string(),
-                )
-                .into())
-            } else {
-                rename(source, target, b, None).map_err(|e| USimpleError::new(1, format!("{e}")))
-            }
-        }
-        _ => {
-            if b.no_target_dir {
-                return Err(UUsageError::new(
-                    1,
-                    format!("mv: extra operand {}", files[2].quote()),
-                ));
-            }
-            let target_dir = paths.last().unwrap();
-            let sources = &paths[..paths.len() - 1];
-
-            move_files_into_dir(sources, target_dir, b)
-        }
+        2 => handle_two_paths(&paths[0], &paths[1], b),
+        _ => handle_multiple_paths(&paths, b),
     }
 }
 

src/uucore/src/lib/features/fs.rs

@@ -617,14 +617,47 @@ pub fn is_symlink_loop(path: &Path) -> bool {
     false
 }
 
+#[cfg(not(unix))]
+// Hard link comparison is not supported on non-Unix platforms
+pub fn are_hardlinks_to_same_file(_source: &Path, _target: &Path) -> bool {
+    false
+}
+
+/// Checks if two paths are hard links to the same file.
+///
+/// # Arguments
+///
+/// * `source` - A reference to a `Path` representing the source path.
+/// * `target` - A reference to a `Path` representing the target path.
+///
+/// # Returns
+///
+/// * `bool` - Returns `true` if the paths are hard links to the same file, and `false` otherwise.
+#[cfg(unix)]
+pub fn are_hardlinks_to_same_file(source: &Path, target: &Path) -> bool {
+    let source_metadata = match fs::metadata(source) {
+        Ok(metadata) => metadata,
+        Err(_) => return false,
+    };
+
+    let target_metadata = match fs::metadata(target) {
+        Ok(metadata) => metadata,
+        Err(_) => return false,
+    };
+
+    source_metadata.ino() == target_metadata.ino() && source_metadata.dev() == target_metadata.dev()
+}
+
 #[cfg(test)]
 mod tests {
     // Note this useful idiom: importing names from outer (for mod tests) scope.
     use super::*;
     #[cfg(unix)]
+    use std::io::Write;
+    #[cfg(unix)]
     use std::os::unix;
     #[cfg(unix)]
-    use tempfile::tempdir;
+    use tempfile::{tempdir, NamedTempFile};
 
     struct NormalizePathTestCase<'a> {
         path: &'a str,
@@ -769,4 +802,44 @@ mod tests {
 
         assert!(is_symlink_loop(&symlink1_path));
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_are_hardlinks_to_same_file_same_file() {
+        let mut temp_file = NamedTempFile::new().unwrap();
+        writeln!(temp_file, "Test content").unwrap();
+
+        let path1 = temp_file.path();
+        let path2 = temp_file.path();
+
+        assert_eq!(are_hardlinks_to_same_file(&path1, &path2), true);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_are_hardlinks_to_same_file_different_files() {
+        let mut temp_file1 = NamedTempFile::new().unwrap();
+        writeln!(temp_file1, "Test content 1").unwrap();
+
+        let mut temp_file2 = NamedTempFile::new().unwrap();
+        writeln!(temp_file2, "Test content 2").unwrap();
+
+        let path1 = temp_file1.path();
+        let path2 = temp_file2.path();
+
+        assert_eq!(are_hardlinks_to_same_file(&path1, &path2), false);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_are_hardlinks_to_same_file_hard_link() {
+        let mut temp_file = NamedTempFile::new().unwrap();
+        writeln!(temp_file, "Test content").unwrap();
+        let path1 = temp_file.path();
+
+        let path2 = temp_file.path().with_extension("hardlink");
+        fs::hard_link(&path1, &path2).unwrap();
+
+        assert_eq!(are_hardlinks_to_same_file(&path1, &path2), true);
+    }
 }

tests/by-util/test_mv.rs

@@ -400,6 +400,39 @@ fn test_mv_same_file() {
         .stderr_is(format!("mv: '{file_a}' and '{file_a}' are the same file\n",));
 }
 
+#[test]
+#[cfg(all(unix, not(target_os = "android")))]
+fn test_mv_same_hardlink() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let file_a = "test_mv_same_file_a";
+    let file_b = "test_mv_same_file_b";
+    at.touch(file_a);
+
+    at.hard_link(file_a, file_b);
+
+    at.touch(file_a);
+    ucmd.arg(file_a)
+        .arg(file_b)
+        .fails()
+        .stderr_is(format!("mv: '{file_a}' and '{file_b}' are the same file\n",));
+}
+
+#[test]
+#[cfg(all(unix, not(target_os = "android")))]
+fn test_mv_same_hardlink_backup_simple() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let file_a = "test_mv_same_file_a";
+    let file_b = "test_mv_same_file_b";
+    at.touch(file_a);
+
+    at.hard_link(file_a, file_b);
+
+    ucmd.arg(file_a)
+        .arg(file_b)
+        .arg("--backup=simple")
+        .succeeds();
+}
+
 #[test]
 fn test_mv_same_file_not_dot_dir() {
     let (at, mut ucmd) = at_and_ucmd!();