Bug 7685 - rsync should not set the creation date on the root folder of an HFS+ volume
Summary: rsync should not set the creation date on the root folder of an HFS+ volume
Status: RESOLVED FIXED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.0.8
Hardware: x86 Mac OS X
: P3 normal (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-16 13:32 UTC by Mike Bombich
Modified: 2011-01-03 18:31 UTC (History)
0 users

See Also:


Attachments
patch to the crtimes patch (815 bytes, patch)
2010-09-16 14:18 UTC, Mike Bombich
no flags Details
Avoid setting inode 2 if it is a directory. (838 bytes, patch)
2010-09-18 12:35 UTC, Wayne Davison
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Bombich 2010-09-16 13:32:04 UTC
HFS+ uses the creation date of the root folder as one reference for identifying hard link references. See http://developer.apple.com/library/mac/technotes/tn/tn1150.html#HardLinks for the reference (albeit only two lines and not particularly explicit). I have reproduced undesirable hard link behavior on Tiger, Leopard, and Snow Leopard using the following script (as you can see, this is certainly an edge case, but I have run into it several times):

##############
#!/bin/sh

source=/Volumes/HLSource
target=/Volumes/HLTarget
rsync="/usr/local/bin/rsync"

cd /tmp
echo "Creating temporary source disk image"
hdiutil create -size 1m -volname HLSource -fs HFS+ -layout NONE -attach HLSource.dmg 1> /dev/null

echo "Creating temporary target disk image (after a delay to guarantee different creation date)"
sleep 2
hdiutil create -size 1m -volname HLTarget -fs HFS+ -layout NONE -attach HLTarget.dmg 1> /dev/null
tgt_dev=`df | awk '/HLTarget/ {print $1}'`

stat -f "  %N: Creation date: %B" $target

echo ""
echo "rsyncing /Volumes/HLSource to target."
$rsync -aHNx --include="/" --exclude=".*" --exclude="*" $source/ $target 1> /dev/null

stat -f "  %N: Creation date: %B" $target

echo ""
echo "Unmounting and remounting the target"
diskutil unmount $tgt_dev 1> /dev/null
diskutil mount $tgt_dev 1> /dev/null

echo "Creating a hard link on the target"
touch $target/one
ln $target/one $target/one_link
stat -f "  %N: inode: %i, Link count: %l" $target/one

echo ""
echo "rsyncing / to target"
$rsync -aHNx --include="/" --exclude=".*" --exclude="*" / $target 1> /dev/null

stat -f "  %N: Creation date: %B" $target

echo ""
echo "Unmounting and remounting the target"
diskutil unmount $tgt_dev 1> /dev/null
diskutil mount $tgt_dev 1> /dev/null

echo "Creating another hard link on the target"
touch $target/two
ln $target/two $target/two_link
stat -f "  %N: inode: %i, Link count: %l" $target/one
stat -f "  %N: inode: %i, Link count: %l" $target/two

#echo "Unmounting and remounting the target"
#diskutil unmount $tgt_dev 1> /dev/null
#diskutil mount $tgt_dev 1> /dev/null

#echo "File list of target root dir"
#stat -f "%N: inode: %i, Link count: %l" $target/one
#stat -f "%N: inode: %i, Link count: %l" $target/two

echo "Cleaning up"
hdiutil eject $source 1> /dev/null
hdiutil eject $target 1> /dev/null
rm /tmp/HLSource.dmg
rm /tmp/HLTarget.dmg

##############

This is yet another filesystem-specific patch, I doubt you would want to commit this in its current form.  Ultimately I think we need a separate object within rsync that we can query for filesystem-specific behavior like this.

This patch (from rsync 3.0.7+fileflags+crtimes) resolves the matter:

##############
crtimes-hfs+.patch
This patch prevents rsync from changing the creation date on the root of a volume. On HFS+, the root inode is always 2, as defined in hfs/hfs_format.h:

kHFSRootFolderID		= 2,	/* Folder ID of the root folder */

So before calling set_create_time(), we verify from the existing stat struct that the inode of the current file is not 2.

diff -Naur rsync-3.0.7_orig/rsync.c rsync-3.0.7/rsync.c
--- rsync-3.0.7_orig/rsync.c	2010-09-16 10:49:54.000000000 -0500
+++ rsync-3.0.7/rsync.c	2010-09-16 10:50:43.000000000 -0500
@@ -480,6 +480,7 @@
 		if (sxp->crtime == 0)
 			sxp->crtime = get_create_time(fname);
 		if (cmp_time(sxp->crtime, file_crtime) != 0
+		 && sxp->st.st_ino != 2 // Don't set the creation date on the root folder of an HFS+ volume
 		 && set_create_time(fname, file_crtime) == 0)
 			updated = 1;
 	}
##############
Comment 1 Mike Bombich 2010-09-16 14:18:44 UTC
Created attachment 5966 [details]
patch to the crtimes patch
Comment 2 Wayne Davison 2010-09-18 12:35:56 UTC
Created attachment 5969 [details]
Avoid setting inode 2 if it is a directory.

Here is my version of the fix.
Comment 3 Mike Bombich 2010-09-19 21:56:34 UTC
Ah yes, avoid another getattrlist, good idea.  Looks great, thanks!
Comment 4 Wayne Davison 2011-01-03 18:31:50 UTC
Will be released in 3.0.8.