Fix overzealous setting of mtime & tweak time comparisons

- Stop setting the mtime on a file we didn't transfer (or didn't verify
  the checksum) when the time diff is within the modify window.
- Stop computing a time difference (-1|0|1) when all we care about is
  time equality.

diff --git a/backup.c b/backup.c
index a173aada39c0c38739e352f750f301a5ce32131d..be406bef83003e604555d9a1158b80f76778657e 100644 (file)
--- a/backup.c
+++ b/backup.c
@@ -336,7 +336,7 @@ int make_backup(const char *fname, BOOL prefer_rename)
 
        save_preserve_xattrs = preserve_xattrs;
        preserve_xattrs = 0;
-       set_file_attrs(buf, file, NULL, fname, ATTRS_SET_NANO);
+       set_file_attrs(buf, file, NULL, fname, ATTRS_ACCURATE_TIME);
        preserve_xattrs = save_preserve_xattrs;
 
        unmake_file(file);

diff --git a/generator.c b/generator.c
index 210e5f31cb63007b805e394421aa2d1db9233987..70e1137456da50fec2d34112c773d605ecb97c68 100644 (file)
--- a/generator.c
+++ b/generator.c
@@ -59,6 +59,7 @@ extern int human_readable;
 extern int ignore_existing;
 extern int ignore_non_existing;
 extern int want_xattr_optim;
+extern int modify_window;
 extern int inplace;
 extern int append_mode;
 extern int make_backups;
@@ -100,7 +101,7 @@ extern struct file_list *cur_flist, *first_flist, *dir_flist;
 extern filter_rule_list filter_list, daemon_filter_list;
 
 int maybe_ATTRS_REPORT = 0;
-int maybe_ATTRS_SET_NANO = 0;
+int maybe_ATTRS_ACCURATE_TIME = 0;
 
 static dev_t dev_zero;
 static int deldelay_size = 0, deldelay_cnt = 0;
@@ -391,12 +392,12 @@ static void do_delete_pass(void)
                rprintf(FINFO, "                    \r");
 }
 
-static inline int time_diff(STRUCT_STAT *stp, struct file_struct *file)
+static inline int time_differs(STRUCT_STAT *stp, struct file_struct *file)
 {
 #ifdef ST_MTIME_NSEC
-       return cmp_time(stp->st_mtime, stp->ST_MTIME_NSEC, file->modtime, F_MOD_NSEC_or_0(file));
+       return !same_time(stp->st_mtime, stp->ST_MTIME_NSEC, file->modtime, F_MOD_NSEC_or_0(file));
 #else
-       return cmp_time(stp->st_mtime, 0L, file->modtime, 0L);
+       return !same_time(stp->st_mtime, 0, file->modtime, 0);
 #endif
 }
 
