-
Notifications
You must be signed in to change notification settings - Fork 345
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add independent update mode for symbolic links #198
base: master
Are you sure you want to change the base?
Conversation
The test included in the PR needs further work to run on FreeBSD apparently. |
I'll note that tweaking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The redundant calls to the macro were intentional, commit reverted.
Noted and added it in commit |
@@ -231,10 +232,15 @@ static int readlink_stat(const char *path, STRUCT_STAT *stp, char *linkbuf) | |||
int link_stat(const char *path, STRUCT_STAT *stp, int follow_dirlinks) | |||
{ | |||
#ifdef SUPPORT_LINKS | |||
if (copy_links) | |||
if (copy_links && update_links == 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they asked for --copy-links they should get --copy-links. Remember that it's only the sending side that sees this flag set, not the receiving side, so combining --copy-links with --update-links would mean that the sender doesn't send any symlinks, and the receiver could deal with any existing symlinks in the destination using the new option. (Thus, drop this change.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two modes appear mutually exclusive from a user standpoint. Either every symlink is transformed into referent (--copy-links
) or copied across if newer (--update-links
).
so combining --copy-links with --update-links would mean that the sender doesn't send any symlink
From testing the PR'd code briefly now, when both modes are enabled, --update-links
trumps --copy-links
, by virtue of a subsequent condition at if (update_links && S_ISLNK(stp->st_mode))
.
Added #274 to discuss how to handle symlink mode precedence.
STRUCT_STAT st; | ||
if (x_stat(path, &st, NULL) == 0 && !S_ISDIR(st.st_mode)) | ||
return x_lstat(path, stp, NULL); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This hunk baffles me. It does a stat of the referent file, and if it's a non-dir it re-stats the symlink's stat that is already present in stp and returns. But if it is not a dir, the following code also returns the same stat data that is already in stp. So, it seems like this can be dropped as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Precisely, if the symlink points at a non-directory referent, return the symlink's stat or fall through. It's only apparent looking at the surrounding code why the x_lstat call is redundant. Committed as aed6235
, works on assumption lstat or lstat64 is guaranteed to return either 0 or -1 on all platforms.
41ccee5
to
17c7806
Compare
6b54077
to
af4de04
Compare
df08e29
to
1dc68a2
Compare
This reverts commit c3437c1, the redundant macro call is intended to avoid running the macro if neither update_links nor follow_dirlinks are enabled.
b42230f
to
7ba3169
Compare
This PR enhances rsync with an independent update mode for symbolic links. Its main purpose is to skip symlinks that are newer on the receiver. The mode is enabled independently of the update mode via the
--update-links
switch. To extend the behaviour of this option to empty directories, an additional--allow-link-update-dir
switch may be enabled. In this mode, a less recent empty directory on the destination may be replaced with a more recent symbolic link from the source.The PR includes a sanity test to verify the impact of the proposed modes on a set of files (relative, dangling, absolute, unsafe, devlink dirlink, empty directory, non-empty directory, fifo) under different circumstances.