Implement --tweak, --no-tweak, --no-tweak-hlinked options. old/matt
authorMatt McCutchen <matt@mattmccutchen.net>
Mon, 17 Mar 2008 23:55:50 +0000 (19:55 -0400)
committerMatt McCutchen <matt@mattmccutchen.net>
Thu, 20 Mar 2008 18:11:24 +0000 (14:11 -0400)
- 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.

generator.c
options.c
receiver.c
rsync.yo
testsuite/itemize.test
testsuite/rsync.fns
testsuite/tweak-opts.test [new file with mode: 0644]
util.c

index ff31e06..4a3e65b 100644 (file)
@@ -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);
index b63956f..54c94d8 100644 (file)
--- 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)
index da90b5b..ecf7295 100644 (file)
@@ -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;
index fdc0e23..8852adf 100644 (file)
--- 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
index 7278034..fff9511 100644 (file)
@@ -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
index de62866..441fbb4 100644 (file)
@@ -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 (file)
index 0000000..459bb11
--- /dev/null
@@ -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 (file)
--- 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)
 {