On 06/04/2023 22:16, Bernhard Voelker wrote:
On 4/6/23 23:02, Arsen Arsenović wrote:
See discussion at https://debbugs.gnu.org/62572
oh, I've fallen quite behind with reading all threads, sorry.
Thanks for the pointer.
Have a nice day,
Berny
In that thread I did suggest a diagnostic would be appropriate here.
Thinking more about it I'm more convinced we should diagnose here
especially as we're changing long standing behavior of --no-clobber.
The attached outputs new "skipped" errors with --no-clobber,
and similar "skipped" messages with --verbose.
cheers,
Pádraig
From 61f7b3a7aefc23150165adb878956c772f805b92 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Fri, 7 Apr 2023 10:25:41 +0100
Subject: [PATCH] cp,mv: issue "skipped" messages when skipping files
* NEWS: Mention the change in behavior to issue a "skipped" error
diagnostic with -n, and the "skipped" message with -v.
* src/copy.c (copy_internal): Adjust to output the "skipped" messages
depending on -i, -n, -u.
* tests/cp/cp-i.sh: Adjust accordingly.
* tests/mv/mv-n.sh: Likewise.
---
NEWS | 7 +++++++
src/copy.c | 27 +++++++++++++++++++++++----
tests/cp/cp-i.sh | 17 ++++++++++++-----
tests/mv/mv-n.sh | 14 +++++++++++---
4 files changed, 53 insertions(+), 12 deletions(-)
diff --git a/NEWS b/NEWS
index e4ed291b4..a9f914e1b 100644
--- a/NEWS
+++ b/NEWS
@@ -27,6 +27,13 @@ GNU coreutils NEWS -*- outline -*-
wc will now diagnose if any total counts have overflowed.
[This bug was present in "the beginning".]
+** Changes in behavior
+
+ 'cp -n' and 'mv -n' now issue an error diagnostic if skipping a file,
+ to correspond with -n inducing a nonzero exit status since coreutils 9.1.
+ Similarly 'cp -v' and 'mv -v' will output a message for each file skipped
+ due to -n, -i, or -u.
+
** New features
cp and mv now support --update=none to always skip existing files
diff --git a/src/copy.c b/src/copy.c
index e7e14c150..a60b50c0e 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2333,6 +2333,8 @@ copy_internal (char const *src_name, char const *dst_name,
if (rename_errno == EEXIST)
{
bool return_now = false;
+ bool return_val = true;
+ bool skipped = false;
if ((x->interactive != I_ALWAYS_NO && x->interactive != I_ALWAYS_SKIP)
&& ! same_file_ok (src_name, &src_sb, dst_dirfd, drelname,
@@ -2385,7 +2387,8 @@ copy_internal (char const *src_name, char const *dst_name,
}
}
- return true;
+ skipped = true;
+ goto skip;
}
}
@@ -2404,7 +2407,9 @@ 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 x->interactive == I_ALWAYS_SKIP;
+
+ skipped = true;
+ return_val = x->interactive == I_ALWAYS_SKIP;
}
}
else
@@ -2415,11 +2420,25 @@ 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 x->interactive == I_ALWAYS_SKIP;
+ {
+ skipped = true;
+ return_val = x->interactive == I_ALWAYS_SKIP;
+ }
+ }
+
+skip:
+ if (skipped)
+ {
+ if (x->verbose)
+ printf (_("skipped %s\n"), quoteaf (dst_name));
+ else if (x->interactive == I_ALWAYS_NO)
+ error (0, 0, _("skipped %s"), quoteaf (dst_name));
+
+ return_now = true;
}
if (return_now)
- return true;
+ return return_val;
if (!S_ISDIR (dst_sb.st_mode))
{
diff --git a/tests/cp/cp-i.sh b/tests/cp/cp-i.sh
index b137bc4a5..6bdaa895f 100755
--- a/tests/cp/cp-i.sh
+++ b/tests/cp/cp-i.sh
@@ -28,12 +28,14 @@ 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_
-echo "'c' -> 'd'" > out_copy
-> out_empty
+echo "'c' -> 'd'" > out_copy || framework_failure_
+echo "skipped 'd'" > out_skip || framework_failure_
+echo "cp: skipped 'd'" > err_skip || framework_failure_
+touch out_empty || framework_failure_
# ask for overwrite, answer no
echo n | returns_ 1 cp -vi c d 2>/dev/null > out1 || fail=1
-compare out1 out_empty || fail=1
+compare out1 out_skip || fail=1
# ask for overwrite, answer yes
echo y | cp -vi c d 2>/dev/null > out2 || fail=1
@@ -45,6 +47,11 @@ compare out3 out_copy || fail=1
# -n wins over -i
echo y | returns_ 1 cp -vin c d 2>/dev/null > out4 || fail=1
+compare out4 out_skip || fail=1
+
+# -n wins over -i non verbose
+echo y | returns_ 1 cp -in c d 2>err4 > out4 || fail=1
+compare err4 err_skip || fail=1
compare out4 out_empty || fail=1
# ask for overwrite, answer yes
@@ -53,11 +60,11 @@ compare out5 out_copy || fail=1
# do not ask, prevent from overwrite
echo n | returns_ 1 cp -vfn c d 2>/dev/null > out6 || fail=1
-compare out6 out_empty || fail=1
+compare out6 out_skip || fail=1
# do not ask, prevent from overwrite
echo n | returns_ 1 cp -vnf c d 2>/dev/null > out7 || fail=1
-compare out7 out_empty || fail=1
+compare out7 out_skip || fail=1
# options --backup and --no-clobber are mutually exclusive
returns_ 1 cp -bn c d 2>/dev/null || fail=1
diff --git a/tests/mv/mv-n.sh b/tests/mv/mv-n.sh
index fbf571368..9823ed280 100755
--- a/tests/mv/mv-n.sh
+++ b/tests/mv/mv-n.sh
@@ -23,12 +23,14 @@ print_ver_ mv
# test miscellaneous combinations of -f -i -n parameters
touch a b || framework_failure_
echo "renamed 'a' -> 'b'" > out_move
+echo "skipped 'b'" > out_skip || framework_failure_
+echo "mv: skipped 'b'" > err_skip || framework_failure_
> out_empty
# ask for overwrite, answer no
touch a b || framework_failure_
echo n | returns_ 1 mv -vi a b 2>/dev/null > out1 || fail=1
-compare out1 out_empty || fail=1
+compare out1 out_skip || fail=1
# ask for overwrite, answer yes
touch a b || framework_failure_
@@ -38,17 +40,23 @@ compare out2 out_move || fail=1
# -n wins (as the last option)
touch a b || framework_failure_
echo y | returns_ 1 mv -vin a b 2>/dev/null > out3 || fail=1
+compare out3 out_skip || fail=1
+
+# -n wins (non verbose)
+touch a b || framework_failure_
+echo y | returns_ 1 mv -in a b 2>err3 > out3 || fail=1
compare out3 out_empty || fail=1
+compare err3 err_skip || fail=1
# -n wins (as the last option)
touch a b || framework_failure_
echo y | returns_ 1 mv -vfn a b 2>/dev/null > out4 || fail=1
-compare out4 out_empty || fail=1
+compare out4 out_skip || fail=1
# -n wins (as the last option)
touch a b || framework_failure_
echo y | returns_ 1 mv -vifn a b 2>/dev/null > out5 || fail=1
-compare out5 out_empty || fail=1
+compare out5 out_skip || fail=1
# options --backup and --no-clobber are mutually exclusive
touch a || framework_failure_
--
2.26.2