@@ -454,7 +455,7 @@ int unchanged_attrs(const char *fname, struct file_struct *file, stat_x *sxp)
 {
        if (S_ISLNK(file->mode)) {
 #ifdef CAN_SET_SYMLINK_TIMES
-               if (preserve_times & PRESERVE_LINK_TIMES && time_diff(&sxp->st, file))
+               if (preserve_times & PRESERVE_LINK_TIMES && time_differs(&sxp->st, file))
                        return 0;
 #endif
 #ifdef CAN_CHMOD_SYMLINK
@@ -474,7 +475,7 @@ int unchanged_attrs(const char *fname, struct file_struct *file, stat_x *sxp)
                        return 0;
 #endif
        } else {
-               if (preserve_times && time_diff(&sxp->st, file))
+               if (preserve_times && time_differs(&sxp->st, file))
                        return 0;
                if (perms_differ(file, sxp))
                        return 0;
@@ -509,12 +510,12 @@ void itemize(const char *fnamecmp, struct file_struct *file, int ndx, int statre
                        if (iflags & ITEM_LOCAL_CHANGE)
                                iflags |= symlink_timeset_failed_flags;
                } else if (keep_time
-                ? time_diff(&sxp->st, file)
+                ? time_differs(&sxp->st, file)
                 : iflags & (ITEM_TRANSFER|ITEM_LOCAL_CHANGE) && !(iflags & ITEM_MATCHED)
                  && (!(iflags & ITEM_XNAME_FOLLOWS) || *xname))
                        iflags |= ITEM_REPORT_TIME;
                if (atimes_ndx && !S_ISDIR(file->mode) && !S_ISLNK(file->mode)
-                && cmp_time(F_ATIME(file), 0, sxp->st.st_atime, 0) != 0)
+                && !same_time(F_ATIME(file), 0, sxp->st.st_atime, 0))
                        iflags |= ITEM_REPORT_ATIME;
 #if !defined HAVE_LCHMOD && !defined HAVE_SETATTRLIST
                if (S_ISLNK(file->mode)) {
@@ -604,7 +605,7 @@ int unchanged_file(char *fn, struct file_struct *file, STRUCT_STAT *st)
        if (ignore_times)
                return 0;
 
-       return time_diff(st, file) == 0;
+       return !time_differs(st, file);
 }
 
 
@@ -781,7 +782,7 @@ static struct file_struct *find_fuzzy(struct file_struct *file, struct file_list
                        if (!S_ISREG(fp->mode) || !F_LENGTH(fp) || fp->flags & FLAG_FILE_SENT)
                                continue;
 
-                       if (F_LENGTH(fp) == F_LENGTH(file) && cmp_time(fp->modtime, 0L, file->modtime, 0L) == 0) {
+                       if (F_LENGTH(fp) == F_LENGTH(file) && same_time(fp->modtime, 0, file->modtime, 0)) {
                                if (DEBUG_GTE(FUZZY, 2))
                                        rprintf(FINFO, "fuzzy size/modtime match for %s\n", f_name(fp, NULL));
                                *fnamecmp_type_ptr = FNAMECMP_FUZZY + i;
@@ -1228,7 +1229,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
                return;
        }
 
-       maybe_ATTRS_SET_NANO = always_checksum ? ATTRS_SET_NANO : 0;
+       maybe_ATTRS_ACCURATE_TIME = always_checksum ? ATTRS_ACCURATE_TIME : 0;
 
        if (skip_dir) {
                if (is_below(file, skip_dir)) {
@@ -1685,7 +1686,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
                goto cleanup;
        }
 
-       if (update_only > 0 && statret == 0 && time_diff(&sx.st, file) > 0) {
+       if (update_only > 0 && statret == 0 && file->modtime - sx.st.st_mtime <= modify_window) {
                if (INFO_GTE(SKIP, 1))
                        rprintf(FINFO, "%s is newer\n", fname);
 #ifdef SUPPORT_HARD_LINKS
@@ -1785,7 +1786,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
                        do_unlink(partialptr);
                        handle_partial_dir(partialptr, PDIR_DELETE);
                }
-               set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT | maybe_ATTRS_SET_NANO);
+               set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT | maybe_ATTRS_ACCURATE_TIME);
                if (itemizing)
                        itemize(fnamecmp, file, ndx, statret, &sx, 0, 0, NULL);
 #ifdef SUPPORT_HARD_LINKS
@@ -2088,7 +2089,7 @@ static void touch_up_dirs(struct file_list *flist, int ndx)
                        do_chmod(fname, file->mode);
                if (need_retouch_dir_times) {
                        STRUCT_STAT st;
-                       if (link_stat(fname, &st, 0) == 0 && time_diff(&st, file)) {
+                       if (link_stat(fname, &st, 0) == 0 && time_differs(&st, file)) {
                                st.st_mtime = file->modtime;
 #ifdef ST_MTIME_NSEC
                                st.ST_MTIME_NSEC = F_MOD_NSEC_or_0(file);

diff --git a/rsync.c b/rsync.c
index 19f09c358551fbe22226234179c0a65b6bd9382c..40e6ad52b5549eeba4145a06bfc49d839a6e4526 100644 (file)
--- a/rsync.c
+++ b/rsync.c
@@ -468,6 +468,21 @@ mode_t dest_mode(mode_t flist_mode, mode_t stat_mode, int dflt_perms,
        return new_mode;
 }
 
+static int same_mtime(struct file_struct *file, STRUCT_STAT *st, int extra_accuracy)
+{
+#ifdef ST_MTIME_NSEC
+       uint32 f1_nsec = F_MOD_NSEC_or_0(file);
+       uint32 f2_nsec = (uint32)st->ST_MTIME_NSEC;
+#else
+       uint32 f1_nsec = 0, f2_nsec = 0;
+#endif
+
+       if (extra_accuracy) /* ignore modify_window when setting the time after a transfer or checksum check */
+               return file->modtime == st->st_mtime && f1_nsec == f2_nsec;
+
+       return same_time(file->modtime, f1_nsec, st->st_mtime , f2_nsec);
+}
+
 int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
                   const char *fnamecmp, int flags)
 {
@@ -570,12 +585,7 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
                memcpy(&sx2.st, &sxp->st, sizeof (sx2.st));
        if (!atimes_ndx || S_ISDIR(sxp->st.st_mode))
                flags |= ATTRS_SKIP_ATIME;
-       if (!(flags & ATTRS_SKIP_MTIME)
-        && (sxp->st.st_mtime != file->modtime
-#ifdef ST_MTIME_NSEC
-         || (flags & ATTRS_SET_NANO && NSEC_BUMP(file) && (uint32)sxp->st.ST_MTIME_NSEC != F_MOD_NSEC(file))
-#endif
-         )) {
+       if (!(flags & ATTRS_SKIP_MTIME) && !same_mtime(file, &sxp->st, flags & ATTRS_ACCURATE_TIME)) {
                sx2.st.st_mtime = file->modtime;
 #ifdef ST_MTIME_NSEC
                sx2.st.ST_MTIME_NSEC = F_MOD_NSEC_or_0(file);
@@ -584,7 +594,7 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
        }
        if (!(flags & ATTRS_SKIP_ATIME)) {
                time_t file_atime = F_ATIME(file);
-               if (cmp_time(sxp->st.st_atime, 0, file_atime, 0) != 0) {
+               if (flags & ATTRS_ACCURATE_TIME || !same_time(sxp->st.st_atime, 0, file_atime, 0)) {
                        sx2.st.st_atime = file_atime;
 #ifdef ST_ATIME_NSEC
                        sx2.st.ST_ATIME_NSEC = 0;
@@ -709,7 +719,7 @@ int finish_transfer(const char *fname, const char *fnametmp,
 
        /* Change permissions before putting the file into place. */
        set_file_attrs(fnametmp, file, NULL, fnamecmp,
-                      ok_to_set_time ? ATTRS_SET_NANO : ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME);
+                      ok_to_set_time ? ATTRS_ACCURATE_TIME : ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME);
 
        /* move tmp file over real file */
        if (DEBUG_GTE(RECV, 1))
@@ -734,7 +744,7 @@ int finish_transfer(const char *fname, const char *fnametmp,
 
   do_set_file_attrs:
        set_file_attrs(fnametmp, file, NULL, fnamecmp,
-                      ok_to_set_time ? ATTRS_SET_NANO : ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME);
+                      ok_to_set_time ? ATTRS_ACCURATE_TIME : ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME);
 
        if (temp_copy_name) {
                if (do_rename(fnametmp, fname) < 0) {

diff --git a/rsync.h b/rsync.h
index f5350da87811b2ac0dfb55baad4c61901a6789f6..e5394a8ed0fa70a30ebacfca5f76068e4349a75f 100644 (file)
--- a/rsync.h
+++ b/rsync.h
@@ -175,7 +175,7 @@
 
 #define ATTRS_REPORT           (1<<0)
 #define ATTRS_SKIP_MTIME       (1<<1)
-#define ATTRS_SET_NANO         (1<<2)
+#define ATTRS_ACCURATE_TIME    (1<<2)
 #define ATTRS_SKIP_ATIME       (1<<3)
 
 #define MSG_FLUSH      2

diff --git a/testsuite/exclude.test b/testsuite/exclude.test
index 099344f237cee7da8b52a15896da09586825168a..fad4fd721a086d0fdb89e7524e3918931e573546 100644 (file)
--- a/testsuite/exclude.test
+++ b/testsuite/exclude.test
@@ -1,6 +1,6 @@
 #! /bin/sh
 
-# Copyright (C) 2003, 2004, 2005 by Wayne Davison <wayned@samba.org>
+# Copyright (C) 2003-2020 Wayne Davison
 
 # This program is distributable under the terms of the GNU GPL (see
 # COPYING).
@@ -12,6 +12,9 @@
 
 . "$suitedir/rsync.fns"
 
+chkfile="$scratchdir/rsync.chk"
+outfile="$scratchdir/rsync.out"
+
 CVSIGNORE='*.junk'
 export CVSIGNORE
 
@@ -113,6 +116,11 @@ rm -rf "$todir"
 # Add a directory symlink.
 ln -s too "$fromdir/bar/down/to/foo/sym"
 
+# Start to prep an --update test dir
+mkdir "$scratchdir/up1" "$scratchdir/up2"
+touch "$scratchdir/up1/older" "$scratchdir/up2/newer"
+touch "$scratchdir/up1/extra-src" "$scratchdir/up2/extra-dest"
+
 # Create chkdir with what we expect to be excluded.
 checkit "$RSYNC -avv '$fromdir/' '$chkdir/'" "$fromdir" "$chkdir"
 sleep 1 # Ensures that the rm commands will tweak the directory times.
@@ -124,6 +132,9 @@ rm "$chkdir"/foo/file[235-9]
 rm "$chkdir"/bar/down/to/foo/to "$chkdir"/bar/down/to/foo/file[235-9]
 rm "$chkdir"/mid/for/foo/extra
 
+# Finish prep for the --update test (run last)
+touch "$scratchdir/up1/newer" "$scratchdir/up2/older"
+
 # Un-tweak the directory times in our first (weak) exclude test (though
 # it's a good test of the --existing option).
 $RSYNC -av --existing --include='*/' --exclude='*' "$fromdir/" "$chkdir/"
@@ -215,5 +226,14 @@ $RSYNC -av $relative_opts --existing --filter='-! */' "$fromdir/foo" "$chkdir/"
 checkit "$RSYNC -avv $relative_opts --exclude='$fromdir/foo/down' \
     '$fromdir/foo' '$todir'" "$chkdir$fromdir/foo" "$todir$fromdir/foo"
 
+# Now we'll test the --update option.
+$RSYNC -aiO --update touch "$scratchdir/up1/" "$scratchdir/up2/" \
+    | tee "$outfile"
+cat <<EOT >"$chkfile"
+>f$all_plus extra-src
+>f..t.$dots newer
+EOT
+diff $diffopt "$chkfile" "$outfile" || test_fail "--update test failed"
+
 # The script would have aborted on error, so getting here means we've won.
 exit 0

diff --git a/util.c b/util.c
index 2c734b7c401fe385554423f9dc0e6d382e298b0c..d86ceb6c6f30f3ba837a8b95f2b9f4dec728b5d1 100644 (file)
--- a/util.c
+++ b/util.c
@@ -1354,30 +1354,17 @@ char *timestring(time_t t)
 
 /* Determine if two time_t values are equivalent (either exact, or in
  * the modification timestamp window established by --modify-window).
- *
- * @retval 0 if the times should be treated as the same
- *
- * @retval +1 if the first is later
- *
- * @retval -1 if the 2nd is later
- **/
-int cmp_time(time_t f1_sec, unsigned long f1_nsec, time_t f2_sec, unsigned long f2_nsec)
+ * Returns 1 if the times the "same", or 0 if they are different. */
+int same_time(time_t f1_sec, unsigned long f1_nsec, time_t f2_sec, unsigned long f2_nsec)
 {
-       if (f2_sec > f1_sec) {
-               /* The final comparison makes sure that modify_window doesn't overflow a
-                * time_t, which would mean that f2_sec must be in the equality window. */
-               if (modify_window <= 0 || (f2_sec > f1_sec + modify_window && f1_sec + modify_window > f1_sec))
-                       return -1;
-       } else if (f1_sec > f2_sec) {
-               if (modify_window <= 0 || (f1_sec > f2_sec + modify_window && f2_sec + modify_window > f2_sec))
-                       return 1;
-       } else if (modify_window < 0) {
-               if (f2_nsec > f1_nsec)
-                       return -1;
-               else if (f1_nsec > f2_nsec)
-                       return 1;
-       }
-       return 0;
+       if (modify_window == 0)
+               return f1_sec == f2_sec;
+       if (modify_window < 0)
+               return f1_sec == f2_sec && f1_nsec == f2_nsec;
+       /* The nano seconds doesn't figure into these checks -- time windows don't care about that. */
+       if (f2_sec > f1_sec)
+               return f2_sec - f1_sec <= modify_window;
+       return f1_sec - f2_sec <= modify_window;
 }
 
 #ifdef __INSURE__XX