cp,mv: ensure -i,f are not overridden by -u

Since 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).

This patch reinstates things so that -i combines with -u instead.
I.e. have -i be protective, rather than selective (like -u).

The -f option of mv is similarly adjusted in this patch,
so now --update does not override any of -f,-i,-n.

* 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 for -i.
* tests/mv/update.sh: Likewise.
* tests/mv/i-3.sh. Add a test case for -f.
Fixes https://bugs.gnu.org/70887

diff --git a/NEWS b/NEWS
index 331a06358748b63fce15e25776e5f971b070f543..3e015361023379103f15c3a843465f5b50d9f245 100644 (file)
--- 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 or --force.
+  [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 215f810bd543f2eea599ca338fcfd77cca647484..23c25a9838ac3ada42db302ea705acb3d140f0ee 100644 (file)
--- a/src/cp.c
+++ b/src/cp.c
@@ -1063,7 +1063,9 @@ main (int argc, char **argv)
           break;
 
         case 'i':
-          x.interactive = I_ASK_USER;
+          /* -i overrides -n, but not --update={none,none-fail}.  */
+          if (no_clobber || x.interactive == I_UNSPECIFIED)
+            x.interactive = I_ASK_USER;
           break;
 
         case 'l':
@@ -1151,7 +1153,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 +1169,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 46893a25f540381d20c7fbd5239447d6075f8945..bbf1e6034874d42f85ca92e5c7dd6253883a1b98 100644 (file)
--- a/src/mv.c
+++ b/src/mv.c
@@ -353,10 +353,18 @@ main (int argc, char **argv)
             version_control_string = optarg;
           break;
         case 'f':
-          x.interactive = I_ALWAYS_YES;
+          /* -f overrides -n, or -i, but not --update={none,none-fail}.  */
+          if (no_clobber
+              || x.interactive == I_ASK_USER
+              || x.interactive == I_UNSPECIFIED)
+            x.interactive = I_ALWAYS_YES;
           break;
         case 'i':
-          x.interactive = I_ASK_USER;
+          /* -i overrides -n, or -f, but not --update={none,none-fail}.  */
+          if (no_clobber
+              || x.interactive == I_ALWAYS_YES
+              || x.interactive == I_UNSPECIFIED)
+            x.interactive = I_ASK_USER;
           break;
         case 'n':
           x.interactive = I_ALWAYS_SKIP;
@@ -394,7 +402,9 @@ main (int argc, char **argv)
                 {
                   /* Default mv operation.  */
                   x.update = false;
-                  x.interactive = I_UNSPECIFIED;
+                  if (x.interactive != I_ASK_USER
+                      && x.interactive != I_ALWAYS_YES)
+                    x.interactive = I_UNSPECIFIED;
                 }
               else if (update_opt == UPDATE_NONE)
                 {
@@ -409,7 +419,9 @@ 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_ALWAYS_YES)
+                    x.interactive = I_UNSPECIFIED;
                 }
             }
           break;

diff --git a/tests/cp/cp-i.sh b/tests/cp/cp-i.sh
index 08da4b31b51de2f0aa05923a213b8194d1a3e23f..2fd5abea8fd743bcc2b52bfe65e081f662a2dd2a 100755 (executable)
--- a/tests/cp/cp-i.sh
+++ b/tests/cp/cp-i.sh
@@ -70,4 +70,23 @@ 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
+# Don't prompt as not updating
+cp -v -i --update=none new old 2>/dev/null >out8 </dev/null || fail=1
+compare /dev/null out8 || fail=1
+# Likewise, but coreutils 9.3 - 9.5 incorrectly ignored the update option
+cp -v --update=none -i new old 2>/dev/null >out8 </dev/null || fail=1
+compare /dev/null out8 || fail=1
+
 Exit $fail

diff --git a/tests/mv/i-3.sh b/tests/mv/i-3.sh
index 90fb7cd21bff4014882208e36a480e59546f5022..2f70e9ed8fc6f4dd5758a063408f35838d9e3722 100755 (executable)
--- a/tests/mv/i-3.sh
+++ b/tests/mv/i-3.sh
@@ -25,8 +25,8 @@ trap '' TTIN # Ignore SIGTTIN
 
 uname -s | grep 'BSD$' && skip_ 'known spurious failure on *BSD'
 
-touch f g h i || framework_failure_
-chmod 0 g i || framework_failure_
+touch f g h i j k || framework_failure_
+chmod 0 g i j k || framework_failure_
 
 
 ls /dev/stdin >/dev/null 2>&1 \
@@ -59,11 +59,20 @@ retry_delay_ check_overwrite_prompt .1 7 || { fail=1; cat out; }
 
 cleanup_
 
-mv -f h i > out 2>&1 || fail=1
+# Make sure there was no prompt with -f
+timeout 10 mv -f h i > out 2>&1 || fail=1
 test -f i || fail=1
 test -f h && fail=1
+case "$(cat out)" in
+  '') ;;
+  *) fail=1 ;;
+esac
 
-# Make sure there was no prompt.
+# Likewise make sure there was no prompt with -f -u
+# coreutils 9.3-9.5 mistakenly did prompt.
+timeout 10 mv -f --update=all j k > out 2>&1 || fail=1
+test -f k || fail=1
+test -f j && fail=1
 case "$(cat out)" in
   '') ;;
   *) fail=1 ;;

diff --git a/tests/mv/update.sh b/tests/mv/update.sh
index b0b4d4acb7d8ed69073f5a6ef64a4718fea145c4..cc4214724a2a11ce187c75f5abc78adfa9ef543c 100755 (executable)
--- 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