On 12/05/2024 00:03, Robert Hill wrote:
After upgrading coreutils from 9.0 to 9.5, the following change occurred:

In coreutils 9.0, the command "cp -Tipruvx /src-dir /dst-dir" requested
interactive confirmation before replacing an old destination file with a
newer source file, as expected.

In coreutils 9.5, the command "cp -Tipruvx /src-dir /dst-dir" no longer
requests interactive confirmation, but just goes ahead and replaces old
destination files with newer source files, which is not expected.

Thank you in advance for looking at this, Bob.

Right.

The thinking was for 9.3 that the new long form --update={older,all} options
would override a previous -i, especially as -i was commonly set in root
users' cp and mv aliases on Red Hat flavored distros.
Then in 9.5 we expanded this so -u behaved the same as --update=older.

In retrospect, users can avoid these aliases in various ways,
and the protective -i option should really combine with -u
rather than being overridden by it.

For completeness, -i following -u would always reinstate the protection.

The attached changes the behavior back to that -i is never overridden.

thanks,
Pádraig
From 2ec640a3d2719ac0204d4976baabe6082b85e502 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sun, 12 May 2024 12:21:19 +0100
Subject: [PATCH] cp,mv: ensure -i is not overridden by -u

Have -i combine with -u instead.
In coreutils 9.3 we had --update={all,older} override -i
In coreutils 9.5 this was expanded to -u
(to make it consistent with --update=older).

* NEWS: Mention the bug fix.
* src/cp.c (main): Don't have -u disable prompting.
* src/mv.c (main): Likewise.
* tests/cp/cp-i.sh: Add a test case.
* tests/mv/update.sh: Likewise.
Fixes https://bugs.gnu.org/70887
---
 NEWS               |  3 +++
 src/cp.c           |  6 ++++--
 src/mv.c           |  6 ++++--
 tests/cp/cp-i.sh   | 13 +++++++++++++
 tests/mv/update.sh |  3 +++
 5 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/NEWS b/NEWS
index febb9ac68..430c2ec54 100644
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,9 @@ GNU coreutils NEWS                                    -*- outline -*-
   rejected as an invalid option.
   [bug introduced in coreutils-9.5]
 
+  cp,mv --update no longer overrides --interactive.
+  [bug introduced in coreutils-9.3]
+
   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 06dbad155..cae4563cb 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -1151,7 +1151,8 @@ main (int argc, char **argv)
                 {
                   /* Default cp operation.  */
                   x.update = false;
-                  x.interactive = I_UNSPECIFIED;
+                  if (x.interactive != I_ASK_USER)
+                    x.interactive = I_UNSPECIFIED;
                 }
               else if (update_opt == UPDATE_NONE)
                 {
@@ -1166,7 +1167,8 @@ main (int argc, char **argv)
               else if (update_opt == UPDATE_OLDER)
                 {
                   x.update = true;
-                  x.interactive = I_UNSPECIFIED;
+                  if (x.interactive != I_ASK_USER)
+                    x.interactive = I_UNSPECIFIED;
                 }
             }
           break;
diff --git a/src/mv.c b/src/mv.c
index 692943a70..0162cd728 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -394,7 +394,8 @@ main (int argc, char **argv)
                 {
                   /* Default mv operation.  */
                   x.update = false;
-                  x.interactive = I_UNSPECIFIED;
+                  if (x.interactive != I_ASK_USER)
+                    x.interactive = I_UNSPECIFIED;
                 }
               else if (update_opt == UPDATE_NONE)
                 {
@@ -409,7 +410,8 @@ main (int argc, char **argv)
               else if (update_opt == UPDATE_OLDER)
                 {
                   x.update = true;
-                  x.interactive = I_UNSPECIFIED;
+                  if (x.interactive != I_ASK_USER)
+                    x.interactive = I_UNSPECIFIED;
                 }
             }
           break;
diff --git a/tests/cp/cp-i.sh b/tests/cp/cp-i.sh
index f99f743dc..28969f9c8 100755
--- a/tests/cp/cp-i.sh
+++ b/tests/cp/cp-i.sh
@@ -70,4 +70,17 @@ returns_ 1 cp -bn c d 2>/dev/null || fail=1
 returns_ 1 cp -b --update=none c d 2>/dev/null || fail=1
 returns_ 1 cp -b --update=none-fail c d 2>/dev/null || fail=1
 
+# Verify -i combines with -u,
+echo old > old || framework_failure_
+touch -d yesterday old || framework_failure_
+echo new > new || framework_failure_
+# coreutils 9.3 had --update={all,older} ignore -i
+echo n | returns_ 1 cp -vi --update=older new old 2>/dev/null >out8 || fail=1
+compare /dev/null out8 || fail=1
+echo n | returns_ 1 cp -vi --update=all new old 2>/dev/null >out8 || fail=1
+compare /dev/null out8 || fail=1
+# coreutils 9.5 also had -u ignore -i
+echo n | returns_ 1 cp -vi -u new old 2>/dev/null >out8 || fail=1
+compare /dev/null out8 || fail=1
+
 Exit $fail
diff --git a/tests/mv/update.sh b/tests/mv/update.sh
index 39ff677b9..ea8d8bdc6 100755
--- a/tests/mv/update.sh
+++ b/tests/mv/update.sh
@@ -38,6 +38,9 @@ for interactive in '' -i; do
   done
 done
 
+# This should prompt. coreutils 9.3-9.5 mistakenly did not
+echo n | returns_ 1 mv -vi -u new old >/dev/null 2>&1 || fail=1
+
 # These should accept all options
 for update_option in '--update' '--update=older' '--update=all' \
  '--update=none' '--update=none-fail'; do
-- 
2.44.0

Reply via email to