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

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.

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.

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.

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.

I'll change the test to use TESTFN later today. Thanks for the feedback.

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.

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.

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 :)

This patch uses TempdirManager to handle the temporary directory and file creation.

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.

Looks good.  You’ll want to use assertEqual here:
    self.assertTrue(1, len(file_list.files))

Do you want to patch filelist.py now?

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.

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.

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

Is this issue still going on?

Yes, this issue is not addressed.  A test is added by the latest patch and reproduces the issue; now bdist_rpm should be changed to make the test pass.  See also my previous comment.

I tried to apply the last patch but it returned me and error of failing hunk. I think it was based on an old version of the test_bdist_rpm.py file.

Hence, I made this updated version of the patch and could get the expected failure during the tests.

I should try and solve this bug soon.

As far as I noticed, in bdist_rpm.py, the _make_spec_file() method generates an .spec file for RPM build and attaches the install script to "setup.py install -O1 --root=$RPM_BUILD_ROOT --record=INSTALLED_FILES".

Later, the .spec refers to the record file as the one which contains all necessary files to build the RPM package with the directive "%files -f INSTALLED_FILES". If any path contains an space character, it will triggers an error.

I could fix this problem by attaching to the installation a small sed script that appends double quotes to the beginning and the end of each line of INSTALLED_FILES. I'm not sure whether this is allowed though. Any ideas how to make this more Pythonic without using sed?

Apparently, it works since the test passed. However, there is an warning which I have no idea what it means: "unknown, 0: Warning: using regular magic file `/etc/magic'".

I attached the diff file with the changes so far.

Previous comment: “If there is a way to make rpm read one line at a time, we should use it.”

If there isn’t such an option, and if sed is guaranteed to be installed on rpm-based systems, we can try the sed script.

In issue21153, I explored this problem as well, before being aware of this issue. I had searched for bdist_rpm.

I've since confirmed that the sed approach is viable. For example, the following works around the issue by overriding the install_script with the sed technique:

$ cat > install.txt
python setup.py install -O1 --root=$RPM_BUILD_ROOT --record=INSTALLED_FILES
sed -i -e 's/.*/"\0"/' INSTALLED_FILES

$ python setup.py bdist_rpm --install-script install.txt

I agree that sed can probably be assumed to be present, though it would be nicer not to depend on it.

It may also be possible not to have to list the files explicitly. In reading the rpm docs (http://www.rpm.org/max-rpm/s1-rpm-inside-files-list-directives.html), I discovered that rpm will traverse a directory, adding it and its contents. Perhaps the whole routine of recording installed files and then listing them explicitly can be replaced by simply referencing the build root.

I decided to test my proposition. It seems that simply specifying / for %files leads to the RPM including parent directories. Here's the output I get when running my test with the sed workaround:

$ rpm -q -filesbypkg -p dist/issue21153-package-0.0.0-1.noarch.rpm
issue21153-package        /usr/local/lib/python2.7/dist-packages/issue21153_package-0.0.0.egg-info
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/__init__.py
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/__init__.pyc
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/__init__.pyo
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/file with spaces.py
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/file with spaces.pyc
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/file with spaces.pyo

If instead I apply the no-record patch (attached), I can run bdist_rpm (not using sed workaround) without errors, but the resulting RPM has more directories specified:

$ rpm -q -filesbypkg -p dist/issue21153-package-0.0.0-1.noarch.rpm
issue21153-package        /
issue21153-package        /usr
issue21153-package        /usr/local
issue21153-package        /usr/local/lib
issue21153-package        /usr/local/lib/python2.7
issue21153-package        /usr/local/lib/python2.7/dist-packages
issue21153-package        /usr/local/lib/python2.7/dist-packages/issue21153_package-0.0.0.egg-info
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/__init__.py
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/__init__.pyc
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/__init__.pyo
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/file with spaces.py
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/file with spaces.pyc
issue21153-package        /usr/local/lib/python2.7/dist-packages/pkg/file with spaces.pyo

Perhaps that's acceptable. If it is, it makes the code simpler and cleaner.

Can someone comment on the impact of the presence of those parent directories in the RPM?

Distutils is now deprecated (see PEP 632) and all tagged issues are being closed. From now until removal, only release blocking issues will be considered for distutils.

If this issue does not relate to distutils, please remove the component and reopen it. If you believe it still requires a fix, most likely the issue should be re-reported at https://github.com/pypa/setuptools