From: Pádraig Brady Date: Mon, 6 Jan 2025 15:48:02 +0000 (+0000) Subject: cp,mv: decouple --update from -f,-i,-n options X-Git-Tag: v9.6~33 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=10406103b81b737597f9a933fc53118e10c0dc11;p=thirdparty%2Fcoreutils.git cp,mv: decouple --update from -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. --- diff --git a/src/copy.c b/src/copy.c index ad61de256b..7ffb998f6c 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 9da99826a9..35068ea5da 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 has 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 23c25a9838..a0ec067147 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 5bbf6d5afd..b3b26abdb4 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 bbf1e60348..cf1ac56e87 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 2fd5abea8f..2d673a2b09 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 >out8