Merge pull request #3724 from niyaznigmatullin/ln_same_file_on_force

ln: symlink --force, src and dst are same file

src/uu/cp/src/cp.rs

@@ -18,7 +18,7 @@ extern crate uucore;
 
 use uucore::display::Quotable;
 use uucore::format_usage;
-use uucore::fs::FileInformation;
+use uucore::fs::{paths_refer_to_same_file, FileInformation};
 
 use std::borrow::Cow;
 
@@ -1657,17 +1657,6 @@ pub fn localize_to_target(root: &Path, source: &Path, target: &Path) -> CopyResu
     Ok(target.join(&local_to_root))
 }
 
-pub fn paths_refer_to_same_file(p1: &Path, p2: &Path, dereference: bool) -> bool {
-    // We have to take symlinks and relative paths into account.
-    let res1 = FileInformation::from_path(p1, dereference);
-    let res2 = FileInformation::from_path(p2, dereference);
-
-    match (res1, res2) {
-        (Ok(info1), Ok(info2)) => info1 == info2,
-        _ => false,
-    }
-}
-
 pub fn path_has_prefix(p1: &Path, p2: &Path) -> io::Result<bool> {
     let pathbuf1 = canonicalize(p1, MissingHandling::Normal, ResolveMode::Logical)?;
     let pathbuf2 = canonicalize(p2, MissingHandling::Normal, ResolveMode::Logical)?;

src/uu/ln/src/ln.rs

@@ -14,7 +14,7 @@ use clap::{crate_version, Arg, Command};
 use uucore::display::Quotable;
 use uucore::error::{UError, UResult};
 use uucore::format_usage;
-use uucore::fs::is_symlink;
+use uucore::fs::{is_symlink, paths_refer_to_same_file};
 
 use std::borrow::Cow;
 use std::error::Error;
@@ -435,13 +435,7 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
         };
         if settings.backup == BackupMode::ExistingBackup && !settings.symbolic {
             // when ln --backup f f, it should detect that it is the same file
-            let dst_abs = canonicalize(dst, MissingHandling::Normal, ResolveMode::Logical)?;
+            if paths_refer_to_same_file(src, dst, true) {
-            let source_abs = canonicalize(
-                source.clone(),
-                MissingHandling::Normal,
-                ResolveMode::Logical,
-            )?;
-            if dst_abs == source_abs {
                 return Err(LnError::SameFile().into());
             }
         }
@@ -460,6 +454,9 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
                 // In case of error, don't do anything
             }
             OverwriteMode::Force => {
+                if !is_symlink(dst) && paths_refer_to_same_file(src, dst, true) {
+                    return Err(LnError::SameFile().into());
+                }
                 if fs::remove_file(dst).is_ok() {};
                 // In case of error, don't do anything
             }

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

@@ -477,6 +477,17 @@ pub fn dir_strip_dot_for_creation(path: &Path) -> PathBuf {
     }
 }
 
+/// Checks if `p1` and `p2` are the same file.
+/// If error happens when trying to get files' metadata, returns false
+pub fn paths_refer_to_same_file<P: AsRef<Path>>(p1: P, p2: P, dereference: bool) -> bool {
+    if let Ok(info1) = FileInformation::from_path(p1, dereference) {
+        if let Ok(info2) = FileInformation::from_path(p2, dereference) {
+            return info1 == info2;
+        }
+    }
+    false
+}
+
 #[cfg(test)]
 mod tests {
     // Note this useful idiom: importing names from outer (for mod tests) scope.

tests/by-util/test_ln.rs

@@ -702,3 +702,16 @@ fn test_hard_logical_dir_fail() {
         .fails()
         .stderr_contains("failed to link 'link-to-dir'");
 }
+
+#[test]
+fn test_symlink_remove_existing_same_src_and_dest() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("a");
+    at.write("a", "sample");
+    ucmd.args(&["-sf", "a", "a"])
+        .fails()
+        .code_is(1)
+        .stderr_contains("Same file");
+    assert!(at.file_exists("a") && !at.symlink_exists("a"));
+    assert_eq!(at.read("a"), "sample");
+}