openstack
/
pbr
Code Issues Proposed changes

Fix white space handling in file names 

Previously, when using data_files with a glob that matched a file with
whitespace in the name, pip would error with a message that the file
does not exist, e.g:

    error: can't copy 'ansible/roles/yatesr.timezone/templates/timezone-Arch': doesn't exist or not a regular file

The problem was that ansible/roles/yatesr.timezone/templates/timezone-Arch
was a truncated form of the actual filename:

    ansible/roles/yatesr.timezone/templates/timezone-Arch Linux.j2

Note the space in the filename and that it has been split on this space.

This change allows you to use a glob that matches files with whitespace
in the name. It does this by quoting the path.

Change-Id: Id2cc32e4e40c1f834b19756e922118d8526358d3
Fixes-Bug: 1810934
This commit is contained in:
Will Szumski 2019-01-08 12:13:56 +00:00
parent 55429ef856
commit 50356da839
5 changed files with 108 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
pbr/hooks/files.py
View File

@ -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()

59
pbr/tests/test_files.py
View File

@ -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'])

25
pbr/tests/test_util.py
View File

@ -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']))

10
pbr/util.py
View File

@ -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

5
releasenotes/notes/fix-handling-of-spaces-in-data-files-glob-0fe0c398d70dfea8.yaml Normal file
View File

@ -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.