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

Preserve links #24

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@mzpqnxow
Copy link

mzpqnxow commented Feb 14, 2019

This supercedes PR #23 and resolives issues #22 and #21

I will close PR #23

Adam Greene added some commits Feb 14, 2019

@mzpqnxow

This comment has been minimized.

Copy link
Author

mzpqnxow commented Feb 14, 2019

I also added a pedantic improvement from @kmarty from his PR #16 which changes "../" to "${DEST}"

Since the incremental directory is always one directory up from the full directory, the existing code works fine, but in my opinion the more correct way to do this is as he suggested, by explicitly specifying the full destination root.

Sorry for lumping all of these commits into one branch, but I think they will not be controversial changes and so can all be accepted or rejected in whole

@mzpqnxow

This comment has been minimized.

Copy link
Author

mzpqnxow commented Feb 14, 2019

@cytopia I just noticed this in the documentation:

By default the only rsync option used is --recursive.
This is due to the fact that some remote NAS implementations do not support
symlinks, changing owner, group or permissions (due to restrictive ACL's).
If you want to use any of those options you can simply append them.
See Example section for how to.

... so maybe you will want to close this PR, or remove the preservation of symbolic links and owners from it. To me, it seems that they should be backed up by default and disabled for the edge case (the NAS devices you mentioned)

I wonder if using --numeric-ids would help with the NAS edge case or if it has nothing to do with that?

@kmarty

This comment has been minimized.

Copy link

kmarty commented Feb 14, 2019

That "pedantic" improvement has a reason:
"../" way:

kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ ./timemachine /tmp/src /tmp/dst --archive
kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ ./timemachine /tmp/src /tmp/dst --archive
kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ ./timemachine /tmp/src /tmp/dst --archive
kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ du -sh /tmp/dst/*
7,0M    /tmp/dst/2019-02-14__17-01-46
7,0M    /tmp/dst/2019-02-14__17-01-48
7,0M    /tmp/dst/2019-02-14__17-01-50
  0B    /tmp/dst/current

Whereas "${DEST}/" way:

kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ ./timemachine /tmp/src /tmp/dst --archive
kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ ./timemachine /tmp/src /tmp/dst --archive
kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ ./timemachine /tmp/src /tmp/dst --archive
kmarty@Kmartys-MBP:~/workspace/linux-timemachine$ du -sh /tmp/dst/*
7,0M    /tmp/dst/2019-02-14__17-03-17
  0B    /tmp/dst/2019-02-14__17-03-20
  0B    /tmp/dst/2019-02-14__17-03-22
  0B    /tmp/dst/current

The "../" doesn't work because "../current" doesn't have to be (and it isn't in this case) at "/tmp/dst/current" thus rsync with "--link-dest" is never called.

From the other side, rsync is OK with --link-dest="../current" and that's why I kept "../" in "--link-dest" argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.