From 4eb1e847e984f6293e0d919109a1f5cdd16b2f7a Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Wed, 31 May 2023 23:53:21 +0200
Subject: [PATCH] cp --no-clobber should fail

---
 src/uu/cp/src/cp.rs      | 15 ++++++---------
 tests/by-util/test_cp.rs |  6 +++---
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs
index 513fb8380..e9fb56ef3 100644
--- a/src/uu/cp/src/cp.rs
+++ b/src/uu/cp/src/cp.rs
@@ -1102,23 +1102,21 @@ fn preserve_hardlinks(
 }
 
 /// When handling errors, we don't always want to show them to the user. This function handles that.
-/// If the error is printed, returns true, false otherwise.
-fn show_error_if_needed(error: &Error) -> bool {
+fn show_error_if_needed(error: &Error) {
     match error {
         // When using --no-clobber, we don't want to show
         // an error message
-        Error::NotAllFilesCopied => (),
+        Error::NotAllFilesCopied => {
+            // Need to return an error code
+        }
         Error::Skipped => {
             // touch a b && echo "n"|cp -i a b && echo $?
             // should return an error from GNU 9.2
-            return true;
         }
         _ => {
             show_error!("{}", error);
-            return true;
         }
     }
-    false
 }
 
 /// Copy all `sources` to `target`.  Returns an
@@ -1175,9 +1173,8 @@ fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResu
                     options,
                     &mut symlinked_files,
                 ) {
-                    if show_error_if_needed(&error) {
-                        non_fatal_errors = true;
-                    }
+                    show_error_if_needed(&error);
+                    non_fatal_errors = true;
                 }
             }
             seen_sources.insert(source);
diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs
index 1feeb0ede..690aa8b4e 100644
--- a/tests/by-util/test_cp.rs
+++ b/tests/by-util/test_cp.rs
@@ -487,7 +487,7 @@ fn test_cp_arg_no_clobber() {
     ucmd.arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HOW_ARE_YOU_SOURCE)
         .arg("--no-clobber")
-        .succeeds();
+        .fails();
 
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "How are you?\n");
 }
@@ -498,7 +498,7 @@ fn test_cp_arg_no_clobber_inferred_arg() {
     ucmd.arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HOW_ARE_YOU_SOURCE)
         .arg("--no-clob")
-        .succeeds();
+        .fails();
 
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "How are you?\n");
 }
@@ -525,7 +525,7 @@ fn test_cp_arg_no_clobber_twice() {
         .arg("--no-clobber")
         .arg("source.txt")
         .arg("dest.txt")
-        .succeeds();
+        .fails();
 
     assert_eq!(at.read("source.txt"), "some-content");
     // Should be empty as the "no-clobber" should keep