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

Add "no follow links" preference #564

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

Conversation

presto8
Copy link

@presto8 presto8 commented May 20, 2019

I am not sure the history or design philosophy behind the current symlink behavior. However, for my application, I never want symlinks to be followed. In case it is helpful to others, I created this commit which implements a "no_follow_links" preference setting. For now, it is simply true or false. True means to never follow symlinks. False means to use the default symlink behavior (follow top-level links only I think). This could potentially be expanded to be a more generic preference, such as "follow_symlinks" with values of "never", "always", "top-level", etc.

Add a new preference "no_follow_links" which can be set in
.duplicacy/preferences to override the default symlink behavior.

If the preference is not set or is set to false, then the symlink
behavior is the same as before this commit.

If the preference is set to true, then symlinks are never followed. The
symlink is stored as a regular file in the backup.

Add a new preference "no_follow_links" which can be set in
.duplicacy/preferences to override the default symlink behavior.

If the preference is not set or is set to false, then the symlink
behavior is the same as before this commit.

If the preference is set to true, then symlinks are never followed. The
symlink is stored as a regular file in the backup.
@CLAassistant
Copy link

CLAassistant commented May 20, 2019

CLA assistant check
All committers have signed the CLA.

@TheBestPessimist
Copy link
Contributor

TheBestPessimist commented May 20, 2019

I gotta say i have mixed feelings about what this PR wants to change. I think i was never in the situation where the root level symlink following caused problems.

@presto8
Copy link
Author

presto8 commented May 20, 2019

Would you be able to share the situation where it is good to automatically follow symlinks? Most software I have encountered does not follow symlinks by default (e.g., rsync, duplicity, borg, ...).

Ultimately, this change is not essential, users can always use the filters to exclude any symlinks that they don't want backed up. In my case, I was surprised when my backup start being several terabytes instead of a few hundred MB due to a symlink being followed off filesystem, hence I thought I would submit the patch. If it is not useful or contrary to the intended design direction, please disregard :-)

@TheBestPessimist
Copy link
Contributor

TheBestPessimist commented May 22, 2019

Following the top-level symlinks is a feature I always (ab)use by having a symlink repository instead of having multiple separate repositories.

@presto8
Copy link
Author

presto8 commented May 24, 2019

Thanks for the link to the symlink repo. I just tried it out now, and I like it! Basically I set up a directory and link to all the directories that I want to from it. If this is the preferred method of operation, then perhaps it could be added to the Quickstart Guide? The reason I ran into trouble is I followed the guide and did it from my home directory, which has a lot of symlinks.

I still think it's a good idea to give the user extra control over symlinks; on the other hand, more features is more code to maintain and document. I will leave it in the maintainers capable hands to decide the fate of this one way or the other... in the meantime, I am enjoying duplicacy. As a long-time Crashplan and Borg user, I really enjoy Duplicacy!

@TheBestPessimist
Copy link
Contributor

If this is the preferred method of operation

I don't think it is.
It's just a feature that I personally love and use, hence my reasoning about not inserting 1 more flag since it's (imo as a windows user who doesn't have symlinks everywhere) a rather minor use case.

@presto8
Copy link
Author

presto8 commented May 24, 2019

Ah, interesting. Yes, I think Windows users use a lot fewer symlinks than Linux users do.

Is the concern over adding one more parameter to every method in the go code? Or one more line in the preferences file? If the later, since the default is to maintain the previous behavior if not present or set to false, then the parameter can just be left out completely...

For the go code, it was fairly invasive to add this one additional parameter. It may be a good idea to consider refactoring the code at some point to pass a single preferences object instead of individual ones...

@TheBestPessimist
Copy link
Contributor

I guess we should see what gchen wants>.

Copy link

@Ati59 Ati59 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add your option in the app.Commands of the main() function.
Around line 1784, flags are defined.
It fixes a bug in your PR using the "set" command that results with this error :
interface conversion: interface {} is nil, not *main.TriBool

@callegar
Copy link

May I suggest that the rather peculiar behavior wrt symlinks is documented in the guide? What happens if the top level includes a symlink to some area outside of it to which the user has permission to read but not to write to? Will restore proceed gracefully when it cannot write to that space? Will a symlink to itself or to the repo root fill the storage or will the cycle be detected?

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.

5 participants