cp,ln,mv: when skipping exit with nonzero status

* NEWS, doc/coreutils.texi: Document this.
* src/copy.c (copy_internal):
* src/ln.c (do_link): Return false when skipping action due to
--interactive or --no-clobber.
* tests/cp/cp-i.sh, tests/cp/preserve-link.sh:
* tests/cp/slink-2-slink.sh, tests/mv/i-1.pl, tests/mv/i-5.sh:
* tests/mv/mv-n.sh, tests/mv/update.sh:
Adjust expectations of exit status to match revised behavior.

diff --git a/NEWS b/NEWS
index d714b8f3b674d6a3626d913dcbd2a5c629cb0b98..c2d3a42ec066cc5d1bc3371ab53bdaa138b35789 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -69,6 +69,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   'cp --reflink=always A B' no longer leaves behind a newly created
   empty file B merely because copy-on-write clones are not supported.
 
+  'cp -n' and 'mv -n' now exit with nonzero status if they skip their
+  action because the destination exists, and likewise for 'cp -i',
+  'ln -i', and 'mv -i' when the user declines.  (POSIX specifies this
+  for 'cp -i' and 'mv -i'.)
+
   cp, mv, and install again read in multiples of the reported block size,
   to support unusual devices that may have this constraint.
   [behavior inadvertently changed in coreutils-7.2]

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index c7494ba4771e4a6f5f26f8ac208854993ff8f227..da73503ed80ea67d56b54f6c184906a102201a61 100644 (file)
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8924,7 +8924,8 @@ via recursive traversal.
 @opindex -i
 @opindex --interactive
 When copying a file other than a directory, prompt whether to
-overwrite an existing destination file.  The @option{-i} option overrides
+overwrite an existing destination file, and fail if the response
+is not affirmative.  The @option{-i} option overrides
 a previous @option{-n} option.
 
 @item -l
@@ -8946,7 +8947,7 @@ a regular file in the destination tree.
 @itemx --no-clobber
 @opindex -n
 @opindex --no-clobber
-Do not overwrite an existing file; silently do nothing instead.
+Do not overwrite an existing file; silently fail instead.
 This option overrides a previous
 @option{-i} option.  This option is mutually exclusive with @option{-b} or
 @option{--backup} option.
@@ -9190,7 +9191,8 @@ results in an error message on systems that do not support symbolic links.
 @opindex --update
 @cindex newer files, copying only
 Do not copy a non-directory that has an existing destination with the
-same or newer modification timestamp.  If timestamps are being preserved,
+same or newer modification timestamp; silently fail instead.
+If timestamps are being preserved,
 the comparison is to the source timestamp truncated to the
 resolutions of the destination file system and of the system calls
 used to update timestamps; this avoids duplicate work if several
@@ -10098,8 +10100,7 @@ options, only the final one takes effect.
 @opindex --interactive
 @cindex prompts, forcing
 Prompt whether to overwrite each existing destination file, regardless
-of its permissions.
-If the response is not affirmative, the file is skipped.
+of its permissions, and fail if the response is not affirmative.
 @mvOptsIfn
 
 @item -n
@@ -10107,7 +10108,7 @@ If the response is not affirmative, the file is skipped.
 @opindex -n
 @opindex --no-clobber
 @cindex prompts, omitting
-Do not overwrite an existing file; silently do nothing instead.
+Do not overwrite an existing file; silently fail instead.
 @mvOptsIfn
 This option is mutually exclusive with @option{-b} or @option{--backup} option.
 
@@ -10123,7 +10124,7 @@ fail with a diagnostic instead of copying and then removing the file.
 @opindex --update
 @cindex newer files, moving only
 Do not move a non-directory that has an existing destination with the
-same or newer modification timestamp.
+same or newer modification timestamp; silently fail instead.
 If the move is across file system boundaries, the comparison is to the
 source timestamp truncated to the resolutions of the destination file
 system and of the system calls used to update timestamps; this avoids
@@ -10216,7 +10217,7 @@ Ignore any previous @option{--interactive} (@option{-i}) option.
 @item -i
 @opindex -i
 Prompt whether to remove each file.
-If the response is not affirmative, the file is skipped.
+If the response is not affirmative, silently skip the file without failing.
 Ignore any previous @option{--force} (@option{-f}) option.
 Equivalent to @option{--interactive=always}.
 
@@ -10249,7 +10250,7 @@ removal is requested.  Equivalent to @option{-I}.
 @item --one-file-system
 @opindex --one-file-system
 @cindex one file system, restricting @command{rm} to
-When removing a hierarchy recursively, skip any directory that is on a
+When removing a hierarchy recursively, do not remove any directory that is on a
 file system different from that of the corresponding command line argument.
 @cindex bind mount
 This option is useful when removing a build ``chroot'' hierarchy,
@@ -10260,7 +10261,7 @@ unmount @file{/home}.  Then, when you use @command{rm -rf} to remove
 your normally throw-away chroot, that command will remove everything
 under @file{/home}, too.
 Use the @option{--one-file-system} option, and it will
