classification
Title: Can't add files with spaces
Type: behavior Stage: patch review
Components: Distutils, Distutils2 Versions: Python 3.2, Python 3.1, Python 2.7, 3rd party
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: a.badger, antonio, asandvig, eric.araujo, johnkeyes, r.david.murray, tarek, xiscu
Priority: normal Keywords: easy, patch

Created on 2003-09-19 09:10 by antonio, last changed 2011-01-07 21:02 by xiscu.

Files
File name Uploaded Description Edit
issue809163.diff johnkeyes, 2010-11-20 01:25 Unit test for including files with spaces review
issue809163v2.diff johnkeyes, 2010-11-20 23:19 Now uses TempdirManager review
issue809163v3.diff johnkeyes, 2010-11-21 09:44 Changed assertTrue to assertEquals review
test_bdist_rpm_filename_with_whitespaces.diff xiscu, 2011-01-07 21:02 Unit test for bdist_rpm that triggers the issue
Messages (15)
msg60388 - (view) Author: Antonio Beamud Montero (antonio) Date: 2003-09-19 09:10
If i try to add data files with blank spaces in its
names using a wildcard in the MANIFEST.in file, it
doesn't do the work well and add only the fisrt part of
the name file. For example:
----- MANIFEST.in ----
images/*
-----------------------
The MANIFEST file generated is ok, but when rpm util
try to process it fail... The solution is to scape the
names or quote the names that are generated inside
INSTALLED_FILES

INSTALLED_FILES
running install
running build
running install_data
writing list of installed files to 'INSTALLED_FILES'
+ /usr/lib/rpm/brp-compress
+ /usr/lib/rpm/brp-strip
+ /usr/lib/rpm/brp-strip-comment-note
Processing files: garra-configurator-0.9.12-1
error: Two files on one line:
/usr/local/apache/htdocs/images/default
error: File must begin with "/": logo-tv-1.png

msg111043 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-07-21 13:00
Thanks for the report Antonio. I’m sorry noone replied before; bugs are triaged by volunteers and distutils had no dedicated maintainer in the previous years. This needs a test to make sure the bug still exists, and if confirmed a patch to fix it. I don’t have time right now but it will be done. Thanks again.
msg117685 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-09-30 01:23
Adding the “easy” keyword so that a beginner or sprinter can find this and write a test.  Hint: Forget the part about RPM, write a test about inclusion of file with spaces in their name.
msg121587 - (view) Author: John Keyes (johnkeyes) Date: 2010-11-20 01:25
This is my first contribution as part of the Bug Weekend (and possibly my first to Python).

I tested this by writing a MANIFEST.in and a very small setup.py but after looking at distutils I narrowed the area down to the FileList.

I wrote a unittest and have attached the diff. I tested this on py3k.
msg121591 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-11-20 03:12
Thanks for diagnosis and the test patch, and welcome to the bug weekend.

Some comments:

test.support has a symbol, TESTFN, which is guaranteed to be unique for the test run and located in an appropriate writeable location.  Many tests use it to create a directory via os.mkdir and then create working files in it.  The test should also clean up the directory afterward, which can be done easily using test.support.rmtree.  Using these two test support items should simplify your test.  The other advantage of using TESTFN rather than the tempfile module is that TESTFN locates the file/directory in a place that the buildbot runner cleans up after each test run, so no cruft is left on the buildbot system even if the test runs ends abnormally in the middle of the test.
msg121626 - (view) Author: John Keyes (johnkeyes) Date: 2010-11-20 12:50
I'll change the test to use TESTFN later today. Thanks for the feedback.
msg121628 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-20 13:00
Thanks for the patch John.  Instead of low-level use of TESTFN, you can use distutils.tests.support.TempdirManager to create a temporary directory and write files to it in a few lines.
msg121785 - (view) Author: John Keyes (johnkeyes) Date: 2010-11-20 21:33
I see that distutils.tests.support.TempdirManager uses tempfile for directory creation.

What's the best course of action here? 

1. Use TESTFN or
2. Use TempdirManager or
3. Change the test to use TempdirManager and change TempdirManager to use TESTFN instead of tempfile.

Thanks.
msg121790 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-20 22:01
TempdirManager hasn’t caused buildbots failures so far, so there is no need to change it.  You can use it, it’s convenient and works :)
msg121811 - (view) Author: John Keyes (johnkeyes) Date: 2010-11-20 23:19
This patch uses TempdirManager to handle the temporary directory and file creation.
msg121816 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2010-11-20 23:56
TempdirManager is new, which is why I forgot about it.  It works with the buildbots because it is a context manager, so if things go badly the cleanup code is still going to run and delete the cruft.  Since the distutils tests are unlikely to hang (which is the only time the cleanup code would not get run), it should be perfectly fine.
msg121857 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-21 03:56
Looks good.  You’ll want to use assertEqual here:
    self.assertTrue(1, len(file_list.files))

Do you want to patch filelist.py now?
msg121892 - (view) Author: John Keyes (johnkeyes) Date: 2010-11-21 09:44
Gah! Silly mistake with the assert.

What filelist.py patch are you referring to? I've tested this on py3k with no changes to filelist.py.

Thanks for your patience.
msg122021 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-11-21 23:34
assertEquals is deprecated, I’ll change to assertEqual.

> What filelist.py patch are you referring to? I've tested this on py3k
> with no changes to filelist.py.
The goal of a test is to show an error so that it can be fixed.  In this case, the test is still useful to make sure things work, especially in distutils2 where we are changing the filelist (now manifest) module a lot.

I think now that my comment “forget RPM” was wrong.  This is not a problem with RPM, but with bdist_rpm, so even if filelist behaves correctly with spaces, bdist_rpm does not.  If there is a way to make rpm read one line at a time, we should use it.

Toshio: Do you have an idea how to do that?  Otherwise we’ll close as wontfix and forward to the bdist_rpm2 project, where they have more freedom to change code.
msg125709 - (view) Author: xiscu (xiscu) Date: 2011-01-07 21:02
This is also my first contribution.

The attached file is a diff against :
- http://code.python.org/hg/branches/py3k/

The actual tip was:
- 9489:4db13b4e76aa

The patch adds only a test that triggers the issue.

Please feel free to comment on that commit (so I can improve)

Thanks
History
Date User Action Args
2011-01-07 21:02:28xiscusetfiles: + test_bdist_rpm_filename_with_whitespaces.diff
nosy: + xiscu
messages: + msg125709

2010-11-21 23:34:57eric.araujosetnosy: + a.badger
messages: + msg122021
2010-11-21 09:44:48johnkeyessetfiles: + issue809163v3.diff

messages: + msg121892
2010-11-21 03:56:03eric.araujosetmessages: + msg121857
2010-11-20 23:56:34r.david.murraysetmessages: + msg121816
2010-11-20 23:19:28johnkeyessetfiles: + issue809163v2.diff

messages: + msg121811
2010-11-20 22:01:14eric.araujosetmessages: + msg121790
2010-11-20 21:33:47johnkeyessetmessages: + msg121785
2010-11-20 13:00:17eric.araujosetassignee: tarek -> eric.araujo
messages: + msg121628
stage: needs patch -> patch review
2010-11-20 12:50:27johnkeyessetmessages: + msg121626
2010-11-20 03:12:03r.david.murraysetnosy: + r.david.murray

messages: + msg121591
stage: test needed -> needs patch
2010-11-20 01:25:53johnkeyessetfiles: + issue809163.diff

nosy: + johnkeyes
messages: + msg121587

keywords: + patch
2010-10-07 09:51:00asandvigsetnosy: + asandvig
2010-09-30 01:23:23eric.araujosetkeywords: + easy

messages: + msg117685
versions: + 3rd party, Python 3.1, Python 2.7, Python 3.2
2010-07-21 13:00:24eric.araujosetassignee: tarek
components: + Distutils2
versions: - Python 2.6, Python 3.0, Python 3.1, Python 2.7
nosy: + eric.araujo, tarek

messages: + msg111043
stage: test needed
2009-02-04 16:45:40akitadasettype: behavior
versions: + Python 2.6, Python 3.0, Python 3.1, Python 2.7, - Python 2.3
2003-09-19 09:10:19antoniocreate