From 564dd47297f2c0f9a2e0aa8f7e5d317f84c81705 Mon Sep 17 00:00:00 2001
From: Jans Heikkinen <jansheikkinen@gmail.com>
Date: Tue, 17 Sep 2024 04:28:34 -0500
Subject: [PATCH] `ln`: allow final destination directory when using `-nf`
 (#5975)

Closes: #5974
---
 src/uu/ln/src/ln.rs      | 72 ++++++++++++++++++++--------------------
 tests/by-util/test_ln.rs | 61 ++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+), 36 deletions(-)

diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs
index a056ee256a1..a19e137e7db 100644
--- a/src/uu/ln/src/ln.rs
+++ b/src/uu/ln/src/ln.rs
@@ -301,45 +301,45 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings)
 
     let mut all_successful = true;
     for srcpath in files {
-        let targetpath =
-            if settings.no_dereference && matches!(settings.overwrite, OverwriteMode::Force) {
-                // In that case, we don't want to do link resolution
-                // We need to clean the target
-                if target_dir.is_symlink() {
-                    if target_dir.is_file() {
-                        if let Err(e) = fs::remove_file(target_dir) {
-                            show_error!("Could not update {}: {}", target_dir.quote(), e);
-                        };
-                    }
-                    if target_dir.is_dir() {
-                        // Not sure why but on Windows, the symlink can be
-                        // considered as a dir
-                        // See test_ln::test_symlink_no_deref_dir
-                        if let Err(e) = fs::remove_dir(target_dir) {
-                            show_error!("Could not update {}: {}", target_dir.quote(), e);
-                        };
+        let targetpath = if settings.no_dereference
+            && matches!(settings.overwrite, OverwriteMode::Force)
+            && target_dir.is_symlink()
+        {
+            // In that case, we don't want to do link resolution
+            // We need to clean the target
+            if target_dir.is_file() {
+                if let Err(e) = fs::remove_file(target_dir) {
+                    show_error!("Could not update {}: {}", target_dir.quote(), e);
+                };
+            }
+            if target_dir.is_dir() {
+                // Not sure why but on Windows, the symlink can be
+                // considered as a dir
+                // See test_ln::test_symlink_no_deref_dir
+                if let Err(e) = fs::remove_dir(target_dir) {
+                    show_error!("Could not update {}: {}", target_dir.quote(), e);
+                };
+            }
+            target_dir.to_path_buf()
+        } else {
+            match srcpath.as_os_str().to_str() {
+                Some(name) => {
+                    match Path::new(name).file_name() {
+                        Some(basename) => target_dir.join(basename),
+                        // This can be None only for "." or "..". Trying
+                        // to create a link with such name will fail with
+                        // EEXIST, which agrees with the behavior of GNU
+                        // coreutils.
+                        None => target_dir.join(name),
                     }
                 }
-                target_dir.to_path_buf()
-            } else {
-                match srcpath.as_os_str().to_str() {
-                    Some(name) => {
-                        match Path::new(name).file_name() {
-                            Some(basename) => target_dir.join(basename),
-                            // This can be None only for "." or "..". Trying
-                            // to create a link with such name will fail with
-                            // EEXIST, which agrees with the behavior of GNU
-                            // coreutils.
-                            None => target_dir.join(name),
-                        }
-                    }
-                    None => {
-                        show_error!("cannot stat {}: No such file or directory", srcpath.quote());
-                        all_successful = false;
-                        continue;
-                    }
+                None => {
+                    show_error!("cannot stat {}: No such file or directory", srcpath.quote());
+                    all_successful = false;
+                    continue;
                 }
-            };
+            }
+        };
 
         if linked_destinations.contains(&targetpath) {
             // If the target file was already created in this ln call, do not overwrite
diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs
index 36baef640de..f869fcc0310 100644
--- a/tests/by-util/test_ln.rs
+++ b/tests/by-util/test_ln.rs
@@ -549,6 +549,67 @@ fn test_symlink_no_deref_dir() {
     assert_eq!(at.resolve_link(link), dir1);
 }
 
+#[test]
+fn test_symlink_no_deref_file_in_destination_dir() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let file1 = "foo";
+    let file2 = "bar";
+
+    let dest = "baz";
+
+    let link1 = "baz/foo";
+    let link2 = "baz/bar";
+
+    at.touch(file1);
+    at.touch(file2);
+    at.mkdir(dest);
+
+    assert!(at.file_exists(file1));
+    assert!(at.file_exists(file2));
+    assert!(at.dir_exists(dest));
+
+    // -n and -f should work alone
+    scene
+        .ucmd()
+        .args(&["-sn", file1, dest])
+        .succeeds()
+        .no_stderr();
+    assert!(at.is_symlink(link1));
+    assert_eq!(at.resolve_link(link1), file1);
+
+    scene
+        .ucmd()
+        .args(&["-sf", file1, dest])
+        .succeeds()
+        .no_stderr();
+    assert!(at.is_symlink(link1));
+    assert_eq!(at.resolve_link(link1), file1);
+
+    // -n alone should fail if destination exists already (it should now)
+    scene.ucmd().args(&["-sn", file1, dest]).fails();
+
+    // -nf should also work
+    scene
+        .ucmd()
+        .args(&["-snf", file1, dest])
+        .succeeds()
+        .no_stderr();
+    assert!(at.is_symlink(link1));
+    assert_eq!(at.resolve_link(link1), file1);
+
+    scene
+        .ucmd()
+        .args(&["-snf", file1, file2, dest])
+        .succeeds()
+        .no_stderr();
+    assert!(at.is_symlink(link1));
+    assert_eq!(at.resolve_link(link1), file1);
+    assert!(at.is_symlink(link2));
+    assert_eq!(at.resolve_link(link2), file2);
+}
+
 #[test]
 fn test_symlink_no_deref_file() {
     let scene = TestScenario::new(util_name!());