-warn about and skip directories on other file systems.
+diagnose and skip directories on other file systems.
 Of course, this will not save your @file{/home} if it and your
 chroot happen to be on the same file system.
 See also @option{--preserve-root=all} to protect command line arguments
@@ -10802,7 +10803,8 @@ Remove existing destination files.
 @opindex -i
 @opindex --interactive
 @cindex prompting, and @command{ln}
-Prompt whether to remove existing destination files.
+Prompt whether to remove existing destination files,
+and fail if the response is not affirmative.
 
 @item -L
 @itemx --logical

diff --git a/src/copy.c b/src/copy.c
index 99834434f125b7bce17ee92eb0fec5cec620f211..f236afd2e2a8b99bf2f69f4df5057bd4a0cc527c 100644 (file)
--- a/src/copy.c
+++ b/src/copy.c
@@ -2197,7 +2197,7 @@ copy_internal (char const *src_name, char const *dst_name,
                         }
                     }
 
-                  return true;
+                  return false;
                 }
             }
 
@@ -2216,7 +2216,7 @@ copy_internal (char const *src_name, char const *dst_name,
                      doesn't end up removing the source file.  */
                   if (rename_succeeded)
                     *rename_succeeded = true;
-                  return true;
+                  return false;
                 }
             }
           else
@@ -2226,7 +2226,7 @@ copy_internal (char const *src_name, char const *dst_name,
                       || (x->interactive == I_ASK_USER
                           && ! overwrite_ok (x, dst_name, dst_dirfd,
                                              dst_relname, &dst_sb))))
-                return true;
+                return false;
             }
 
           if (return_now)

diff --git a/src/ln.c b/src/ln.c
index 05d89990e7013f8fece3b1580ae1d6a371479c88..1c3307cac915f90081819cf66f1b3b82d5651561 100644 (file)
--- a/src/ln.c
+++ b/src/ln.c
@@ -281,7 +281,7 @@ do_link (char const *source, int destdir_fd, char const *dest_base,
                       if (!yesno ())
                         {
                           free (rel_source);
-                          return true;
+                          return false;
                         }
                     }
 

diff --git a/tests/cp/cp-i.sh b/tests/cp/cp-i.sh
index 6fbc31760942b107cdcfd7d5105a4052fd3f6f90..b137bc4a5562ab71736691f4004047c79c6fe843 100755 (executable)
--- a/tests/cp/cp-i.sh
+++ b/tests/cp/cp-i.sh
@@ -24,7 +24,7 @@ touch a/c || framework_failure_
 
 
 # coreutils 6.2 cp would neglect to prompt in this case.
-echo n | cp -iR a b 2>/dev/null || fail=1
+echo n | returns_ 1 cp -iR a b 2>/dev/null || fail=1
 
 # test miscellaneous combinations of -f -i -n parameters
 touch c d || framework_failure_
@@ -32,7 +32,7 @@ echo "'c' -> 'd'" > out_copy
 > out_empty
 
 # ask for overwrite, answer no
-echo n | cp -vi  c d 2>/dev/null > out1 || fail=1
+echo n | returns_ 1 cp -vi  c d 2>/dev/null > out1 || fail=1
 compare out1 out_empty || fail=1
 
 # ask for overwrite, answer yes
@@ -44,7 +44,7 @@ echo y | cp -vni c d 2>/dev/null > out3 || fail=1
 compare out3 out_copy  || fail=1
 
 # -n wins over -i
-echo y | cp -vin c d 2>/dev/null > out4 || fail=1
+echo y | returns_ 1 cp -vin c d 2>/dev/null > out4 || fail=1
 compare out4 out_empty || fail=1
 
 # ask for overwrite, answer yes
@@ -52,11 +52,11 @@ echo y | cp -vfi c d 2>/dev/null > out5 || fail=1
 compare out5 out_copy  || fail=1
 
 # do not ask, prevent from overwrite
-echo n | cp -vfn c d 2>/dev/null > out6 || fail=1
+echo n | returns_ 1 cp -vfn c d 2>/dev/null > out6 || fail=1
 compare out6 out_empty || fail=1
 
 # do not ask, prevent from overwrite
-echo n | cp -vnf c d 2>/dev/null > out7 || fail=1
+echo n | returns_ 1 cp -vnf c d 2>/dev/null > out7 || fail=1
 compare out7 out_empty || fail=1
 
 # options --backup and --no-clobber are mutually exclusive

diff --git a/tests/cp/preserve-link.sh b/tests/cp/preserve-link.sh
index eb83b0f2b778312c9be73107873755b0f3260d48..2adaffd44ff78f92c408770727556662d150b83a 100755 (executable)
--- a/tests/cp/preserve-link.sh
+++ b/tests/cp/preserve-link.sh
@@ -81,7 +81,7 @@ for f in f linkm; do
 
   # Copy all the hard links across.  With cp from coreutils-8.12
   # and prior, it would sometimes mistakenly copy rather than link.
