Pádraig Brady <[email protected]> writes: > Yes I can see the intent of disallowing -p with -C. > I.e. if there is no content,ownership,perm change > then don't update anything in the destination, including timestamps. > I.e. -C is for performance, and -p would lose some of that. > > However if the user explicitly wants the timestamps updated > there should be no problem with that. > As for performance implications, there may be some, especially for repeated > install runs, > as the updated mtime would be written to disk and the meta-data cache > would be invalidated. > But if the user wants up to date mtime then it doesn't completely invalidate > the performance obtained with -C, as the page cache shouldn't be invalidated, > and we would still be avoiding data writes.
Here is a proposed patch that allows the options to be used together. Collin
>From 2ecbca265bdb1414ecbb4de96a2de3d7ce706d2c Mon Sep 17 00:00:00 2001 Message-ID: <2ecbca265bdb1414ecbb4de96a2de3d7ce706d2c.1772409020.git.collin.fu...@gmail.com> From: Collin Funk <[email protected]> Date: Sun, 1 Mar 2026 15:31:28 -0800 Subject: [PATCH] install: allow the combination of --compare and --preserve-timestamps * NEWS: Mention the improvement. * src/install.c (change_timestamps): Move function definition. (need_copy): Preserve the timestamps if copying is skipped and --preserve-timestamps is in use. (main): Don't error when --compare and --preserve-timestamps are combined. * tests/install/install-C.sh: Add some test cases. --- NEWS | 3 +++ src/install.c | 48 ++++++++++++++++++-------------------- tests/install/install-C.sh | 27 +++++++++++++++++++-- 3 files changed, 51 insertions(+), 27 deletions(-) diff --git a/NEWS b/NEWS index 7eb70b6d1..500c50e6b 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,9 @@ GNU coreutils NEWS -*- outline -*- 'groups' and 'id' will now exit sooner after a write error, which is significant when listing information for many users. + 'install' now allows the combination of the --compare and + --preserve-timestamps options. + 'nl' now supports multi-byte --section-delimiter characters. 'shuf -i' now operates up to two times faster on systems with unlocked stdio diff --git a/src/install.c b/src/install.c index 58ecfbc31..e358231d2 100644 --- a/src/install.c +++ b/src/install.c @@ -163,6 +163,24 @@ extra_mode (mode_t input) return !! (input & ~ mask); } +/* Set the timestamps of file DEST aka DIRFD+RELNAME to match those of SRC_SB. + Return true if successful. */ + +static bool +change_timestamps (struct stat const *src_sb, char const *dest, + int dirfd, char const *relname) +{ + struct timespec timespec[2] = { get_stat_atime (src_sb), + get_stat_mtime (src_sb) }; + + if (utimensat (dirfd, relname, timespec, 0)) + { + error (0, errno, _("cannot set timestamps for %s"), quoteaf (dest)); + return false; + } + return true; +} + /* Return true if copy of file SRC_NAME to file DEST_NAME aka DEST_DIRFD+DEST_RELNAME is necessary. */ static bool @@ -246,6 +264,11 @@ need_copy (char const *src_name, char const *dest_name, bool content_match = have_same_content (src_fd, dest_fd); + /* Preserve the timestamps if we are skipping copying which would otherwise + do it for us. */ + if (content_match && x->preserve_timestamps) + change_timestamps (&src_sb, dest_name, dest_dirfd, dest_relname); + close (src_fd); close (dest_fd); return !content_match; @@ -460,24 +483,6 @@ change_attributes (char const *name, int dirfd, char const *relname) return ok; } -/* Set the timestamps of file DEST aka DIRFD+RELNAME to match those of SRC_SB. - Return true if successful. */ - -static bool -change_timestamps (struct stat const *src_sb, char const *dest, - int dirfd, char const *relname) -{ - struct timespec timespec[2] = { get_stat_atime (src_sb), - get_stat_mtime (src_sb) }; - - if (utimensat (dirfd, relname, timespec, 0)) - { - error (0, errno, _("cannot set timestamps for %s"), quoteaf (dest)); - return false; - } - return true; -} - /* Strip the symbol table from the file NAME. We could dig the magic number out of the file first to determine whether to strip it, but the header files and @@ -1051,13 +1056,6 @@ main (int argc, char **argv) error (0, 0, _("WARNING: ignoring --strip-program option as -s option was " "not specified")); - if (copy_only_if_needed && x.preserve_timestamps) - { - error (0, 0, _("options --compare (-C) and --preserve-timestamps are " - "mutually exclusive")); - usage (EXIT_FAILURE); - } - if (copy_only_if_needed && strip_files) { error (0, 0, _("options --compare (-C) and --strip are mutually " diff --git a/tests/install/install-C.sh b/tests/install/install-C.sh index 7f188c06e..67552ddd6 100755 --- a/tests/install/install-C.sh +++ b/tests/install/install-C.sh @@ -115,8 +115,31 @@ compare out out_installed_second || fail=1 ginstall -Cv -m$mode2 a b > out || fail=1 compare out out_empty || fail=1 -# options -C and --preserve-timestamps are mutually exclusive -returns_ 1 ginstall -C --preserve-timestamps a b || fail=1 +# Check -C without --preserve-timestamps with files having the same contents. +echo a > a || framework_failure_ +echo a > b || framework_failure_ +touch -d 2026-01-01 a || framework_failure_ +ginstall -C a b || fail=1 +test b -nt a || fail=1 + +# Likewise, but with --preserve-timestamps. +ginstall -C --preserve-timestamps a b || fail=1 +case $(stat --format=%y b) in + 2026-01-01*) ;; + *) fail=1 ;; +esac + +# Check -C without --preserve-timestamps with files having differing contents. +echo b > b || framework_failure_ +ginstall -C a b || fail=1 +test b -nt a || fail=1 + +# Check -C with --preserve-timestamps with files having differing contents. +ginstall -C --preserve-timestamps a b || fail=1 +case $(stat --format=%y b) in + 2026-01-01*) ;; + *) fail=1 ;; +esac # options -C and --strip are mutually exclusive returns_ 1 ginstall -C --strip --strip-program=echo a b || fail=1 -- 2.53.0
