[Rsync-patches] [PATCH] Implement --tweak, --no-tweak, --no-tweak-hlinked options.

Matt McCutchen <matt at mattmccutchen.net>
Thu Mar 20 19:54:11 PDT 2008


- The meat of the implementation is in recv_generator.
- Comprehensive test case included!  In the process, I enhanced check_perms and
  moved the code to test a config.h setting from itemize.test to a function
  config_test in rsync.fns.
---

*** This is a test of the rsync-patches mailing list!  This is only a test! ***

Matt

 generator.c               |   76 +++++++++++++++++++----
 options.c                 |   25 +++++++-
 receiver.c                |    3 +
 rsync.yo                  |   29 +++++++++
 testsuite/itemize.test    |    3 +-
 testsuite/rsync.fns       |    6 ++-
 testsuite/tweak-opts.test |  150 +++++++++++++++++++++++++++++++++++++++++++++
 util.c                    |    4 +-
 8 files changed, 275 insertions(+), 21 deletions(-)
 create mode 100644 testsuite/tweak-opts.test

diff --git a/generator.c b/generator.c
index ff31e06..4a3e65b 100644
--- a/generator.c
+++ b/generator.c
@@ -58,6 +58,7 @@ extern int update_only;
 extern int ignore_existing;
 extern int ignore_non_existing;
 extern int inplace;
+extern int tweak_attrs;
 extern int append_mode;
 extern int make_backups;
 extern int csum_length;
@@ -1210,6 +1211,11 @@ static void list_file_entry(struct file_struct *f)
 	}
 }
 
+static int may_tweak(STRUCT_STAT *st)
+{
+	return tweak_attrs == 2 || (tweak_attrs == 1 && st->st_nlink == 1);
+}
+
 static int phase = 0;
 static int dflt_perms;
 
@@ -1501,6 +1507,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 
 	if (preserve_links && S_ISLNK(file->mode)) {
 #ifdef SUPPORT_LINKS
+		int iflags = 0;
 		const char *sl = F_SYMLINK(file);
 		if (safe_symlinks && unsafe_symlink(sl, fname)) {
 			if (verbose) {
@@ -1521,7 +1528,15 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 			else if ((len = readlink(fname, lnk, MAXPATHLEN-1)) > 0
 			      && strncmp(lnk, sl, len) == 0 && sl[len] == '\0') {
 				/* The link is pointing to the right place. */
-				set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT);
+				if (may_tweak(&sx.st)) {
+					if (verbose > 2)
+						rprintf(FINFO, "possibly tweaking attributes of %s\n", fname);
+					set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT);
+				} else if (!unchanged_attrs(fname, file, &sx)) {
+					if (verbose > 2)
+						rprintf(FINFO, "recreating %s due to changed attributes\n", fname);
+					goto recreate_symlink;
+				}
 				if (itemizing)
 					itemize(fname, file, ndx, 0, &sx, 0, 0, NULL);
 #if defined SUPPORT_HARD_LINKS && defined CAN_HARDLINK_SYMLINK
@@ -1531,7 +1546,9 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 				if (remove_source_files == 1)
 					goto return_with_success;
 				goto cleanup;
-			}
+			} else
+				iflags = ITEM_REPORT_CHANGE;
+		  recreate_symlink:
 			/* Not the right symlink (or not a symlink), so
 			 * delete it. */
 			if (delete_item(fname, sx.st.st_mode, del_opts | DEL_FOR_SYMLINK) != 0)
@@ -1565,18 +1582,20 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 			set_file_attrs(fname, file, NULL, NULL, 0);
 			if (itemizing) {
 				itemize(fname, file, ndx, statret, &sx,
-					ITEM_LOCAL_CHANGE|ITEM_REPORT_CHANGE, 0, NULL);
+					ITEM_LOCAL_CHANGE|iflags, 0, NULL);
 			}
-			if (code != FNONE && verbose)
+			if ((iflags & ITEM_REPORT_CHANGE) && code != FNONE && verbose)
 				rprintf(code, "%s -> %s\n", fname, sl);
 #ifdef SUPPORT_HARD_LINKS
 			if (preserve_hard_links && F_IS_HLINKED(file))
 				finish_hard_link(file, fname, ndx, NULL, itemizing, code, -1);
 #endif
