Improve --omit-dir-times & --omit-link-times

The code now better handles skipping time setting on dirs and/or links
when --atimes and/or --crtimes is specified without --times.

Author: WayneD

NEWS.md

@@ -12,10 +12,10 @@
    newer for your copy, rsync now sends `--no-W` to the remote rsync in such a
    scenario (just in case the remote rsync is a version with this bug).
 
- - Fix a bug with `--mkpath` if a single-file copy specifies an existing
+ - Fixed a bug with `--mkpath` if a single-file copy specifies an existing
    destination dir with a non-existing destination filename.
 
- - Fix `--update -vv` to output "is uptodate" instead of "is newer" messages
+ - Fixed `--update -vv` to output "is uptodate" instead of "is newer" messages
    for files that are being skipped due to an identical modify time.  (This
    was a new output quirk in 3.2.3.)
 
@@ -51,7 +51,7 @@
  - Reduced memory usage for an incremental transfer that has a bunch of small
    diretories.
 
- - Added support for `--atimes` on macOS and fix using using it without -t.
+ - Added support for `--atimes` on macOS and fixed using using it without -t.
 
  - Rsync can now update the xattrs on a read-only file when your user can
    temporarily add user-write permission to the file. (It always worked for a
@@ -120,7 +120,7 @@
  - Restored the ability to use `--bwlimit=0` to specify no bandwidth limit.  (It
    was accidentally broken in 3.2.2.)
 
- - Fix a bug when combining `--delete-missing-args` with `--no-implied-dirs` &
+ - Fixed a bug when combining `--delete-missing-args` with `--no-implied-dirs` &
    `-R` where rsync might create the destination path of a missing arg.  The
    code also avoids some superfluous warnings for nested paths of removed args.
 

generator.c

@@ -44,7 +44,9 @@ extern int preserve_specials;
 extern int preserve_hard_links;
 extern int preserve_executability;
 extern int preserve_perms;
-extern int preserve_times;
+extern int preserve_mtimes;
+extern int omit_dir_times;
+extern int omit_link_times;
 extern int delete_mode;
 extern int delete_before;
 extern int delete_during;
@@ -460,7 +462,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 && any_time_differs(sxp, file, fname))
+		if (preserve_mtimes && !omit_link_times && any_time_differs(sxp, file, fname))
 			return 0;
 #endif
 #ifdef CAN_CHMOD_SYMLINK
