[PATCH] cp: preserve time stamps on symlinks, too

[Jim Meyering]

This patch may not be ready for prime time.
I wrote it and the test a few days ago, but haven't
gone back and looked closely.  There are just too many
branches and use cases, and I'm sure I haven't considered
all of them.  However, all tests do pass, at least on
recent gnu/linux systems.

If anyone feels like giving this a try or reviewing it,
I'd appreciate any feedback.

>From c4257ab9ef85e5d44bd39b6508c35b849abd5bd4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 27 Jul 2009 17:08:02 +0200
Subject: [PATCH] cp -pP (and e.g., -a): preserve time stamps on symlinks, too

* src/copy.c (copy_internal): Remove variable, "preserve_metadata".
Replace with "dest_is_symlink".  That covers all cases but one:
the one in which cp --link has created hard links to non-directories.
In that case, there is no need to update attributes of the links.
Use utimensat+AT_SYMLINK_NOFOLLOW so that we can preserve timestamps
of symlinks, too.
* NEWS (New features): Mention this.
* tests/Makefile.am (TESTS): Add cp/preserve-slink-time.
* tests/cp/preserve-slink-time: New file.
http://bugzilla.redhat.com/230866
---
 NEWS                         |    2 ++
 src/copy.c                   |   28 ++++++++++++----------------
 tests/Makefile.am            |    1 +
 tests/cp/preserve-slink-time |   42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 57 insertions(+), 16 deletions(-)
 create mode 100755 tests/cp/preserve-slink-time

diff --git a/NEWS b/NEWS
index 80c60e2..8ae8f01 100644
--- a/NEWS
+++ b/NEWS
@@ -35,6 +35,8 @@ GNU coreutils NEWS                                    -*- 
outline -*-

   chroot now accepts the options --userspec and --groups.

+  cp now preserves time stamps on symbolic links, when possible
+
   cp, install, mv: take advantage of btrfs' O(1) copy-on-write feature
   when both the source and destination are on the same btrfs partition.

diff --git a/src/copy.c b/src/copy.c
index bbed336..8985883 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1211,7 +1211,7 @@ copy_internal (char const *src_name, char const *dst_name,
   bool backup_succeeded = false;
   bool delayed_ok;
   bool copied_as_regular = false;
-  bool preserve_metadata;
+  bool dest_is_symlink = false;
   bool have_dst_lstat = false;

   if (x->move_mode && rename_succeeded)
@@ -1810,12 +1810,6 @@ copy_internal (char const *src_name, char const 
*dst_name,
        }
     }

-  /* In certain modes (cp's --symbolic-link), and for certain file types
-     (symlinks and hard links) it doesn't make sense to preserve metadata,
-     or it's possible to preserve only some of it.
-     In such cases, set this variable to zero.  */
-  preserve_metadata = true;
-
   if (S_ISDIR (src_mode))
     {
       struct dir_list *dir;
@@ -1909,8 +1903,7 @@ copy_internal (char const *src_name, char const *dst_name,
     }
   else if (x->symbolic_link)
     {
-      preserve_metadata = false;
-
+      dest_is_symlink = true;
       if (*src_name != '/')
        {
          /* Check that DST_NAME denotes a file in the current directory.  */
@@ -1962,7 +1955,6 @@ copy_internal (char const *src_name, char const *dst_name,
 #endif
           )
     {
-      preserve_metadata = false;
       if (link (src_name, dst_name))
        {
          error (0, errno, _("cannot create link %s"), quote (dst_name));
@@ -2007,6 +1999,7 @@ copy_internal (char const *src_name, char const *dst_name,
   else if (S_ISLNK (src_mode))
     {
       char *src_link_val = areadlink_with_size (src_name, src_sb.st_size);
+      dest_is_symlink = true;
       if (src_link_val == NULL)
        {
          error (0, errno, _("cannot read symbolic link %s"), quote (src_name));
@@ -2045,9 +2038,6 @@ copy_internal (char const *src_name, char const *dst_name,
       if (x->preserve_security_context)
        restore_default_fscreatecon_or_die ();

-      /* There's no need to preserve timestamps or permissions.  */
-      preserve_metadata = false;
-
       if (x->preserve_ownership)
        {
          /* Preserve the owner and group of the just-`copied'
@@ -2084,8 +2074,10 @@ copy_internal (char const *src_name, char const 
*dst_name,
        record_file (x->dest_info, dst_name, &sb);
     }

-  if ( ! preserve_metadata)
-    return true;
+  /* If we've just created a hard-link due to cp's --link option,
+     we're done.  */
+  if (x->hard_link && ! S_ISDIR (src_mode))
+    return delayed_ok;

   if (copied_as_regular)
     return delayed_ok;
@@ -2108,7 +2100,7 @@ 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 (utimens (dst_name, timespec) != 0)
+      if (utimensat (AT_FDCWD, dst_name, timespec, AT_SYMLINK_NOFOLLOW) != 0)
        {
          error (0, errno, _("preserving times for %s"), quote (dst_name));
          if (x->require_preserve)
@@ -2116,6 +2108,10 @@ copy_internal (char const *src_name, char const 
*dst_name,
        }
     }

+  /* The operations beyond this point may dereference a symlink.  */
+  if (dest_is_symlink)
+    return delayed_ok;
+
   /* Avoid calling chown if we know it's not necessary.  */
   if (x->preserve_ownership
       && (new_dst || !SAME_OWNER_AND_GROUP (src_sb, dst_sb)))
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 59737a0..ad19d02 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -287,6 +287,7 @@ TESTS =                                             \
   cp/parent-perm-race                          \
   cp/perm                                      \
   cp/preserve-2                                        \
+  cp/preserve-slink-time                       \
   cp/proc-short-read                           \
   cp/proc-zero-len                             \
   cp/r-vs-symlink                              \
diff --git a/tests/cp/preserve-slink-time b/tests/cp/preserve-slink-time
new file mode 100755
index 0000000..7503e8b
--- /dev/null
+++ b/tests/cp/preserve-slink-time
@@ -0,0 +1,42 @@
+#!/bin/sh
+# Verify that cp -Pp preserves times even on symlinks.
+
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  cp --version
+fi
+
+. $srcdir/test-lib.sh
+require_openat_support_
+
+ln -s no-such dangle || framework_failure
+
+# If the current file system lacks sub-second resolution, sleep for 2s to
+# ensure that the times on the copy are different from those of the original.
+case $(stat --format=%y dangle) in
+  ??:??:??.000000000) sleep 2;;
+esac
+
+cp -Pp dangle d2 || framework_failure
+stat --format=%y dangle > t1 || framework_failure
+stat --format=%y d2 > t2 || framework_failure
+
+fail=0
+compare t1 t2 || fail=1
+
+Exit $fail
--
1.6.4.212.g4719