duplicity-team team mailing list archive

[Aaron Whitehouse <lists@xxxxxxxxxxxxxxxxxx>]

Aaron Whitehouse has proposed merging lp:~aaron-whitehouse/duplicity/trailing_slash_match_dirs into lp:duplicity.

Requested reviews:
  duplicity-team (duplicity-team)
Related bugs:
  Bug #1479545 in Duplicity: "Globbing patterns with trailing slashes should only match directories"
  https://bugs.launchpad.net/duplicity/+bug/1479545

For more details, see:
https://code.launchpad.net/~aaron-whitehouse/duplicity/trailing_slash_match_dirs/+merge/266527

Made globs with trailing slashes only match directories, not files, fixing Bug #1479545.
-- 
Your team duplicity-team is requested to review the proposed merge of lp:~aaron-whitehouse/duplicity/trailing_slash_match_dirs into lp:duplicity.

=== modified file 'duplicity/selection.py'
--- duplicity/selection.py	2015-07-29 08:35:52 +0000
+++ duplicity/selection.py	2015-07-31 09:21:59 +0000
@@ -473,24 +473,39 @@
 
         """
         # Internal. Used by glob_get_sf and unit tests.
+        match_only_dirs = False
+
+        if filename != "/" and filename[-1] == "/":
+            match_only_dirs = True
+            # Remove trailing / from directory name (unless that is the entire
+            # string)
+            filename = filename[:-1]
+
         if not filename.startswith(self.prefix):
             raise FilePrefixError(filename)
         index = tuple(filter(lambda x: x,
                              filename[len(self.prefix):].split("/")))
-        return self.glob_get_tuple_sf(index, include)
+        return self.glob_get_tuple_sf(index, include, match_only_dirs)
 
-    def glob_get_tuple_sf(self, tuple, include):
+    def glob_get_tuple_sf(self, tuple, include, match_only_dirs=False):
         """Return selection function based on tuple"""
         # Internal. Used by glob_get_filename_sf.
+
         def include_sel_func(path):
-            if (path.index == tuple[:len(path.index)] or
+            if match_only_dirs and not path.isdir():
+                # If the glob ended with a /, only match directories
+                return None
+            elif (path.index == tuple[:len(path.index)] or
                     path.index[:len(tuple)] == tuple):
                 return 1  # /foo/bar implicitly matches /foo, vice-versa
             else:
                 return None
 
         def exclude_sel_func(path):
-            if path.index[:len(tuple)] == tuple:
+            if match_only_dirs and not path.isdir():
+                # If the glob ended with a /, only match directories
+                return None
+            elif path.index[:len(tuple)] == tuple:
                 return 0  # /foo excludes /foo/bar, not vice-versa
             else:
                 return None
@@ -519,8 +534,13 @@
 
         """
         # Internal. Used by glob_get_sf and unit tests.
+
+        match_only_dirs = False
+
         if glob_str != "/" and glob_str[-1] == "/":
-            # Remove trailing / from directory name (unless that is the entire string)
+            match_only_dirs = True
+            # Remove trailing / from directory name (unless that is the entire
+            # string)
             glob_str = glob_str[:-1]
 
         if glob_str.lower().startswith("ignorecase:"):
@@ -539,7 +559,10 @@
                                "|".join(self.glob_get_prefix_res(glob_str)))
 
         def include_sel_func(path):
-            if glob_comp_re.match(path.name):
+            if match_only_dirs and not path.isdir():
+                # If the glob ended with a /, only match directories
+                return None
+            elif glob_comp_re.match(path.name):
                 return 1
             elif scan_comp_re.match(path.name):
                 return 2
@@ -547,7 +570,10 @@
                 return None
 
         def exclude_sel_func(path):
-            if glob_comp_re.match(path.name):
+            if match_only_dirs and not path.isdir():
+                # If the glob ended with a /, only match directories
+                return None
+            elif glob_comp_re.match(path.name):
                 return 0
             else:
                 return None

=== modified file 'po/duplicity.pot'
--- po/duplicity.pot	2015-07-29 10:50:28 +0000
+++ po/duplicity.pot	2015-07-31 09:21:59 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: PACKAGE VERSION\n"
 "Report-Msgid-Bugs-To: Kenneth Loafman <kenneth@xxxxxxxxxxx>\n"
-"POT-Creation-Date: 2015-07-29 11:39+0100\n"
+"POT-Creation-Date: 2015-07-31 09:14+0100\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@xxxxxx>\n"

=== modified file 'testing/functional/test_selection.py'
--- testing/functional/test_selection.py	2015-07-28 17:48:40 +0000
+++ testing/functional/test_selection.py	2015-07-31 09:21:59 +0000
@@ -874,5 +874,56 @@
         restored = self.directory_tree_to_list_of_lists(restore_dir)
         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']])
+
 if __name__ == "__main__":
     unittest.main()

=== modified file 'testing/unit/test_selection.py'
--- testing/unit/test_selection.py	2015-07-29 10:50:28 +0000
+++ testing/unit/test_selection.py	2015-07-31 09:21:59 +0000
@@ -23,10 +23,12 @@
 import types
 import StringIO
 import unittest
+import duplicity.path
 
 from duplicity.selection import *  # @UnusedWildImport
 from duplicity.lazy import *  # @UnusedWildImport
 from . import UnitTestCase
+from mock import patch
 
 
 class MatchingTest(UnitTestCase):
@@ -57,24 +59,28 @@
         self.assertRaises(FilePrefixError, self.Select.glob_get_normal_sf, "foo", 1)
 
         sf2 = self.Select.glob_get_sf("testfiles/select/usr/local/bin/", 1)
-        assert sf2(self.makeext("usr")) == 1
-        assert sf2(self.makeext("usr/local")) == 1
-        assert sf2(self.makeext("usr/local/bin")) == 1
-        assert sf2(self.makeext("usr/local/doc")) is None
-        assert sf2(self.makeext("usr/local/bin/gzip")) == 1
-        assert sf2(self.makeext("usr/local/bingzip")) is None
+
+        with patch('duplicity.path.ROPath.isdir', return_value=True):
+            assert sf2(self.makeext("usr")) == 1
+            assert sf2(self.makeext("usr/local")) == 1
+            assert sf2(self.makeext("usr/local/bin")) == 1
+            assert sf2(self.makeext("usr/local/doc")) is None
+            assert sf2(self.makeext("usr/local/bin/gzip")) == 1
+            assert sf2(self.makeext("usr/local/bingzip")) is None
 
     def test_tuple_exclude(self):
         """Test exclude selection function made from a regular filename"""
         self.assertRaises(FilePrefixError, self.Select.glob_get_normal_sf, "foo", 0)
 
         sf2 = self.Select.glob_get_sf("testfiles/select/usr/local/bin/", 0)
-        assert sf2(self.makeext("usr")) is None
-        assert sf2(self.makeext("usr/local")) is None
-        assert sf2(self.makeext("usr/local/bin")) == 0
-        assert sf2(self.makeext("usr/local/doc")) is None
-        assert sf2(self.makeext("usr/local/bin/gzip")) == 0
-        assert sf2(self.makeext("usr/local/bingzip")) is None
+
+        with patch('duplicity.path.ROPath.isdir', return_value=True):
+            assert sf2(self.makeext("usr")) is None
+            assert sf2(self.makeext("usr/local")) is None
+            assert sf2(self.makeext("usr/local/bin")) == 0
+            assert sf2(self.makeext("usr/local/doc")) is None
+            assert sf2(self.makeext("usr/local/bin/gzip")) == 0
+            assert sf2(self.makeext("usr/local/bingzip")) is None
 
     def test_glob_star_include(self):
         """Test a few globbing patterns, including **"""