From 676283ce93b8fcba81a4e2ab6088e16a86f2f7fc Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Thu, 31 Mar 2022 00:27:59 +0200
Subject: [PATCH] ln: implement fixes for tests/ln/backup-1.sh

When doing
ln b b~
ln -f --b=simple a b

First, we create a backup of b
Then, we force the override of a => b but we make sure that the backup is
done.

So, we had a bug in the ordering of the actions.
we were first removing b. Therefore, losing the capability to do a backup of this.
---
 src/uu/ln/src/ln.rs      | 28 ++++++++++++++++------------
 tests/by-util/test_ln.rs | 26 ++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs
index 4d0495e6d..378c89872 100644
--- a/src/uu/ln/src/ln.rs
+++ b/src/uu/ln/src/ln.rs
@@ -402,18 +402,6 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
     };
 
     if is_symlink(dst) || dst.exists() {
-        match settings.overwrite {
-            OverwriteMode::NoClobber => {}
-            OverwriteMode::Interactive => {
-                print!("{}: overwrite {}? ", uucore::util_name(), dst.quote());
-                if !read_yes() {
-                    return Ok(());
-                }
-                fs::remove_file(dst)?;
-            }
-            OverwriteMode::Force => fs::remove_file(dst)?,
-        };
-
         backup_path = match settings.backup {
             BackupMode::NoBackup => None,
             BackupMode::SimpleBackup => Some(simple_backup_path(dst, &settings.suffix)),
@@ -435,6 +423,22 @@ fn link(src: &Path, dst: &Path, settings: &Settings) -> UResult<()> {
         if let Some(ref p) = backup_path {
             fs::rename(dst, p)?;
         }
+        match settings.overwrite {
+            OverwriteMode::NoClobber => {}
+            OverwriteMode::Interactive => {
+                print!("{}: overwrite {}? ", uucore::util_name(), dst.quote());
+                if !read_yes() {
+                    return Ok(());
+                }
+
+                if fs::remove_file(dst).is_ok() {};
+                // In case of error, don't do anything
+            }
+            OverwriteMode::Force => {
+                if fs::remove_file(dst).is_ok() {};
+                // In case of error, don't do anything
+            }
+        };
     }
 
     if settings.symbolic {
diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs
index 9bda5d412..f54ac5f39 100644
--- a/tests/by-util/test_ln.rs
+++ b/tests/by-util/test_ln.rs
@@ -624,3 +624,29 @@ fn test_backup_same_file() {
         .fails()
         .stderr_contains("'file1' and './file1' are the same file");
 }
+
+#[test]
+fn test_backup_force() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.write("a", "a\n");
+    at.write("b", "b2\n");
+
+    scene.ucmd().args(&["-s", "b", "b~"]).succeeds().no_stderr();
+    assert!(at.file_exists("a"));
+    assert!(at.file_exists("b"));
+    assert!(at.file_exists("b~"));
+    scene
+        .ucmd()
+        .args(&["-f", "--b=simple", "a", "b"])
+        .succeeds()
+        .no_stderr();
+    assert!(at.file_exists("a"));
+    assert!(at.file_exists("b"));
+    assert!(at.file_exists("b~"));
+    assert_eq!(at.read("a"), "a\n");
+    assert_eq!(at.read("b"), "a\n");
+    // we should have the same content as b as we had time to do a backup
+    assert_eq!(at.read("b~"), "b2\n");
+}