duplicity-team team mailing list archive

[Aaron Whitehouse <lists@xxxxxxxxxxxxxxxxxx>]

Aaron Whitehouse has proposed merging lp:~aaron-whitehouse/duplicity/bug_1620085_exclude-if-present-locked-folder into lp:duplicity/0.7-series.

Requested reviews:
  duplicity-team (duplicity-team)
Related bugs:
  Bug #1620085 in Duplicity: "--exclude-if-present gives OSError looking for tag in locked folders"
  https://bugs.launchpad.net/duplicity/+bug/1620085

For more details, see:
https://code.launchpad.net/~aaron-whitehouse/duplicity/bug_1620085_exclude-if-present-locked-folder/+merge/304872

* Fixed bug #1620085: OSError when using --exclude-if-present with a locked directory in backup path.
* Added tests for errors on locked files.
* Add functional tests for --exclude-if-present.
-- 
Your team duplicity-team is requested to review the proposed merge of lp:~aaron-whitehouse/duplicity/bug_1620085_exclude-if-present-locked-folder into lp:duplicity/0.7-series.

=== modified file 'duplicity/selection.py'
--- duplicity/selection.py	2016-08-12 20:50:29 +0000
+++ duplicity/selection.py	2016-09-04 22:28:47 +0000
@@ -447,10 +447,24 @@
 
         def exclude_sel_func(path):
             # do not follow symbolic links when checking for file existence!
-            if path.isdir() and path.append(filename).exists():
-                return 0
-            else:
-                return None
+            if path.isdir():
+                # First check path is read accessible
+                if not (os.access(path.name, os.R_OK)):
+                    # Path is not read accessible
+                    # ToDo: Ideally this error would only show if the folder
+                    # was ultimately included by the full set of selection
+                    # functions. Currently this will give an error for any
+                    # locked directory within the folder being backed up.
+                    log.Warn(_(
+                        "Error accessing possibly locked file %s") % util.ufn(
+                        path.name),
+                        log.WarningCode.cannot_read, util.escape(path.name))
+                    if diffdir.stats:
+                        diffdir.stats.Errors += 1
+                elif path.append(filename).exists():
+                    return 0
+                else:
+                    return None
 
         if include == 0:
             sel_func = exclude_sel_func

=== modified file 'testing/functional/test_selection.py'
--- testing/functional/test_selection.py	2015-07-31 08:22:31 +0000
+++ testing/functional/test_selection.py	2016-09-04 22:28:47 +0000
@@ -837,6 +837,57 @@
         self.restore_and_check()
 
 
