diff --git a/pbr/hooks/files.py b/pbr/hooks/files.py index 750ac32c..0fe0df55 100644 --- a/pbr/hooks/files.py +++ b/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) - finished.append("%s = " % new_prefix) + new_prefix = dirpath.replace(unquoted_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() diff --git a/pbr/tests/test_files.py b/pbr/tests/test_files.py index ed67f7bc..94a2d9ad 100644 --- a/pbr/tests/test_files.py +++ b/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/ = ' - '\nshare/ansible/kolla-ansible = ' - '\nshare/ansible/kolla-ansible/test = ' - '\n ansible/kolla-ansible/test/baz', + "\n'share/ansible/' = " + "\n'share/ansible/kolla-ansible' = " + "\n'share/ansible/kolla-ansible/test' = " + "\n 'ansible/kolla-ansible/test/baz'", config['files']['data_files']) diff --git a/pbr/tests/test_util.py b/pbr/tests/test_util.py index 6814ac7b..393bc5cd 100644 --- a/pbr/tests/test_util.py +++ b/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'])) diff --git a/pbr/util.py b/pbr/util.py index 55d73f8b..e931b5ff 100644 --- a/pbr/util.py +++ b/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 diff --git a/releasenotes/notes/fix-handling-of-spaces-in-data-files-glob-0fe0c398d70dfea8.yaml b/releasenotes/notes/fix-handling-of-spaces-in-data-files-glob-0fe0c398d70dfea8.yaml new file mode 100644 index 00000000..793ba73b --- /dev/null +++ b/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 + `_ for more details.