@@ -480,7 +482,7 @@ int unchanged_attrs(const char *fname, struct file_struct *file, stat_x *sxp)
 			return 0;
 #endif
 	} else {
-		if (preserve_times && any_time_differs(sxp, file, fname))
+		if (preserve_mtimes && any_time_differs(sxp, file, fname))
 			return 0;
 		if (perms_differ(file, sxp))
 			return 0;
@@ -504,9 +506,9 @@ void itemize(const char *fnamecmp, struct file_struct *file, int ndx, int statre
 	     const char *xname)
 {
 	if (statret >= 0) { /* A from-dest-dir statret can == 1! */
-		int keep_time = !preserve_times ? 0
-		    : S_ISDIR(file->mode) ? preserve_times & PRESERVE_DIR_TIMES
-		    : S_ISLNK(file->mode) ? preserve_times & PRESERVE_LINK_TIMES
+		int keep_time = !preserve_mtimes ? 0
+		    : S_ISDIR(file->mode) ? !omit_dir_times
+		    : S_ISLNK(file->mode) ? !omit_link_times
 		    : 1;
 
 		if (S_ISREG(file->mode) && F_LENGTH(file) != sxp->st.st_size)
@@ -1411,7 +1413,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 		} else
 			added_perms = 0;
 		if (is_dir < 0) {
-			if (!(preserve_times & PRESERVE_DIR_TIMES))
+			if (!preserve_mtimes || omit_dir_times)
 				goto cleanup;
 			/* In inc_recurse mode we want to make sure any missing
 			 * directories get created while we're still processing
@@ -2238,7 +2240,7 @@ void generate_files(int f_out, const char *local_name)
 	}
 	solo_file = local_name;
 	dir_tweaking = !(list_only || solo_file || dry_run);
-	need_retouch_dir_times = preserve_times & PRESERVE_DIR_TIMES;
+	need_retouch_dir_times = preserve_mtimes && !omit_dir_times;
 	loopchk_limit = allowed_lull ? allowed_lull * 5 : 200;
 	symlink_timeset_failed_flags = ITEM_REPORT_TIME
 	    | (protocol_version >= 30 || !am_server ? ITEM_REPORT_TIMEFAIL : 0);

log.c

@@ -34,7 +34,7 @@ extern int module_id;
 extern int allow_8bit_chars;
 extern int protocol_version;
 extern int always_checksum;
-extern int preserve_times;
+extern int preserve_mtimes;
 extern int msgs2stderr;
 extern int xfersum_type;
 extern int checksum_type;
@@ -706,15 +706,15 @@ static void log_formatted(enum logcode code, const char *format, const char *op,
 				c[1] = 'L';
 				c[3] = '.';
 				c[4] = !(iflags & ITEM_REPORT_TIME) ? '.'
-				     : !preserve_times || !receiver_symlink_times
+				     : !preserve_mtimes || !receiver_symlink_times
 				    || (iflags & ITEM_REPORT_TIMEFAIL) ? 'T' : 't';
 			} else {
 				c[1] = S_ISDIR(file->mode) ? 'd'
 				     : IS_SPECIAL(file->mode) ? 'S'
 				     : IS_DEVICE(file->mode) ? 'D' : 'f';
 				c[3] = !(iflags & ITEM_REPORT_SIZE) ? '.' : 's';
 				c[4] = !(iflags & ITEM_REPORT_TIME) ? '.'
-				     : !preserve_times ? 'T' : 't';
+				     : !preserve_mtimes ? 'T' : 't';
 			}
 			c[2] = !(iflags & ITEM_REPORT_CHANGE) ? '.' : 'c';
 			c[5] = !(iflags & ITEM_REPORT_PERMS) ? '.' : 'p';

options.c

@@ -58,9 +58,11 @@ int preserve_devices = 0;
 int preserve_specials = 0;
 int preserve_uid = 0;
 int preserve_gid = 0;
-int preserve_times = 0;
+int preserve_mtimes = 0;
 int preserve_atimes = 0;
 int preserve_crtimes = 0;
+int omit_dir_times = 0;
+int omit_link_times = 0;
 int update_only = 0;
 int open_noatime = 0;
 int cvs_exclude = 0;
@@ -308,8 +310,6 @@ static int verbose = 0;
 static int do_stats = 0;
 static int do_progress = 0;
 static int daemon_opt;   /* sets am_daemon after option error-reporting */
-static int omit_dir_times = 0;
-static int omit_link_times = 0;
 static int F_option_cnt = 0;
 static int modify_window_set;
 static int itemize_changes = 0;
@@ -622,9 +622,9 @@ static struct poptOption long_options[] = {
   {"xattrs",          'X', POPT_ARG_NONE,   0, 'X', 0, 0 },
   {"no-xattrs",        0,  POPT_ARG_VAL,    &preserve_xattrs, 0, 0, 0 },
   {"no-X",             0,  POPT_ARG_VAL,    &preserve_xattrs, 0, 0, 0 },
-  {"times",           't', POPT_ARG_VAL,    &preserve_times, 1, 0, 0 },
-  {"no-times",         0,  POPT_ARG_VAL,    &preserve_times, 0, 0, 0 },
-  {"no-t",             0,  POPT_ARG_VAL,    &preserve_times, 0, 0, 0 },
+  {"times",           't', POPT_ARG_VAL,    &preserve_mtimes, 1, 0, 0 },
+  {"no-times",         0,  POPT_ARG_VAL,    &preserve_mtimes, 0, 0, 0 },
+  {"no-t",             0,  POPT_ARG_VAL,    &preserve_mtimes, 0, 0, 0 },
   {"atimes",          'U', POPT_ARG_NONE,   0, 'U', 0, 0 },
   {"no-atimes",        0,  POPT_ARG_VAL,    &preserve_atimes, 0, 0, 0 },
   {"no-U",             0,  POPT_ARG_VAL,    &preserve_atimes, 0, 0, 0 },
@@ -1518,7 +1518,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 			preserve_links = 1;
 #endif
 			preserve_perms = 1;
-			preserve_times = 1;
+			preserve_mtimes = 1;
 			preserve_gid = 1;
 			preserve_uid = 1;
 			preserve_devices = 1;
@@ -2257,20 +2257,8 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		parse_filter_str(&filter_list, backup_dir_buf, rule_template(0), 0);
 	}
 
-	if (preserve_times) {
-		preserve_times = PRESERVE_FILE_TIMES;
-		if (!omit_dir_times)
-			preserve_times |= PRESERVE_DIR_TIMES;
-#ifdef CAN_SET_SYMLINK_TIMES
-		if (!omit_link_times)
-			preserve_times |= PRESERVE_LINK_TIMES;
-#endif
-	}
-
-	if (make_backups && !backup_dir) {
-		omit_dir_times = 0; /* Implied, so avoid -O to sender. */
-		preserve_times &= ~PRESERVE_DIR_TIMES;
-	}
+	if (make_backups && !backup_dir)
+		omit_dir_times = -1; /* Implied, so avoid -O to sender. */
 
 	if (stdout_format) {
 		if (am_server && log_format_has(stdout_format, 'I'))
@@ -2498,7 +2486,7 @@ void server_options(char **args, int *argc_p)
 			argstr[x++] = 'K';
 		if (prune_empty_dirs)
 			argstr[x++] = 'm';
-		if (omit_dir_times)
+		if (omit_dir_times > 0)
 			argstr[x++] = 'O';
 		if (omit_link_times)
 			argstr[x++] = 'J';
@@ -2531,7 +2519,7 @@ void server_options(char **args, int *argc_p)
 		argstr[x++] = 'g';
 	if (preserve_devices) /* ignore preserve_specials here */
 		argstr[x++] = 'D';
-	if (preserve_times)
+	if (preserve_mtimes)
 		argstr[x++] = 't';
 	if (preserve_atimes) {
 		argstr[x++] = 'U';

rsync.c

@@ -32,7 +32,9 @@ extern int preserve_acls;
 extern int preserve_xattrs;
 extern int preserve_perms;
 extern int preserve_executability;
-extern int preserve_times;
+extern int preserve_mtimes;
+extern int omit_dir_times;
+extern int omit_link_times;
 extern int am_root;
 extern int am_server;
 extern int am_daemon;
@@ -575,18 +577,18 @@ int set_file_attrs(const char *fname, struct file_struct *file, stat_x *sxp,
 		set_xattr(fname, file, fnamecmp, sxp);
 #endif
 
-	if (!preserve_times)
-		flags |= ATTRS_SKIP_MTIME | (atimes_ndx ? 0 : ATTRS_SKIP_ATIME) | (crtimes_ndx ? 0 : ATTRS_SKIP_CRTIME);
-	else if ((!(preserve_times & PRESERVE_DIR_TIMES) && S_ISDIR(sxp->st.st_mode))
-	      || (!(preserve_times & PRESERVE_LINK_TIMES) && S_ISLNK(sxp->st.st_mode)))
+	if ((omit_dir_times && S_ISDIR(sxp->st.st_mode))
+	 || (omit_link_times && S_ISLNK(sxp->st.st_mode)))
 		flags |= ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME | ATTRS_SKIP_CRTIME;
-	else if (sxp != &sx2)
-		memcpy(&sx2.st, &sxp->st, sizeof (sx2.st));
+	if (!preserve_mtimes)
+		flags |= ATTRS_SKIP_MTIME;
 	if (!atimes_ndx || S_ISDIR(sxp->st.st_mode))
 		flags |= ATTRS_SKIP_ATIME;
 	/* Don't set the creation date on the root folder of an HFS+ volume. */
 	if (sxp->st.st_ino == 2 && S_ISDIR(sxp->st.st_mode))
 		flags |= ATTRS_SKIP_CRTIME;
+	if (sxp != &sx2 && !(flags & (ATTRS_SKIP_MTIME | ATTRS_SKIP_ATIME)))
+		memcpy(&sx2.st, &sxp->st, sizeof sx2.st);
 	if (!(flags & ATTRS_SKIP_MTIME) && !same_mtime(file, &sxp->st, flags & ATTRS_ACCURATE_TIME)) {
 		sx2.st.st_mtime = file->modtime;
 #ifdef ST_MTIME_NSEC

rsync.h

@@ -1327,10 +1327,6 @@ extern int errno;
 #define IS_SPECIAL(mode) (S_ISSOCK(mode) || S_ISFIFO(mode))
 #define IS_DEVICE(mode) (S_ISCHR(mode) || S_ISBLK(mode))
 
-#define PRESERVE_FILE_TIMES	(1<<0)
-#define PRESERVE_DIR_TIMES	(1<<1)
-#define PRESERVE_LINK_TIMES	(1<<2)
-
 /* Initial mask on permissions given to temporary files.  Mask off setuid
      bits and group access because of potential race-condition security
      holes, and mask other access because mode 707 is bizarre */

t_stub.c

@@ -29,10 +29,11 @@ int protect_args = 0;
 int module_id = -1;
 int relative_paths = 0;
 int module_dirlen = 0;
-int preserve_times = 0;
+int preserve_mtimes = 0;
 int preserve_xattrs = 0;
 int preserve_perms = 0;
 int preserve_executability = 0;
+int omit_link_times = 0;
 int open_noatime = 0;
 size_t max_alloc = 0; /* max_alloc is needed when combined with util2.o */
 char *partial_dir;

util1.c

@@ -31,8 +31,9 @@ extern int do_fsync;
 extern int protect_args;
 extern int modify_window;
 extern int relative_paths;
-extern int preserve_times;
+extern int preserve_mtimes;
 extern int preserve_xattrs;
+extern int omit_link_times;
 extern int preallocate_files;
 extern char *module_dir;
 extern unsigned int module_dirlen;
@@ -159,8 +160,8 @@ int set_times(const char *fname, STRUCT_STAT *stp)
 
 #include "case_N.h"
 		switch_step++;
-		if (preserve_times & PRESERVE_LINK_TIMES) {
-			preserve_times &= ~PRESERVE_LINK_TIMES;
+		if (!omit_link_times) {
+			omit_link_times = 1;
 			if (S_ISLNK(stp->st_mode))
 				return 1;
 		}