-			/* This does not check remove_source_files == 1
-			 * because this is one of the items that the old
-			 * --remove-sent-files option would remove. */
-			if (remove_source_files)
+			/* When the symlink value changed, we do not check
+			 * remove_source_files == 1 because this is one of the
+			 * items that the old --remove-sent-files option would
+			 * remove. */
+			if ((iflags & ITEM_REPORT_CHANGE) ? remove_source_files
+			      : remove_source_files == 1)
 				goto return_with_success;
 		}
 #endif
@@ -1585,6 +1604,7 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 
 	if ((am_root && preserve_devices && IS_DEVICE(file->mode))
 	 || (preserve_specials && IS_SPECIAL(file->mode))) {
+		int iflags = 0;
 		uint32 *devp = F_RDEV_P(file);
 		dev_t rdev = MAKEDEV(DEV_MAJOR(devp), DEV_MINOR(devp));
 		if (statret == 0) {
@@ -1602,7 +1622,15 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 			 && BITS_EQUAL(sx.st.st_mode, file->mode, _S_IFMT)
 			 && sx.st.st_rdev == rdev) {
 				/* The device or special file is identical. */
-				set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT);
+				if (may_tweak(&sx.st)) {
+					if (verbose > 2)
+						rprintf(FINFO, "possibly tweaking attributes of %s\n", fname);
+					set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT);
+				} else if (!unchanged_attrs(fname, file, &sx)) {
+					if (verbose > 2)
+						rprintf(FINFO, "recreating %s due to changed attributes\n", fname);
+					goto recreate_D;
+				}
 				if (itemizing)
 					itemize(fname, file, ndx, 0, &sx, 0, 0, NULL);
 #ifdef SUPPORT_HARD_LINKS
@@ -1612,7 +1640,9 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 				if (remove_source_files == 1)
 					goto return_with_success;
 				goto cleanup;
-			}
+			} else
+				iflags = ITEM_REPORT_CHANGE;
+		  recreate_D:
 			if (delete_item(fname, sx.st.st_mode, del_opts | del_for_flag) != 0)
 				goto cleanup;
 		} else if (basis_dir[0] != NULL) {
@@ -1649,9 +1679,9 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 			set_file_attrs(fname, file, NULL, NULL, 0);
 			if (itemizing) {
 				itemize(fname, file, ndx, statret, &sx,
-					ITEM_LOCAL_CHANGE|ITEM_REPORT_CHANGE, 0, NULL);
+					ITEM_LOCAL_CHANGE|iflags, 0, NULL);
 			}
-			if (code != FNONE && verbose)
+			if ((iflags & ITEM_REPORT_CHANGE) && code != FNONE && verbose)
 				rprintf(code, "%s\n", fname);
 #ifdef SUPPORT_HARD_LINKS
 			if (preserve_hard_links && F_IS_HLINKED(file))
