Pádraig Brady wrote: > Jim Meyering wrote: >> AFAIK, I am the only one who has built the latest snapshot: >> >> http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/17604 >> >> Though it's been only two days. >> >> Unless I hear of new bug reports or portability problems soon, >> expect coreutils-7.5 to be released in the next few days. > > checks pass on Fedora 11 and Ubuntu 7.10 > 2 failures on Fedora Core 5 due to copy::utimensat_if_possible() failing with: > > $ (cd tests && make check TESTS=cp/abuse VERBOSE=yes) > cp: preserving times for `c/1': No such file or directory > > $ (cd tests && make check TESTS=mv/part-symlink VERBOSE=yes) > mv: preserving times for `loc_reg': Too many levels of symbolic links
These highlighted a couple of issue I think on systems without utimensat(). 1. The symlink _target_ gets its time updated 2. If 1 fails then the process returns a failure I've fixed both in the attached patch hopefully by only doing the explicit utimensat() on symlinks, and only giving a warning if errno==ENOTSUP. Note it might be cleaner to add a O_NOFOLLOW flag to utimens() in gnulib, but that would be more invasive. I also tweaked the tests not to fail on systems without utimensat(). cheers, Pádraig.
>From d337e3c7c081e1262ea13fbe239938c451b0a93d Mon Sep 17 00:00:00 2001 From: =?utf-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com> Date: Thu, 13 Aug 2009 10:39:10 +0100 Subject: [PATCH] cp, mv: fix issues with preserving timestamps of copied symlinks * src/copy.c (copy_internal): On systems without utimensat() don't revert to utimens as that will update the timestamp of the symlink target. Also just warn on systems without utimensat(), don't fail. * tests/cp/abuse: To work around a failure on systems without utimensat(), use cp -dR rather than cp -a. * tests/mv/part-symlink: Skip the test on systems without utimensat(), as mv -b will warn there, messing up the test. --- src/copy.c | 20 +++++++++++--------- tests/cp/abuse | 2 +- tests/mv/part-symlink | 6 ++++++ 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/copy.c b/src/copy.c index bed90c4..068eef7 100644 --- a/src/copy.c +++ b/src/copy.c @@ -118,18 +118,17 @@ static bool owner_failure_ok (struct cp_options const *x); static char const *top_level_src_name; static char const *top_level_dst_name; -/* Wrap utimensat-with-AT_FDCWD and utimens, to keep these - cpp directives out of the main code. */ +/* Try to update the timestamp of a symlink. + We don't revert to utimens as that will follow the link. */ static inline int -utimensat_if_possible (char const *file, struct timespec const *timespec) +utimens_symlink (char const *file, struct timespec const *timespec) { - return #if HAVE_UTIMENSAT - utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW) + return utimensat (AT_FDCWD, file, timespec, AT_SYMLINK_NOFOLLOW); #else - utimens (file, timespec) + errno = ENOTSUP; + return -1; #endif - ; } /* Perform the O(1) btrfs clone operation, if possible. @@ -2117,10 +2116,13 @@ copy_internal (char const *src_name, char const *dst_name, timespec[0] = get_stat_atime (&src_sb); timespec[1] = get_stat_mtime (&src_sb); - if (utimensat_if_possible (dst_name, timespec) != 0) + if ((dest_is_symlink + ? utimens_symlink (dst_name, timespec) + : utimens (dst_name, timespec)) + != 0) { error (0, errno, _("preserving times for %s"), quote (dst_name)); - if (x->require_preserve) + if (x->require_preserve && ! (dest_is_symlink && errno == ENOTSUP)) return false; } } diff --git a/tests/cp/abuse b/tests/cp/abuse index 285c531..e9086b8 100755 --- a/tests/cp/abuse +++ b/tests/cp/abuse @@ -37,7 +37,7 @@ for i in dangling-dest existing-dest; do test $i = existing-dest && echo i > t test $i = dangling-dest && rm -f t - cp -a a/1 b/1 c 2> out && fail=1 + cp -dR a/1 b/1 c 2> out && fail=1 compare out exp || fail=1 diff --git a/tests/mv/part-symlink b/tests/mv/part-symlink index 78da70a..576bb40 100755 --- a/tests/mv/part-symlink +++ b/tests/mv/part-symlink @@ -27,6 +27,12 @@ fi cleanup_() { rm -rf "$other_partition_tmpdir"; } . "$abs_srcdir/other-fs-tmpdir" +# without utimensat() mv -b will give warnings about not +# being able to preserve the timestamps of symlinks. +# So just skip the test on older systems like this. +grep '^#define HAVE_UTIMENSAT' "$CONFIG_HEADER" > /dev/null || + skip_test_ 'system will warn about preserving timestamps of symlinks' + pwd_tmp=`pwd` # Unset CDPATH. Otherwise, output from the `cd dir' command -- 1.6.2.5