Re: no feedback on snapshot? coreutils-7.5 coming soon

[Pádraig Brady]

Jim Meyering wrote:
> Pádraig Brady wrote:
>> These highlighted a couple of issues 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.
> 
> Thanks for working on that.
> 
> Previously, copying with cp -a, symlink times would not be preserved
> (silently, since cp made no attempt).  It sounds like with this change,
> on systems without utimensat, cp would print a warning for each and
> every symlink.

I was wondering about printing the warning,
and was wary about now silently not preserving symlink times.
I.E. being silently inconsistent. I guess it's better
to be quiet in this case? That also means I can
reinstate the mv/part-symlink test on systems without utimensat().
Updated patch attached.

cheers,
Pádraig.

>From cfd4e430bd433ce3b38561300accad1fa56ee89f Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
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
use to utimens as that will set the timestamp of the symlink target.
* tests/cp/abuse: To work around possible attribute preservation
failures breaking the test, use cp -dR rather than cp -a.
---
 src/copy.c     |   19 +++++++++++--------
 tests/cp/abuse |    2 +-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index bed90c4..73c1949 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -118,18 +118,18 @@ 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)
+  /* Don't set errno=ENOTSUP here as we don't want
+     to output an error message for this case.  */
+  return 0;
 #endif
-    ;
 }
 
 /* Perform the O(1) btrfs clone operation, if possible.
@@ -2117,7 +2117,10 @@ 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)
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
 
-- 
1.6.2.5