-  cp -au s t || fail=1
+  returns_ 1 cp -au s t || fail=1
 
   same_inode t/s/f t/s/linkm || fail=1
   same_inode t/s/f t/s/linke || fail=1

diff --git a/tests/cp/slink-2-slink.sh b/tests/cp/slink-2-slink.sh
index 00e3feeb3178a7b21f9f02a26d12ae9f58da2818..ece8d393dd69f56bb2ab46a91af7512edf97120f 100755 (executable)
--- a/tests/cp/slink-2-slink.sh
+++ b/tests/cp/slink-2-slink.sh
@@ -26,7 +26,7 @@ ln -s file b || framework_failure_
 ln -s no-such-file c || framework_failure_
 ln -s no-such-file d || framework_failure_
 
-cp --update --no-dereference a b || fail=1
-cp --update --no-dereference c d || fail=1
+returns_ 1 cp --update --no-dereference a b || fail=1
+returns_ 1 cp --update --no-dereference c d || fail=1
 
 Exit $fail

diff --git a/tests/mv/i-1.pl b/tests/mv/i-1.pl
index ac4d746409923feb2f4420ae8d2a77b118db11fb..f2ee0e9ccd83aee2f9ddccbd963a226848620263 100755 (executable)
--- a/tests/mv/i-1.pl
+++ b/tests/mv/i-1.pl
@@ -32,7 +32,7 @@ my @Tests =
       {IN => {src => "a\n"}}, {IN => {dst => "b\n"}}, '<', {IN => "n\n"},
       {ERR => "mv: overwrite 'dst'? "},
       {POST => sub { -r 'src' or die "test $test_a failed\n"}},
-      {EXIT => 0},
+      {EXIT => 1},
      ],
     );
 

diff --git a/tests/mv/i-5.sh b/tests/mv/i-5.sh
index 66e9cf067358d79eda5661840dfcbd613a459a6f..fabb275140d5da7861b706328799a19b175be840 100755 (executable)
--- a/tests/mv/i-5.sh
+++ b/tests/mv/i-5.sh
@@ -24,6 +24,6 @@ touch b || framework_failure_
 
 
 # coreutils 6.2 mv would neglect to prompt in this case.
-echo n | mv -i a b 2>/dev/null || fail=1
+echo n | returns_ 1 mv -i a b 2>/dev/null || fail=1
 
 Exit $fail

diff --git a/tests/mv/mv-n.sh b/tests/mv/mv-n.sh
index 49db268417618075fe7bd6f6380f132ae7fe2011..fbf571368dc65fa04e29c4f2ec403640e5887d94 100755 (executable)
--- a/tests/mv/mv-n.sh
+++ b/tests/mv/mv-n.sh
@@ -27,7 +27,7 @@ echo "renamed 'a' -> 'b'" > out_move
 
 # ask for overwrite, answer no
 touch a b || framework_failure_
-echo n | mv -vi a b 2>/dev/null > out1 || fail=1
+echo n | returns_ 1 mv -vi a b 2>/dev/null > out1 || fail=1
 compare out1 out_empty || fail=1
 
 # ask for overwrite, answer yes
@@ -37,17 +37,17 @@ compare out2 out_move || fail=1
 
 # -n wins (as the last option)
 touch a b || framework_failure_
-echo y | mv -vin a b 2>/dev/null > out3 || fail=1
+echo y | returns_ 1 mv -vin a b 2>/dev/null > out3 || fail=1
 compare out3 out_empty || fail=1
 
 # -n wins (as the last option)
 touch a b || framework_failure_
-echo y | mv -vfn a b 2>/dev/null > out4 || fail=1
+echo y | returns_ 1 mv -vfn a b 2>/dev/null > out4 || fail=1
 compare out4 out_empty || fail=1
 
 # -n wins (as the last option)
 touch a b || framework_failure_
-echo y | mv -vifn a b 2>/dev/null > out5 || fail=1
+echo y | returns_ 1 mv -vifn a b 2>/dev/null > out5 || fail=1
 compare out5 out_empty || fail=1
 
 # options --backup and --no-clobber are mutually exclusive

diff --git a/tests/mv/update.sh b/tests/mv/update.sh
index d3ec6120c51174d04fd9ddd3ca88f2b5a1b72025..f71297c2ba919eb665c6a363da8a2006fd52c235 100755 (executable)
--- a/tests/mv/update.sh
+++ b/tests/mv/update.sh
@@ -29,7 +29,8 @@ for interactive in '' -i; do
     # This is a no-op, with no prompt.
     # With coreutils-6.9 and earlier, using --update with -i would
     # mistakenly elicit a prompt.
-    $cp_or_mv $interactive --update old new < /dev/null > out 2>&1 || fail=1
+    returns_ 1 $cp_or_mv $interactive --update old new </dev/null >out 2>&1 ||
+      fail=1
     compare /dev/null out || fail=1
     case "$(cat new)" in new) ;; *) fail=1 ;; esac
     case "$(cat old)" in old) ;; *) fail=1 ;; esac