From: Wayne Davison Date: Sat, 13 Jun 2020 09:32:15 +0000 (-0700) Subject: Fix overzealous setting of mtime & tweak time comparisons X-Git-Tag: v3.2.0pre1~8 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d32696129097fcc2c0560da2f6b1d481b854a2b9;p=thirdparty%2Frsync.git 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 a173aada..be406bef 100644 --- 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 210e5f31..70e11374 100644 --- 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 19f09c35..40e6ad52 100644 --- 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 f5350da8..e5394a8e 100644 --- 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 099344f2..fad4fd72 100644 --- a/testsuite/exclude.test +++ b/testsuite/exclude.test @@ -1,6 +1,6 @@ #! /bin/sh -# Copyright (C) 2003, 2004, 2005 by Wayne Davison +# 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 <"$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 2c734b7c..d86ceb6c 100644 --- 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