@@ -1769,13 +1799,31 @@ static void recv_generator(char *fname, struct file_struct *file, int ndx,
 	else if (fnamecmp_type == FNAMECMP_FUZZY)
 		;
 	else if (unchanged_file(fnamecmp, file, &sx.st)) {
+		/* fnamecmp == fname, fnamecmp_type == FNAMECMP_FNAME */
+		int iflags = 0;
+
 		if (partialptr) {
 			do_unlink(partialptr);
 			handle_partial_dir(partialptr, PDIR_DELETE);
 		}
-		set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT);
+		if (may_tweak(&sx.st)) {
+			/* Currently, we call set_file_attrs on all tweakable
+			 * files, though ideally it would have no effect when
+			 * unchanged_attrs returns true. */
+			if (verbose > 2)
+				rprintf(FINFO, "possibly tweaking attributes of %s\n", fname);
+			set_file_attrs(fname, file, &sx, NULL, maybe_ATTRS_REPORT);
+		} else if (!unchanged_attrs(fname, file, &sx)) {
+			/* Need to recreate the file.
+			 * copy_altdest_file sets its attributes, etc. */
+			if (verbose > 2)
+				rprintf(FINFO, "recreating %s due to changed attributes\n", fname);
+			if (!dry_run && copy_altdest_file(fnamecmp, fname, file))
+				goto cleanup;
+			iflags |= ITEM_LOCAL_CHANGE;
+		}
 		if (itemizing)
-			itemize(fnamecmp, file, ndx, statret, &sx, 0, 0, NULL);
+			itemize(fnamecmp, file, ndx, statret, &sx, iflags, 0, NULL);
 #ifdef SUPPORT_HARD_LINKS
 		if (preserve_hard_links && F_IS_HLINKED(file))
 			finish_hard_link(file, fname, ndx, &sx.st, itemizing, code, -1);
diff --git a/options.c b/options.c
index b63956f..54c94d8 100644
--- a/options.c
+++ b/options.c
@@ -121,6 +121,7 @@ int modify_window = 0;
 int blocking_io = -1;
 int checksum_seed = 0;
 int inplace = 0;
+int tweak_attrs = 2; /* 2 = always, 1 = if not hlinked, 0 = never */
 int delay_updates = 0;
 long block_size = 0; /* "long" because popt can't set an int32. */
 char *skip_compress = NULL;
@@ -329,6 +330,8 @@ void usage(enum logcode F)
   rprintf(F,"     --inplace               update destination files in-place (SEE MAN PAGE)\n");
   rprintf(F,"     --append                append data onto shorter files\n");
   rprintf(F,"     --append-verify         like --append, but with old data in file checksum\n");
+  rprintf(F,"     --no-tweak              recreate dest files instead of tweaking attrs\n");
+  rprintf(F,"     --no-tweak-hlinked      ... if they have multiple hard links\n");
   rprintf(F," -d, --dirs                  transfer directories without recursing\n");
   rprintf(F," -l, --links                 copy symlinks as symlinks\n");
   rprintf(F," -L, --copy-links            transform symlink into referent file/dir\n");
@@ -545,6 +548,9 @@ static struct poptOption long_options[] = {
   {"append",           0,  POPT_ARG_NONE,   0, OPT_APPEND, 0, 0 },
   {"append-verify",    0,  POPT_ARG_VAL,    &append_mode, 2, 0, 0 },
   {"no-append",        0,  POPT_ARG_VAL,    &append_mode, 0, 0, 0 },
+  {"no-tweak",         0,  POPT_ARG_VAL,    &tweak_attrs, 0, 0, 0 },
+  {"no-tweak-hlinked", 0,  POPT_ARG_VAL,    &tweak_attrs, 1, 0, 0 },
+  {"tweak",            0,  POPT_ARG_VAL,    &tweak_attrs, 2, 0, 0 },
   {"del",              0,  POPT_ARG_NONE,   &delete_during, 0, 0, 0 },
   {"delete",           0,  POPT_ARG_NONE,   &delete_mode, 0, 0, 0 },
   {"delete-before",    0,  POPT_ARG_NONE,   &delete_before, 0, 0, 0 },
@@ -1584,11 +1590,24 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 		partial_dir = tmp_partialdir;
 
 	if (inplace) {
+		char *inplace_opt = append_mode ? "append" : "inplace";
 #ifdef HAVE_FTRUNCATE
+		if (tweak_attrs == 0) {
+			snprintf(err_buf, sizeof err_buf,
+					"--no-tweak controverts --%s.  Remove --%s.\n",
+					inplace_opt, inplace_opt);
+			return 0;
+		}
+		if (tweak_attrs == 1) {
+			snprintf(err_buf, sizeof err_buf,
+					"The combination of --no-tweak-hlinked and --%s is not implemented.\n",
+					inplace_opt);
+			return 0;
+		}
 		if (partial_dir) {
 			snprintf(err_buf, sizeof err_buf,
 				 "--%s cannot be used with --%s\n",
-				 append_mode ? "append" : "inplace",
+				 inplace_opt,
 				 delay_updates ? "delay-updates" : "partial-dir");
 			return 0;
 		}
@@ -1602,7 +1621,7 @@ int parse_arguments(int *argc_p, const char ***argv_p)
 #else
 		snprintf(err_buf, sizeof err_buf,
 			 "--%s is not supported on this %s\n",
-			 append_mode ? "append" : "inplace",
+			 inplace_opt,
 			 am_server ? "server" : "client");
 		return 0;
 #endif
@@ -1935,6 +1954,8 @@ void server_options(char **args, int *argc_p)
 			args[ac++] = "--super";
 		if (size_only)
 			args[ac++] = "--size-only";
+		if (tweak_attrs != 2)
+			args[ac++] = (tweak_attrs == 1) ? "--no-tweak-hlinked" : "--no-tweak";
 	} else {
 		if (skip_compress) {
 			if (asprintf(&arg, "--skip-compress=%s", skip_compress) < 0)
diff --git a/receiver.c b/receiver.c
index da90b5b..ecf7295 100644
--- a/receiver.c
+++ b/receiver.c
@@ -449,6 +449,9 @@ int recv_files(int f_in, char *local_name)
 			maybe_log_item(file, iflags, itemizing, xname);
 #ifdef SUPPORT_XATTRS
 			if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && !dry_run)
+				/* If the file needed to be recreated before we
+				 * set xattrs, the generator has already noticed
+				 * the difference in xattrs and done so. */
 				set_file_attrs(fname, file, NULL, fname, 0);
 #endif
 			continue;
diff --git a/rsync.yo b/rsync.yo
index fdc0e23..8852adf 100644
--- a/rsync.yo
+++ b/rsync.yo
@@ -329,6 +329,8 @@ to the detailed description below for a complete description.  verb(
      --inplace               update destination files in-place
      --append                append data onto shorter files
      --append-verify         --append w/old data in file checksum
+     --no-tweak              recreate dest files rather than tweak attrs
+     --no-tweak-hlinked      ... if they are hard-linked
  -d, --dirs                  transfer directories without recursing
  -l, --links                 copy symlinks as symlinks
  -L, --copy-links            transform symlink into referent file/dir
@@ -743,6 +745,33 @@ bf(--append-verify), so if you are interacting with an older rsync (or the
 transfer is using a protocol prior to 30), specifying either append option
 will initiate an bf(--append-verify) transfer.
 
+dit(bf(--no-tweak)) If a corresponding source file and destination file
+are determined to have identical data (or symlink target path, etc.) but differ
+in preserved attributes, rsync's default behavior (which can be explicitly
+requested via bf(--tweak)) is to tweak the attributes of the destination file
+in place.  bf(--no-tweak) makes rsync recreate the destination file instead.
+
+You can use bf(--no-tweak) to avoid the race inherent in
+bf(--no-tweak-hlinked) if the destination is subject to concurrent
+modification.  It may also be useful to ensure that, if multiple attributes
+of the destination file need updating, the attributes visible at the
+destination path change simultaneously.  (Caveat: In the current
+implementation, the abbreviated extended attributes of the recreated file
+may be set after it is moved into place.)
+
+This option conflicts with bf(--inplace) and with bf(--append) because those
+combinations don't make sense.
+
+dit(bf(--no-tweak-hlinked)) Like bf(--no-tweak) but only affects destination
+files that have more than one hard link.
+You can use bf(--no-tweak-linked) to safely update a backup that has files
+hard-linked from a previous backup if you are not worried about concurrent
+modification to the destination.
+
+This option currently conflicts with bf(--inplace) and bf(--append).  In
+the future, it might selectively disable those options for multiply linked
+destination files.
+
 dit(bf(-d, --dirs)) Tell the sending side to include any directories that
 are encountered.  Unlike bf(--recursive), a directory's contents are not copied
 unless the directory name specified is "." or ends with a trailing slash
diff --git a/testsuite/itemize.test b/testsuite/itemize.test
index 7278034..fff9511 100644
--- a/testsuite/itemize.test
+++ b/testsuite/itemize.test
@@ -28,8 +28,7 @@ ln "$fromdir/foo/config1" "$fromdir/foo/extra"
 rm -f "$to2dir"
 
 # Check if rsync is set to hard-link symlinks.
-confile=`echo "$scratchdir" | sed 's;/testtmp/itemize$;/config.h;'`
-if egrep '^#define CAN_HARDLINK_SYMLINK 1' "$confile" >/dev/null; then
+if config_test CAN_HARDLINK_SYMLINK; then
     L=hL
 else
     L=cL
diff --git a/testsuite/rsync.fns b/testsuite/rsync.fns
index de62866..441fbb4 100644
--- a/testsuite/rsync.fns
+++ b/testsuite/rsync.fns
@@ -78,7 +78,7 @@ rsync_ls_lR() {
 }
 
 check_perms() {
-    perms=`"$TOOLDIR/tls" "$1" | sed 's/^[-d]\(.........\).*/\1/'`
+    perms=`"$TOOLDIR/tls" "$1" | sed 's/^[-dlp]\(.........\).*/\1/'`
     if test $perms = $2; then
 	return 0
     fi
@@ -91,6 +91,10 @@ rsync_getgroups() {
     "$TOOLDIR/getgroups"
 }
 
+confile=`echo "$scratchdir" | sed 's;/testtmp/[^/]*$;/config.h;'`
+config_test() {
+    egrep "^#define $1 1" "$confile" >/dev/null
+}
 
 ####################
 # Build test directories $todir and $fromdir, with $fromdir full of files.
diff --git a/testsuite/tweak-opts.test b/testsuite/tweak-opts.test
new file mode 100644
index 0000000..459bb11
--- /dev/null
+++ b/testsuite/tweak-opts.test
@@ -0,0 +1,150 @@
+#! /bin/sh
+
+# This program is distributable under the terms of the GNU GPL (see
+# COPYING).
+
+# Test the --tweak, --no-tweak, and --no-tweak-hlinked options.
+
+. $srcdir/testsuite/rsync.fns
+
+# Replacement for [ a -ef b ] that doesn't follow symlinks.
+samefile() {
+	inum1=`ls -di "$1" | sed 's/ .*$//'`
+	inum2=`ls -di "$2" | sed 's/ .*$//'`
+	[ "$inum1" = "$inum2" ]
+}
+
+chkfile="$scratchdir/rsync.chk"
+outfile="$scratchdir/rsync.out"
+
+makepath "$fromdir"
+
+# usage: testit OPTION FTYPE ATTR
+testit() {
+	option="$1"
+	ftype=$2
+	attr=$3
+	testid="$option $ftype $attr"
+	echo "Running test: ($testid)"
+
+	# Prepare the destination directory:
+	# file1: attr different from source
+	# file2: attr different from source, hard-linked with file2hl
+	# file3: attr same as source, hard-linked with file3hl
+	extraopt=''
+	rm -rf "$todir"
+	$RSYNC -a "$fromdir/" "$todir/"
+	case "$ftype,$attr" in
+	*,p)
+		chmod 700 "$todir/file1" "$todir/file2"
+		icode='...p.....'
+		;;
+	L,t)
+		# Grab the new symlinks.
+		$RSYNC -a "$scratchdir/newlinks/" "$todir/"
+		icode='..t......'
+		;;
+	*,t)
+		touch -d @1194208678 "$todir/file1" "$todir/file2"
+		# Make sure unchanged_attrs signals a difference in mtime.
+		extraopt='--checksum'
+		icode='..t......'
+		;;
+	t_symlink)
+	esac
+	ln "$todir/file2" "$todir/file2hl"
+	ln "$todir/file3" "$todir/file3hl"
+
+	# Determine the expected results.
+	case "$option" in
+	''|*--tweak)
+		# Tweak both files, tweaking through to file2hl.
+		a1='.'
+		a2='.'
+		changes_f2hl=1
+		;;
+	--no-tweak-hlinked)
+		# Tweak file1 and recreate file2, breaking the link to file2hl.
+		a1='.'
+		a2='c'
+		changes_f2hl=''
+		;;
+	--no-tweak)
+		# Recreate both file1 and file2, breaking the link to file2hl.
+		a1='c'
+		a2='c'
+		changes_f2hl=''
+		;;
+	esac
+	if [ "$ftype" = L ]; then
+		ltgt=' -> foo'
+	else
+		ltgt=''
+	fi
+	cat >"$chkfile" <<EOCHK
+$a1$ftype$icode file1$ltgt
+$a2$ftype$icode file2$ltgt
+EOCHK
+
+	$RSYNC -a -i --omit-dir-times $option $extraopt "$fromdir/" "$todir/" | tee "$outfile"
+	diff $diffopt "$chkfile" "$outfile" || test_fail "itemize mismatch for test ($testid)"
+
+	if [ $changes_f2hl ]; then
+		samefile "$todir/file2" "$todir/file2hl" || test_fail "rsync should not have broken the file2 hard link"
+		if [ $attr = p ]; then
+			check_perms "$todir/file2hl" rw------- "test ($testid)"
+		fi
+	else
+		! samefile "$todir/file2" "$todir/file2hl" || test_fail "rsync should have broken the file2 hard link"
+		if [ $attr = p ]; then
+			check_perms "$todir/file2hl" rwx------ "test ($testid)"
+		fi
+	fi
+
+	# Nothing should ever happen to file3.
+	samefile "$todir/file3" "$todir/file3hl" || test_fail "rsync should not have broken the file3 hard link"
+}
+
+# REGULAR FILES
+echo data >"$fromdir/file1"
+echo data >"$fromdir/file2"
+echo data >"$fromdir/file3"
+chmod 600 "$fromdir/file1" "$fromdir/file2" "$fromdir/file3"
+for o in '' '--no-tweak --tweak' '--no-tweak-hlinked' '--no-tweak'; do
+	for a in p t; do
+		testit "$o" f $a
+	done
+done
+
+# SYMLINKS
+rm -f "$fromdir/file1" "$fromdir/file2" "$fromdir/file3"
+if config_test CAN_HARDLINK_SYMLINK && config_test HAVE_LUTIMES; then
+	ln -s foo "$fromdir/file1"
+	ln -s foo "$fromdir/file2"
+	ln -s foo "$fromdir/file3"
+	# Since we don't have `touch -h', sleep once and make some newer
+	# symlinks that each test can copy into the destination directory.
+	sleep 1
+	makepath "$scratchdir/newlinks"
+	ln -s foo "$scratchdir/newlinks/file1"
+	ln -s foo "$scratchdir/newlinks/file2"
+	for o in '' '--no-tweak --tweak' '--no-tweak-hlinked' '--no-tweak'; do
+		# For symlinks, we test only times, not permissions.
+		testit "$o" L t
+	done
+fi
+
+# SPECIAL FILES
+rm -f "$fromdir/file1" "$fromdir/file2" "$fromdir/file3"
+if config_test CAN_HARDLINK_SPECIAL \
+	&& mkfifo "$fromdir/file1" "$fromdir/file2" "$fromdir/file3"; then
+	chmod 600 "$fromdir/file1" "$fromdir/file2" "$fromdir/file3"
+	for o in '' '--no-tweak --tweak' '--no-tweak-hlinked' '--no-tweak'; do
+		for a in p t; do
+			testit "$o" S $a
+		done
+	done
+fi
+
+# 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 5a8333e..add1486 100644
--- a/util.c
+++ b/util.c
@@ -265,8 +265,8 @@ static int safe_read(int desc, char *ptr, size_t len)
 /* Copy a file.  If ofd < 0, copy_file unlinks and opens the "dest" file.
  * Otherwise, it just writes to and closes the provided file descriptor.
  *
- * This is used in conjunction with the --temp-dir, --backup, and
- * --copy-dest options. */
+ * This is used in conjunction with the --temp-dir, --backup,
+ * --copy-dest, and --no-tweak* options. */
 int copy_file(const char *source, const char *dest, int ofd,
 	      mode_t mode, int create_bak_dir)
 {
-- 
1.5.5.rc0.17.g777c




More information about the rsync-patches mailing list