Subject: Re: bug#70727: cp doesn't recognize the new --update=none-fail option
From: Pádraig Brady <[email-masked]>
To: Attila Fidan <[email-masked]>, [email-masked]
Date: Fri, 3 May 2024 10:26:40 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

On 03/05/2024 05:12, Attila Fidan via GNU coreutils Bug Reports wrote:
> Hi,
> 
> I wanted to use the new cp --update=none-fail option introduced in 9.5,
> but it said "invalid argument ‘none-fail’ for ‘--update’". It turns out
> that the commit (49912bac286eb3c0ef7d1567ae790193ad5eb2e8) adding it
> forgot to add the new operation to update_type[] and
> update_type_string[] in cp.c like it did for mv.c. After patching
> coreutils locally the functionality works as expected.
> 
> It seems like the test suite didn't catch this because there's no
> cp/update.sh test like there is for mv. There's a test for if using
> --backup and --update=none-fail are mutually exclusive by checking if cp
> returns 1, but an invalid argument also makes cp return 1 :)
> 
> I didn't include a patch in case a change to the test suite is wanted,
> but the proposed change is tiny and rather obvious.

Well that's embarrassing.
I implemented in cp first, tested that manually,
then must have messed up that hunk when rebasing.

The attached should fix this.

Marking this as done.

thanks,
Pádraig.

From de49e993ea8b6dcdf6cada3c0f44a6371514f952 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Fri, 3 May 2024 10:18:50 +0100
Subject: [PATCH] cp: actually support --update=none-fail

* src/cp.c: Add the entries for the --update=none-fail option.
* tests/mv/update.sh: Add a test case.
* NEWS: Mention the bug fix.
Fixes https://bugs.gnu.org/70727
---
 NEWS               |  4 ++++
 src/cp.c           |  4 ++--
 tests/mv/update.sh | 11 +++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 389f72516..7e8ccb34f 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,10 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 ** Bug fixes
 
+  cp fixes support for --update=none-fail, which would have been
+  rejected as an invalid option.
+  [bug introduced in coreutils-9.5]
+
   ls and printf fix shell quoted output in the edge case of escaped
   first and last characters, and single quotes in the string.
   [bug introduced in coreutils-8.26]
diff --git a/src/cp.c b/src/cp.c
index 28b0217db..06dbad155 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -104,11 +104,11 @@ ARGMATCH_VERIFY (reflink_type_string, reflink_type);
 
 static char const *const update_type_string[] =
 {
-  "all", "none", "older", nullptr
+  "all", "none", "none-fail", "older", nullptr
 };
 static enum Update_type const update_type[] =
 {
-  UPDATE_ALL, UPDATE_NONE, UPDATE_OLDER,
+  UPDATE_ALL, UPDATE_NONE, UPDATE_NONE_FAIL, UPDATE_OLDER,
 };
 ARGMATCH_VERIFY (update_type_string, update_type);
 
diff --git a/tests/mv/update.sh b/tests/mv/update.sh
index 164357803..39ff677b9 100755
--- a/tests/mv/update.sh
+++ b/tests/mv/update.sh
@@ -38,6 +38,17 @@ for interactive in '' -i; do
   done
 done
 
+# These should accept all options
+for update_option in '--update' '--update=older' '--update=all' \
+ '--update=none' '--update=none-fail'; do
+
+  touch file1 || framework_failure_
+  mv $update_option file1 file2 || fail=1
+  test -f file1 && fail=1
+  cp $update_option file2 file1 || fail=1
+  rm file1 file2 || framework_failure_
+done
+
 # These should perform the rename / copy
 for update_option in '--update' '--update=older' '--update=all' \
  '--update=none --update=all'; do
-- 
2.44.0