Make v1.1.x style permission handling configurable via expert settings.

[b3nmore]

No description provided.

[buhtz]

Why do we need this PR? Dear @b3nmore please provide more details.

I think there was a reason why the current (1.2) version does handle the permission different then the old version (1.1.x). I do not know the reason but maybe @Germar does?

[b3nmore]

Why do we need this PR? Dear @b3nmore please provide more details.

When upgrading from 1.1.x to 1.2 incremental backup does not work for the first backup, basically due to a change how rsync is called in 1.2. That means, that even if there is only one single file change in respect to the last (1.1.x) snapshot, 1.2 will make a full backup of all files. And that means it takes very long for a new snapshot and it will need nearly twice the disc space. With this PR you can continue to use your old snapshots for incremental backups even with 1.2.

I think there was a reason why the current (1.2) version does handle the permission different then the old version (1.1.x). I do not know the reason but maybe @Germar does?

If I remember correctly there something about the reasoning in the referenced issue.

[ghost]

I uploaded a dpkg-repack'ed version of backintime with @b3nmore patch here, in case it is of help to someone.

[ghost]

@b3nmore Thanks again for the patch. There seems so be at least two more lines that need to be changed in common/snapshots.py.

Line 447 cmd_prefix = tools.rsyncPrefix(self.config, no_perms = False, use_mode = ['ssh'])
Line 848 cmd = tools.rsyncPrefix(self.config, no_perms = False)

I removed the "no_perms = False" part of both and am doing some testing.

[ghost]

There seems so be at least two more lines that need to be changed in common/snapshots.py.

Line 447 cmd_prefix = tools.rsyncPrefix(self.config, no_perms = False, use_mode = ['ssh'])
Line 848 cmd = tools.rsyncPrefix(self.config, no_perms = False)

[ghost]

Why do we need this PR? Dear @b3nmore please provide more details.

When upgrading from 1.1.x to 1.2 incremental backup does not work for the first backup [...]

From my point of view, the real reason for this PR is that the new permission handling is not what most people would want, since it leads to backing up files that have not changed, if only their permissions have changed. Not recognizing older backups when upgrading from 1.1.x to 1.2 is only a side effect of this.

Again, from my point of view, the new way of handling permissions should be completely removed from BiT. No one should want to create a new backup of a file only because its permissions have changed.

[ghost]

I created the PR #1128 which removes the new permissions handling without adding any config option.

[aryoda]

The charm of this PR is that the user can configure the --perms/--no-perms behavior in the settings GUI (which this PR extends) while in PR #1128 the new permissions handling will be removed again.

[aryoda]

I have just pulled the PR into a branch of my forked BiT.

The changed settings dialog looks like this:

The merge breaks at some unit tests that do not expect "no_perms" and needed additional work to fix/adjust the expected results of the affected unit tests:

$ python3 -m unittest  -b test/test_backintime.py
======================================================================
FAIL: test_local_snapshot_is_successful (test.test_backintime.TestBackInTime)
end to end test - from BIT initialization through snapshot
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/xxx/dev/backintime/common/test/test_backintime.py", line 153, in test_local_snapshot_is_successful
...
    cmd_prefix = tools.rsyncPrefix(self.config, no_perms = False, use_mode = ['ssh'])
TypeError: rsyncPrefix() got an unexpected keyword argument 'no_perms'
stdout: 
Back In Time
Version: 1.3.2 git branch '1086_perms_settings_GUI' hash '2186964'

As discussed in #1128 I do put this on hold until we can decide about the new permission system in general...

[buhtz]

Dear @b3nmore ,

my apologize. I set this to "Draft" because it is outdated which is not your fault.

Your contribution won't disappear. We will take this into account while investigating and fixing #988.

Kind
Christian

[buhtz]

I appreciate the work. But I vote to close the PR because this seems not to be the long term solution for the underlying problem.

The two major issues relevant here are #994 and #988 . The problem behind it is much bigger and need further investigation. The root of the problem is not even clear. I'm not convinced that that "v1.1.x style permission handling" is the right way compared to the current "1.2 style". And I won't try to support two different ways of style permission handling.

Myself I need much deeper understanding of how rsync handle things like this and how BIT do use rsync here. This will be ground work.

[emtiu]

I agree with @buhtz here. I hope to see the day when we finally kill #994 and #988!

[aryoda]

If we should ever revive this closed PR:

The code changes would also require to implement the fix for #994 which is (as known so far) triggered by rsync --delete causing to change permissions of read-only files under some circumstances...