+class TestTrailingSlash2(IncludeExcludeFunctionalTest):
+    """ This tests the behaviour of globbing strings with a trailing slash"""
+    # See Bug #1479545 (https://bugs.launchpad.net/duplicity/+bug/1479545)
+
+    def test_no_trailing_slash(self):
+        """ Test that including 1.py works as expected"""
+        self.backup("full", "testfiles/select2",
+                    options=["--include", "testfiles/select2/1.py",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [['1.py']])
+
+    def test_trailing_slash(self):
+        """ Test that globs with a trailing slash only match directories"""
+        # Regression test for Bug #1479545
+        # (https://bugs.launchpad.net/duplicity/+bug/1479545)
+        self.backup("full", "testfiles/select2",
+                    options=["--include", "testfiles/select2/1.py/",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [])
+
+    def test_include_files_not_subdirectories(self):
+        """ Test that a trailing slash glob followed by a * glob only matches
+        files and not subdirectories"""
+        self.backup("full", "testfiles/select2",
+                    options=["--exclude", "testfiles/select2/*/",
+                             "--include", "testfiles/select2/*",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [['1.doc', '1.py']])
+
+    def test_include_subdirectories_not_files(self):
+        """ Test that a trailing slash glob only matches directories"""
+        self.backup("full", "testfiles/select2",
+                    options=["--include", "testfiles/select2/1/1sub1/**/",
+                             "--exclude", "testfiles/select2/1/1sub1/**",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [['1'], ['1sub1'],
+                                    ['1sub1sub1', '1sub1sub2', '1sub1sub3']])
+
+
 class TestGlobbingReplacement(IncludeExcludeFunctionalTest):
     """ This tests the behaviour of the extended shell globbing pattern replacement functions."""
     # See the manual for a description of behaviours, but in summary:
@@ -875,55 +926,85 @@
         self.assertEqual(restored, self.expected_restored_tree)
 
 
-class TestTrailingSlash(IncludeExcludeFunctionalTest):
-    """ This tests the behaviour of globbing strings with a trailing slash"""
-    # See Bug #1479545 (https://bugs.launchpad.net/duplicity/+bug/1479545)
-
-    def test_no_trailing_slash(self):
-        """ Test that including 1.py works as expected"""
-        self.backup("full", "testfiles/select2",
-                    options=["--include", "testfiles/select2/1.py",
-                             "--exclude", "**"])
-        self.restore()
-        restore_dir = 'testfiles/restore_out'
-        restored = self.directory_tree_to_list_of_lists(restore_dir)
-        self.assertEqual(restored, [['1.py']])
-
-    def test_trailing_slash(self):
-        """ Test that globs with a trailing slash only match directories"""
-        # ToDo: Bug #1479545
-        # (https://bugs.launchpad.net/duplicity/+bug/1479545)
-        self.backup("full", "testfiles/select2",
-                    options=["--include", "testfiles/select2/1.py/",
-                             "--exclude", "**"])
-        self.restore()
-        restore_dir = 'testfiles/restore_out'
-        restored = self.directory_tree_to_list_of_lists(restore_dir)
-        self.assertEqual(restored, [])
-
-    def test_include_files_not_subdirectories(self):
-        """ Test that a trailing slash glob followed by a * glob only matches
-        files and not subdirectories"""
-        self.backup("full", "testfiles/select2",
-                    options=["--exclude", "testfiles/select2/*/",
-                             "--include", "testfiles/select2/*",
-                             "--exclude", "**"])
-        self.restore()
-        restore_dir = 'testfiles/restore_out'
-        restored = self.directory_tree_to_list_of_lists(restore_dir)
-        self.assertEqual(restored, [['1.doc', '1.py']])
-
-    def test_include_subdirectories_not_files(self):
-        """ Test that a trailing slash glob only matches directories"""
-        self.backup("full", "testfiles/select2",
-                    options=["--include", "testfiles/select2/1/1sub1/**/",
-                             "--exclude", "testfiles/select2/1/1sub1/**",
-                             "--exclude", "**"])
-        self.restore()
-        restore_dir = 'testfiles/restore_out'
-        restored = self.directory_tree_to_list_of_lists(restore_dir)
-        self.assertEqual(restored, [['1'], ['1sub1'],
-                                     ['1sub1sub1', '1sub1sub2', '1sub1sub3']])
+class TestExcludeIfPresent(IncludeExcludeFunctionalTest):
+    """ This tests the behaviour of duplicity's --exclude-if-present option"""
+
+    def test_exclude_if_present_baseline(self):
+        """ Test that duplicity normally backs up files"""
+        with open("testfiles/select2/1/1sub1/1sub1sub1/.nobackup", "w") as tag:
+            tag.write("Files in this folder should not be backed up.")
+        self.backup("full", "testfiles/select2/1/1sub1",
+                    options=["--include", "testfiles/select2/1/1sub1/1sub1sub1/*",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [['1sub1sub1'],
+                                    ['.nobackup', '1sub1sub1_file.txt']])
+
+    def test_exclude_if_present_excludes(self):
+        """ Test that duplicity excludes files with relevant tag"""
+        with open("testfiles/select2/1/1sub1/1sub1sub1/.nobackup", "w") as tag:
+            tag.write("Files in this folder should not be backed up.")
+        self.backup("full", "testfiles/select2/1/1sub1",
+                    options=["--exclude-if-present", ".nobackup",
+                             "--include", "testfiles/select2/1/1sub1/1sub1sub1/*",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [])
+
+    def test_exclude_if_present_excludes_2(self):
+        """ Test that duplicity excludes files with relevant tag"""
+        with open("testfiles/select2/1/1sub1/1sub1sub1/EXCLUDE.tag", "w") as tag:
+            tag.write("Files in this folder should also not be backed up.")
+        self.backup("full", "testfiles/select2/1/1sub1",
+                    options=["--exclude-if-present", "EXCLUDE.tag",
+                             "--include", "testfiles/select2/1/1sub1/1sub1sub1/*",
+                             "--exclude", "**"])
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [])
+
+
+class TestLockedFoldersNoError(IncludeExcludeFunctionalTest):
+    """ This tests that inaccessible folders do not cause an error"""
+
+    def test_locked_baseline(self):
+        """ Test no error if locked in path but excluded"""
+        folder_to_lock = "testfiles/select2/1/1sub1/1sub1sub3"
+        initial_mode = os.stat(folder_to_lock).st_mode
+        os.chmod(folder_to_lock, 0o0000)
+        self.backup("full", "testfiles/select2/1/1sub1",
+                    options=["--include", "testfiles/select2/1/1sub1/1sub1sub1/*",
+                             "--exclude", "**"])
+        os.chmod(folder_to_lock, initial_mode)
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [['1sub1sub1'],
+                                    ['1sub1sub1_file.txt']])
+
+    def test_locked_excl_if_present(self):
+        """ Test no error if excluded locked with --exclude-if-present"""
+        # Regression test for Bug #1620085
+        # https://bugs.launchpad.net/duplicity/+bug/1620085
+        folder_to_lock = "testfiles/select2/1/1sub1/1sub1sub3"
+        initial_mode = os.stat(folder_to_lock).st_mode
+        os.chmod(folder_to_lock, 0o0000)
+        self.backup("full", "testfiles/select2/1/1sub1",
+                    options=["--exclude-if-present", "EXCLUDE.tag",
+                             "--include", "testfiles/select2/1/1sub1/1sub1sub1/*",
+                             "--exclude", "**"])
+        os.chmod(folder_to_lock, initial_mode)
+        self.restore()
+        restore_dir = 'testfiles/restore_out'
+        restored = self.directory_tree_to_list_of_lists(restore_dir)
+        self.assertEqual(restored, [['1sub1sub1'],
+                                    ['1sub1sub1_file.txt']])
+
 
 if __name__ == "__main__":
     unittest.main()