Re: [PATCH] cp --update --preserve

[Jim Meyering]

Paolo Montrasio <address@hidden> wrote:
> I was trying to combine the many options of cp to create a simple
> backup program when I run into an unexpected behaviour of the program. I
> was using the cp from the fileutils 1.4.2 distribution, so I downloaded
> coreutils-5.0 and compiled it and I still have the same problem. Here
> it is the test case.

[ Your message had some very long lines.
  Please wrap long lines of text so they fit in about
  80 columns (but don't wrap patches). ]

Thank you for the report and patch.
However there are two problems.

1) your patch breaks --update semantics when --update
   is used without --preserve.  You probably meant for it to
   act like the patch I've included below.

2) In any case, I'm not sure that such a change would be a good idea,
   since it does subvert the semantics of --update.  But I'm open
   to arguments either way.

Index: copy.c
===================================================================
RCS file: /fetish/cu/src/copy.c,v
retrieving revision 1.156
diff -u -p -r1.156 copy.c
--- copy.c      4 Jan 2004 21:04:27 -0000       1.156
+++ copy.c      14 Jan 2004 22:16:30 -0000
@@ -42,6 +42,11 @@
 #include "utimens.h"
 #include "xreadlink.h"
 
+/* Compare timestamps, ignoring any sub-second differences.  */
+#define ST_TIME_CMP_SEC(a, b, s) \
+  ((a).s < (b).s ? -1 : (a).s > (b).s ? 1 : 0)
+#define MTIME_CMP_SEC(a, b) ST_TIME_CMP_SEC (a, b, st_mtime)
+
 #define DO_CHOWN(Chown, File, New_uid, New_gid)                                
\
   (Chown (File, New_uid, New_gid)                                      \
    /* If non-root uses -p, it's ok if we can't preserve ownership.     \
@@ -939,17 +944,26 @@ copy_internal (const char *src_path, con
                  return 1;
                }
 
-             if (x->update && MTIME_CMP (src_sb, dst_sb) <= 0)
-               {
-                 /* We're using --update and the source file is older
-                    than the destination file, so there is no need to
-                    copy or move.  */
-                 /* Pretend the rename succeeded, so the caller (mv)
-                    doesn't end up removing the source file.  */
-                 if (rename_succeeded)
-                   *rename_succeeded = 1;
-                 return 0;
-               }
+             {
+               /* When using --update and preserving timestamps,
+                  don't use nanoseconds when comparing timestamps. */
+               bool up_to_date =
+                 (x->preserve_timestamps && x->update
+                  ? MTIME_CMP_SEC (src_sb, dst_sb) <= 0
+                  : x->update && MTIME_CMP (src_sb, dst_sb) <= 0);
+
+               if (up_to_date)
+                 {
+                   /* We're using --update and the source file is not newer
+                      than the destination file, so there is no need to
+                      copy or move.  */
+                   /* Pretend the rename succeeded, so the caller (mv)
+                      doesn't end up removing the source file.  */
+                   if (rename_succeeded)
+                     *rename_succeeded = 1;
+                   return 0;
+                 }
+             }
            }
 
          /* When there is an existing destination file, we may end up