On 12/05/2024 21:31, Pádraig Brady wrote:
On 12/05/2024 16:06, Paul Eggert wrote:
On 2024-05-12 04:49, Pádraig Brady wrote:
@@ -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;
Thanks for looking into this messy area. Here is a comment from another
pair of eyes.
Could you elaborate a bit more about why these two bits of code change
x.interactive at all? That is, why doesn't update_opt simply affect
x.update? Why does update_opt bother to override a previous setting of
x.interactive to I_ALWAYS_YES, I_ALWAYS_NO, or I_ALWAYS_SKIP?
Another way to put it: shouldn't x.update simply reflect the value of
the --update option, whereas x.interactive reflects reflects whether -f,
-i, -n are used? Although this would require changes to copy.c, it'd
make the code easier to follow.
I agree that some refactoring would be good here.
At least x.update should be renamed to x.update_older.
As interactive selection, and file dates all relate
to selecting which files to update, it's tempting to conflate the settings.
However you're right that this introduces complexities when
trying to avoid all inconsistencies. Currently for example:
$ cp -v -i --update=none new old # Won't prompt as expected
$ cp -v --update=none -i new old # Unexpectedly ignores update option
So yes we should separate these things.
Another way to put it: why should, for example, --update=all override a
previous -f or (partly) -n but not a previous -i?
Right -f is significant for mv here (for completeness -f for cp is a separate
thing).
I.e. we need to treat I_ALWAYS_YES specially in mv with the current scheme.
BTW -n is not overridden by any --update option currently,
and this change effectively applies the same logic to -i now.
The attached patch set should address this.
Marking this as done.
I'll push the attached tomorrow.
cheers,
Pádraig
From 55728961377715701c1c83c7ae94e60dc404a37e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 6 Jan 2025 13:01:47 +0000
Subject: [PATCH 1/3] doc: clarify mv -f operation in texinfo
* doc/coreutils.texi (mv invocation): Be less ambiguous,
in that -f is significant for any replacement operation
on the destination, not just unlinking.
---
doc/coreutils.texi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 9afec5271..585760741 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -10282,7 +10282,7 @@ The program accepts the following options. Also see @ref{Common options}.
@opindex -f
@opindex --force
@cindex prompts, omitting
-Do not prompt the user before removing a destination file.
+Do not prompt the user before replacing a destination file.
@macro mvOptsIfn
If you specify more than one of the @option{-i}, @option{-f}, @option{-n}
options, only the final one takes effect.
--
2.47.1
From a6ab944e19684d0ccd0568fa8c06f71921c1e6c3 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 2/3] 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
---
NEWS | 3 +++
src/cp.c | 10 +++++++---
src/mv.c | 20 ++++++++++++++++----
tests/cp/cp-i.sh | 19 +++++++++++++++++++
tests/mv/i-3.sh | 17 +++++++++++++----
tests/mv/update.sh | 3 +++
6 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/NEWS b/NEWS
index 331a06358..3e0153610 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 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 215f810bd..23c25a983 100644
--- 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 46893a25f..bbf1e6034 100644
--- 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 08da4b31b..2fd5abea8 100755
--- 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 90fb7cd21..2f70e9ed8 100755
--- 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 b0b4d4acb..cc4214724 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.47.1
From 1658e470a1d7e421a1ace74352183fa91edfaa88 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Mon, 6 Jan 2025 15:48:02 +0000
Subject: [PATCH 3/3] cp,mv: uncouple --update and -f,-i,-n options
* src/copy.h: Change update member from bool to enum.
* src/copy.c: s/interactive == I_ALWAYS_NO/update == UPDATE_NONE_FAIL/;
s/interactive == I_ALWAYS_SKIP/update == UPDATE_NONE/;
s/update/update == UPDATE_OLDER/;
* src/install.c: Init with UPDATE_ALL, rather than false.
* src/cp.c: Likewise. Simply parse -f,-i,-n to x.interactive,
and parse --update to x.update.
* src/mv.c: Likewise.
* tests/cp/cp-i.sh: Add a test case where -n --update -i
honors the --update option, which would previously have been
ignored due to the preceding -n.
---
src/copy.c | 35 ++++++++++++++--------------
src/copy.h | 16 ++++++-------
src/cp.c | 50 +++++++++-------------------------------
src/install.c | 2 +-
src/mv.c | 60 ++++++++++--------------------------------------
tests/cp/cp-i.sh | 3 +++
6 files changed, 51 insertions(+), 115 deletions(-)
diff --git a/src/copy.c b/src/copy.c
index ad61de256..7ffb998f6 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -2060,8 +2060,8 @@ abandon_move (const struct cp_options *x,
struct stat const *dst_sb)
{
affirm (x->move_mode);
- return (x->interactive == I_ALWAYS_NO
- || x->interactive == I_ALWAYS_SKIP
+ return (x->update == UPDATE_NONE
+ || x->update == UPDATE_NONE_FAIL
|| ((x->interactive == I_ASK_USER
|| (x->interactive == I_UNSPECIFIED
&& x->stdin_tty
@@ -2231,7 +2231,7 @@ copy_internal (char const *src_name, char const *dst_name,
if (rename_errno == 0
? !x->last_file
: rename_errno != EEXIST
- || (x->interactive != I_ALWAYS_NO && x->interactive != I_ALWAYS_SKIP))
+ || (x->update != UPDATE_NONE && x->update != UPDATE_NONE_FAIL))
{
char const *name = rename_errno == 0 ? dst_name : src_name;
int dirfd = rename_errno == 0 ? dst_dirfd : AT_FDCWD;
@@ -2293,14 +2293,12 @@ copy_internal (char const *src_name, char const *dst_name,
{
/* Normally, fill in DST_SB or set NEW_DST so that later code
can use DST_SB if NEW_DST is false. However, don't bother
- doing this when rename_errno == EEXIST and X->interactive is
- I_ALWAYS_NO or I_ALWAYS_SKIP, something that can happen only
- with mv in which case x->update must be false which means
- that even if !NEW_DST the move will be abandoned without
- looking at DST_SB. */
+ doing this when rename_errno == EEXIST and not updating,
+ which means that even if !NEW_DST the move will be abandoned
+ without looking at DST_SB. */
if (! (rename_errno == EEXIST
- && (x->interactive == I_ALWAYS_NO
- || x->interactive == I_ALWAYS_SKIP)))
+ && (x->update == UPDATE_NONE
+ || x->update == UPDATE_NONE_FAIL)))
{
/* Regular files can be created by writing through symbolic
links, but other files cannot. So use stat on the
@@ -2345,7 +2343,7 @@ copy_internal (char const *src_name, char const *dst_name,
bool return_val = true;
bool skipped = false;
- if ((x->interactive != I_ALWAYS_NO && x->interactive != I_ALWAYS_SKIP)
+ if ((x->update != UPDATE_NONE && x->update != UPDATE_NONE_FAIL)
&& ! same_file_ok (src_name, &src_sb, dst_dirfd, drelname,
&dst_sb, x, &return_now))
{
@@ -2354,7 +2352,7 @@ copy_internal (char const *src_name, char const *dst_name,
return false;
}
- if (x->update && !S_ISDIR (src_mode))
+ if (x->update == UPDATE_OLDER && !S_ISDIR (src_mode))
{
/* When preserving timestamps (but not moving within a file
system), don't worry if the destination timestamp is
@@ -2418,27 +2416,27 @@ copy_internal (char const *src_name, char const *dst_name,
*rename_succeeded = true;
skipped = true;
- return_val = x->interactive == I_ALWAYS_SKIP;
+ return_val = x->update == UPDATE_NONE;
}
}
else
{
if (! S_ISDIR (src_mode)
- && (x->interactive == I_ALWAYS_NO
- || x->interactive == I_ALWAYS_SKIP
+ && (x->update == UPDATE_NONE
+ || x->update == UPDATE_NONE_FAIL
|| (x->interactive == I_ASK_USER
&& ! overwrite_ok (x, dst_name, dst_dirfd,
dst_relname, &dst_sb))))
{
skipped = true;
- return_val = x->interactive == I_ALWAYS_SKIP;
+ return_val = x->update == UPDATE_NONE;
}
}
skip:
if (skipped)
{
- if (x->interactive == I_ALWAYS_NO)
+ if (x->update == UPDATE_NONE_FAIL)
error (0, 0, _("not replacing %s"), quoteaf (dst_name));
else if (x->debug)
printf (_("skipped %s\n"), quoteaf (dst_name));
@@ -3110,7 +3108,8 @@ skip:
int symlink_err = force_symlinkat (src_link_val, dst_dirfd, dst_relname,
x->unlink_dest_after_failed_open, -1);
- if (0 < symlink_err && x->update && !new_dst && S_ISLNK (dst_sb.st_mode)
+ if (0 < symlink_err && x->update == UPDATE_OLDER
+ && !new_dst && S_ISLNK (dst_sb.st_mode)
&& dst_sb.st_size == strlen (src_link_val))
{
/* See if the destination is already the desired symlink.
diff --git a/src/copy.h b/src/copy.h
index 9da99826a..c619c8ea6 100644
--- a/src/copy.h
+++ b/src/copy.h
@@ -63,7 +63,7 @@ enum Update_type
/* Always update.. */
UPDATE_ALL,
- /* Update if dest older. */
+ /* Update if (nondirectory) dest older mtime. */
UPDATE_OLDER,
/* Leave existing files. */
@@ -76,11 +76,10 @@ enum Update_type
/* This type is used to help mv (via copy.c) distinguish these cases. */
enum Interactive
{
- I_ALWAYS_YES = 1,
- I_ALWAYS_NO, /* Skip and fail. */
- I_ALWAYS_SKIP, /* Skip and ignore. */
- I_ASK_USER,
- I_UNSPECIFIED
+ I_UNSPECIFIED,
+ I_ALWAYS_YES, /* -f. */
+ I_ALWAYS_SKIP, /* -n (Skip and ignore). */
+ I_ASK_USER, /* -i. */
};
/* How to handle symbolic links. */
@@ -256,9 +255,8 @@ struct cp_options
Create destination directories as usual. */
bool symbolic_link;
- /* If true, do not copy a nondirectory that has an existing destination
- with the same or newer modification time. */
- bool update;
+ /* Control if destination files are replaced. */
+ enum Update_type update;
/* If true, display the names of the files before copying them. */
bool verbose;
diff --git a/src/cp.c b/src/cp.c
index 23c25a983..a0ec06714 100644
--- a/src/cp.c
+++ b/src/cp.c
@@ -863,7 +863,7 @@ cp_option_init (struct cp_options *x)
/* Not used. */
x->stdin_tty = false;
- x->update = false;
+ x->update = UPDATE_ALL;
x->verbose = false;
x->keep_directory_symlink = false;
@@ -984,7 +984,6 @@ main (int argc, char **argv)
char *target_directory = nullptr;
bool no_target_directory = false;
char const *scontext = nullptr;
- bool no_clobber = false;
initialize_main (&argc, &argv);
set_program_name (argv[0]);
@@ -1063,9 +1062,7 @@ main (int argc, char **argv)
break;
case 'i':
- /* -i overrides -n, but not --update={none,none-fail}. */
- if (no_clobber || x.interactive == I_UNSPECIFIED)
- x.interactive = I_ASK_USER;
+ x.interactive = I_ASK_USER;
break;
case 'l':
@@ -1078,8 +1075,6 @@ main (int argc, char **argv)
case 'n':
x.interactive = I_ALWAYS_SKIP;
- no_clobber = true;
- x.update = false;
break;
case 'P':
@@ -1143,36 +1138,10 @@ main (int argc, char **argv)
break;
case 'u':
- if (! no_clobber) /* -n > -u */
- {
- enum Update_type update_opt = UPDATE_OLDER;
- if (optarg)
- update_opt = XARGMATCH ("--update", optarg,
- update_type_string, update_type);
- if (update_opt == UPDATE_ALL)
- {
- /* Default cp operation. */
- x.update = false;
- if (x.interactive != I_ASK_USER)
- x.interactive = I_UNSPECIFIED;
- }
- else if (update_opt == UPDATE_NONE)
- {
- x.update = false;
- x.interactive = I_ALWAYS_SKIP;
- }
- else if (update_opt == UPDATE_NONE_FAIL)
- {
- x.update = false;
- x.interactive = I_ALWAYS_NO;
- }
- else if (update_opt == UPDATE_OLDER)
- {
- x.update = true;
- if (x.interactive != I_ASK_USER)
- x.interactive = I_UNSPECIFIED;
- }
- }
+ x.update = UPDATE_OLDER;
+ if (optarg)
+ x.update = XARGMATCH ("--update", optarg,
+ update_type_string, update_type);
break;
case 'v':
@@ -1236,9 +1205,12 @@ main (int argc, char **argv)
usage (EXIT_FAILURE);
}
+ if (x.interactive == I_ALWAYS_SKIP)
+ x.update = UPDATE_NONE;
+
if (make_backups
- && (x.interactive == I_ALWAYS_SKIP
- || x.interactive == I_ALWAYS_NO))
+ && (x.update == UPDATE_NONE
+ || x.update == UPDATE_NONE_FAIL))
{
error (0, 0,
_("--backup is mutually exclusive with -n or --update=none-fail"));
diff --git a/src/install.c b/src/install.c
index 5bbf6d5af..b3b26abdb 100644
--- a/src/install.c
+++ b/src/install.c
@@ -290,7 +290,7 @@ cp_option_init (struct cp_options *x)
x->stdin_tty = false;
x->open_dangling_dest_symlink = false;
- x->update = false;
+ x->update = UPDATE_ALL;
x->require_preserve_context = false; /* Not used by install currently. */
x->preserve_security_context = false; /* Whether to copy context from src. */
x->set_security_context = nullptr; /* Whether to set sys default context. */
diff --git a/src/mv.c b/src/mv.c
index bbf1e6034..cf1ac56e8 100644
--- a/src/mv.c
+++ b/src/mv.c
@@ -154,7 +154,7 @@ cp_option_init (struct cp_options *x)
x->stdin_tty = isatty (STDIN_FILENO);
x->open_dangling_dest_symlink = false;
- x->update = false;
+ x->update = UPDATE_ALL;
x->verbose = false;
x->dest_info = nullptr;
x->src_info = nullptr;
@@ -327,7 +327,6 @@ main (int argc, char **argv)
int n_files;
char **file;
bool selinux_enabled = (0 < is_selinux_enabled ());
- bool no_clobber = false;
initialize_main (&argc, &argv);
set_program_name (argv[0]);
@@ -353,23 +352,13 @@ main (int argc, char **argv)
version_control_string = optarg;
break;
case 'f':
- /* -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;
+ x.interactive = I_ALWAYS_YES;
break;
case 'i':
- /* -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;
+ x.interactive = I_ASK_USER;
break;
case 'n':
x.interactive = I_ALWAYS_SKIP;
- no_clobber = true;
- x.update = false;
break;
case DEBUG_OPTION:
x.debug = x.verbose = true;
@@ -392,38 +381,10 @@ main (int argc, char **argv)
no_target_directory = true;
break;
case 'u':
- if (! no_clobber)
- {
- enum Update_type update_opt = UPDATE_OLDER;
- if (optarg)
- update_opt = XARGMATCH ("--update", optarg,
- update_type_string, update_type);
- if (update_opt == UPDATE_ALL)
- {
- /* Default mv operation. */
- x.update = false;
- if (x.interactive != I_ASK_USER
- && x.interactive != I_ALWAYS_YES)
- x.interactive = I_UNSPECIFIED;
- }
- else if (update_opt == UPDATE_NONE)
- {
- x.update = false;
- x.interactive = I_ALWAYS_SKIP;
- }
- else if (update_opt == UPDATE_NONE_FAIL)
- {
- x.update = false;
- x.interactive = I_ALWAYS_NO;
- }
- else if (update_opt == UPDATE_OLDER)
- {
- x.update = true;
- if (x.interactive != I_ASK_USER
- && x.interactive != I_ALWAYS_YES)
- x.interactive = I_UNSPECIFIED;
- }
- }
+ x.update = UPDATE_OLDER;
+ if (optarg)
+ x.update = XARGMATCH ("--update", optarg,
+ update_type_string, update_type);
break;
case 'v':
x.verbose = true;
@@ -533,10 +494,13 @@ main (int argc, char **argv)
for (int i = 0; i < n_files; i++)
strip_trailing_slashes (file[i]);
+ if (x.interactive == I_ALWAYS_SKIP)
+ x.update = UPDATE_NONE;
+
if (make_backups
&& (x.exchange
- || x.interactive == I_ALWAYS_SKIP
- || x.interactive == I_ALWAYS_NO))
+ || x.update == UPDATE_NONE
+ || x.update == UPDATE_NONE_FAIL))
{
error (0, 0,
_("cannot combine --backup with "
diff --git a/tests/cp/cp-i.sh b/tests/cp/cp-i.sh
index 2fd5abea8..2d673a2b0 100755
--- a/tests/cp/cp-i.sh
+++ b/tests/cp/cp-i.sh
@@ -88,5 +88,8 @@ 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
+# Likewise, but coreutils 9.3 - 9.5 incorrectly ignored the update option
+cp -v -n --update=none -i new old 2>/dev/null >out8 </dev/null || fail=1
+compare /dev/null out8 || fail=1
Exit $fail
--
2.47.1
