Skip to content
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

Do not sync owner/group for FAT filesystems #20

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

matthijskooijman
Copy link

This can potentially cause rsync error messages, especially if the filesystem is already mounted with a uid= option (which will not be copied to the destination mount, leading to different owners and thus rsync trying and failing to fix the owner).

This commit was previously submitted as part of billw2#140 (but really unrelated to the core of that PR, so I'm submitting it separately here now) and have been in use in my project since that PR was submitted two years ago. Now I've just rebased and reviewed them, the changes still applied without issue.

This can potentially cause rsync error messages, especially if the
filesystem is already mounted with a uid= option (which will not be
copied to the destination mount, leading to different owners and thus
rsync trying and failing to fix the owner).
@matthijskooijman
Copy link
Author

Prompted by @framps' suggestion of adding more comments inline, I've added clarification of the FAT-specific rsync options in a comment, and also restuctured the code to not have two complete lists of rsync options (one for FAT and one for the rest), which are hard to read and prone to getting out of sync. Now, for FAT filesystems, the --no-owner and --no-goup options are added to the default options, which is a lot easier to read and more robust.

I have not been able to test this yet, so I'm marking this as draft until I did.

@matthijskooijman matthijskooijman marked this pull request as draft April 10, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant