Merge "Fix white space handling in file names"

pbr/hooks/files.py

@@ -14,6 +14,7 @@
 # under the License.
 
 import os
+import shlex
 import sys
 
 from pbr import find_package
@@ -35,6 +36,14 @@ def get_man_section(section):
     return os.path.join(get_manpath(), 'man%s' % section)
 
 
+def unquote_path(path):
+    # unquote the full path, e.g: "'a/full/path'" becomes "a/full/path", also
+    # strip the quotes off individual path components because os.walk cannot
+    # handle paths like: "'i like spaces'/'another dir'", so we will pass it
+    # "i like spaces/another dir" instead.
+    return "".join(shlex.split(path))
+
+
 class FilesConfig(base.BaseConfig):
 
     section = 'files'
@@ -57,25 +66,28 @@ class FilesConfig(base.BaseConfig):
                 target = target.strip()
                 if not target.endswith(os.path.sep):
                     target += os.path.sep
-                for (dirpath, dirnames, fnames) in os.walk(source_prefix):
+                unquoted_prefix = unquote_path(source_prefix)
+                unquoted_target = unquote_path(target)
+                for (dirpath, dirnames, fnames) in os.walk(unquoted_prefix):
                     # As source_prefix is always matched, using replace with a
                     # a limit of one is always going to replace the path prefix
                     # and not accidentally replace some text in the middle of
                     # the path
-                    new_prefix = dirpath.replace(source_prefix, target, 1)
+                    new_prefix = dirpath.replace(unquoted_prefix,
-                    finished.append("%s = " % new_prefix)
+                                                 unquoted_target, 1)
+                    finished.append("'%s' = " % new_prefix)
                     finished.extend(
-                        [" %s" % os.path.join(dirpath, f) for f in fnames])
+                        [" '%s'" % os.path.join(dirpath, f) for f in fnames])
             else:
                 finished.append(line)
 
         self.data_files = "\n".join(finished)
 
     def add_man_path(self, man_path):
-        self.data_files = "%s\n%s =" % (self.data_files, man_path)
+        self.data_files = "%s\n'%s' =" % (self.data_files, man_path)
 
     def add_man_page(self, man_page):
-        self.data_files = "%s\n  %s" % (self.data_files, man_page)
+        self.data_files = "%s\n  '%s'" % (self.data_files, man_page)
 
     def get_man_sections(self):
         man_sections = dict()

pbr/tests/test_files.py

@@ -37,12 +37,17 @@ class FilesConfigTest(base.BaseTestCase):
         pkg_etc = os.path.join(pkg_fixture.base, 'etc')
         pkg_ansible = os.path.join(pkg_fixture.base, 'ansible',
                                    'kolla-ansible', 'test')
+        dir_spcs = os.path.join(pkg_fixture.base, 'dir with space')
+        dir_subdir_spc = os.path.join(pkg_fixture.base, 'multi space',
+                                      'more spaces')
         pkg_sub = os.path.join(pkg_etc, 'sub')
         subpackage = os.path.join(
             pkg_fixture.base, 'fake_package', 'subpackage')
         os.makedirs(pkg_sub)
         os.makedirs(subpackage)
         os.makedirs(pkg_ansible)
+        os.makedirs(dir_spcs)
+        os.makedirs(dir_subdir_spc)
         with open(os.path.join(pkg_etc, "foo"), 'w') as foo_file:
             foo_file.write("Foo Data")
         with open(os.path.join(pkg_sub, "bar"), 'w') as foo_file:
@@ -51,6 +56,10 @@ class FilesConfigTest(base.BaseTestCase):
             baz_file.write("Baz Data")
         with open(os.path.join(subpackage, "__init__.py"), 'w') as foo_file:
             foo_file.write("# empty")
+        with open(os.path.join(dir_spcs, "file with spc"), 'w') as spc_file:
+            spc_file.write("# empty")
+        with open(os.path.join(dir_subdir_spc, "file with spc"), 'w') as file_:
+            file_.write("# empty")
 
         self.useFixture(base.DiveDir(pkg_fixture.base))
 
@@ -79,9 +88,49 @@ class FilesConfigTest(base.BaseTestCase):
         )
         files.FilesConfig(config, 'fake_package').run()
         self.assertIn(
-            '\netc/pbr/ = \n etc/foo\netc/pbr/sub = \n etc/sub/bar',
+            "\n'etc/pbr/' = \n 'etc/foo'\n'etc/pbr/sub' = \n 'etc/sub/bar'",
             config['files']['data_files'])
 
+    def test_data_files_with_spaces(self):
+        config = dict(
+            files=dict(
+                data_files="\n  'i like spaces' = 'dir with space'/*"
+            )
+        )
+        files.FilesConfig(config, 'fake_package').run()
+        self.assertIn(
+            "\n'i like spaces/' = \n 'dir with space/file with spc'",
+            config['files']['data_files'])
+
+    def test_data_files_with_spaces_subdirectories(self):
+        # test that we can handle whitespace in subdirectories
+        data_files = "\n 'one space/two space' = 'multi space/more spaces'/*"
+        expected = (
+            "\n'one space/two space/' = "
+            "\n 'multi space/more spaces/file with spc'")
+        config = dict(
+            files=dict(
+                data_files=data_files
+            )
+        )
+        files.FilesConfig(config, 'fake_package').run()
+        self.assertIn(expected, config['files']['data_files'])
+
+    def test_data_files_with_spaces_quoted_components(self):
+        # test that we can quote individual path components
+        data_files = (
+            "\n'one space'/'two space' = 'multi space'/'more spaces'/*"
+        )
+        expected = ("\n'one space/two space/' = "
+                    "\n 'multi space/more spaces/file with spc'")
+        config = dict(
+            files=dict(
+                data_files=data_files
+            )
+        )
+        files.FilesConfig(config, 'fake_package').run()
+        self.assertIn(expected, config['files']['data_files'])
+
     def test_data_files_globbing_source_prefix_in_directory_name(self):
         # We want to test that the string, "docs", is not replaced in a
         # subdirectory name, "sub-docs"
@@ -92,8 +141,8 @@ class FilesConfigTest(base.BaseTestCase):
         )
         files.FilesConfig(config, 'fake_package').run()
         self.assertIn(
-            '\nshare/ansible/ = '
+            "\n'share/ansible/' = "
-            '\nshare/ansible/kolla-ansible = '
+            "\n'share/ansible/kolla-ansible' = "
-            '\nshare/ansible/kolla-ansible/test = '
+            "\n'share/ansible/kolla-ansible/test' = "
-            '\n ansible/kolla-ansible/test/baz',
+            "\n 'ansible/kolla-ansible/test/baz'",
             config['files']['data_files'])

pbr/tests/test_util.py

@@ -172,3 +172,28 @@ class TestProvidesExtras(base.BaseTestCase):
         config = config_from_ini(ini)
         kwargs = util.setup_cfg_to_setup_kwargs(config)
         self.assertEqual(['foo', 'bar'], kwargs['provides_extras'])
+
+
+class TestDataFilesParsing(base.BaseTestCase):
+
+    scenarios = [
+        ('data_files', {
+            'config_text': """
+            [files]
+            data_files =
+                'i like spaces/' =
+                    'dir with space/file with spc 2'
+                    'dir with space/file with spc 1'
+            """,
+            'data_files': [
+                ('i like spaces/', ['dir with space/file with spc 2',
+                                    'dir with space/file with spc 1'])
+            ]
+        })]
+
+    def test_handling_of_whitespace_in_data_files(self):
+        config = config_from_ini(self.config_text)
+        kwargs = util.setup_cfg_to_setup_kwargs(config)
+
+        self.assertEqual(self.data_files,
+                         list(kwargs['data_files']))

pbr/util.py

@@ -64,6 +64,7 @@ import logging  # noqa
 from collections import defaultdict
 import os
 import re
+import shlex
 import sys
 import traceback
 
@@ -372,21 +373,22 @@ def setup_cfg_to_setup_kwargs(config, script_args=()):
                 for line in in_cfg_value:
                     if '=' in line:
                         key, value = line.split('=', 1)
-                        key, value = (key.strip(), value.strip())
+                        key_unquoted = shlex.split(key.strip())[0]
+                        key, value = (key_unquoted, value.strip())
                         if key in data_files:
                             # Multiple duplicates of the same package name;
                             # this is for backwards compatibility of the old
                             # format prior to d2to1 0.2.6.
                             prev = data_files[key]
-                            prev.extend(value.split())
+                            prev.extend(shlex.split(value))
                         else:
-                            prev = data_files[key.strip()] = value.split()
+                            prev = data_files[key.strip()] = shlex.split(value)
                     elif firstline:
                         raise errors.DistutilsOptionError(
                             'malformed package_data first line %r (misses '
                             '"=")' % line)
                     else:
-                        prev.extend(line.strip().split())
+                        prev.extend(shlex.split(line.strip()))
                     firstline = False
                 if arg == 'data_files':
                     # the data_files value is a pointlessly different structure

releasenotes/notes/fix-handling-of-spaces-in-data-files-glob-0fe0c398d70dfea8.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes the handling of spaces in data_files globs. Please see `bug 1810934
+    <https://bugs.launchpad.net/pbr/+bug/1810934>`_ for more details.