homepage

This bug is forwarded from redhat's bugzilla:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=198877
-------------------------------------------
Description of problem:
setup.py fails to build rpm.
How reproducible:
Always
Steps to Reproduce:
1. Create dir "foo" with these files:
foo.py
setup.py
setup.py contains:
----
#!/usr/bin/env python
from distutils.core import setup
setup(
name='foo',
version='0.1',
py_modules=['foo'],
)
----
contents of file foo.py is not important.
2. from the 'foo' directory run
python setup.py bdist_rpm
 
Actual results:
building of rpm fails with
Checking for unpackaged file(s): /usr/lib/rpm/check-files
/var/tmp/foo-0.1-1-buildroot
error: Installed (but unpackaged) file(s) found:
 /usr/lib/python2.4/site-packages/foo.pyo
Expected results:
The command above should create a rpm package.

Indeed, this is a well-known issue. Many packages put an "optimize=1" in
their setup.cfg in order to solve it.

Ideally packages should not need to add "optimize=1" to their setup.cfg, 
as this currently breaks Jython compatibility. This is because Jython, 
including 2.5, does not support the -O flag.

passing optimize=1 does not help when there is a script
(...scripts=['bla.py']...) in the given distribution. The error will be
thrown for bla.pyo and bla.pyc

I couldn't reproduce the problem under Python 2.5, 2.6 and current
trunk. I am not sure I have the same conditions though, because 
I use a standalone RPM version under mac os x (see
http://rpm5.org/files/rpm/rpm-5.0/BINARY/)
Could you try to reproduce it under the latest 2.6 release ?
Nevertheless, I have started to write a test i'll include (see the patch)

Tarek, the issue can only be reproduced on Redhat/Fedora-like systems (I
have a Mandriva here and therefore can't check). But it's likely that
the issue still exists.

ok then: I am creating a Fedora 5 VM. It'll be useful in any case in
the future to follow bdist_rpm issues.

Note that:
This bug now is tracked in Fedora as
https://bugzilla.redhat.com/show_bug.cgi?id=236535
The root of the problem on Fedora is that SElinux will give noisy
warnings if python tries to to access or create non-existing foo.pyo for
a packaged foo.py. Fedoras solution to this is to always create and
package .pyo files to which proper SElinux labels can be attached. The
.pyo files are automatically created by
/usr/lib/rpm/brp-python-bytecompile behind the scenes when a rpm is
build. The problem is that bdist_rpm only knows and lists the .pyo files
created by setup and thus doesn't include the brp-python-bytecompile .pyos.
It could be argued that Fedora thus enforces a custom policy on RPM, and
it should be Fedoras job to complete that job and patch bdist_rpm to
match that custom policy. See the pending patch at 
https://bugzilla.redhat.com/show_bug.cgi?id=236535#c20
However, it would be nice if Python distutils acknowledged that
downstream Fedora has a special need here and helped solving it. Perhaps
the patch could be accepted in distutils so that bdist_rpm always runs
setup with -O1? Or some other change that could make it easy to
configure/customize distutils to match the platforms needs ...
Finally, note that Fedora and Red Hat (and apparently also Suse and
Mandriva) uses rpm.org, and rpm5 is a competing fork. (Rpm5 probably
claims that rpm.org is the evil guys who forked.)
Tarek, I assume you meant you are creating a Fedora 10 or RHEL/CentOS 5 VM?
I will be glad to help testing and answering further questions.

> Perhaps the patch could be accepted in distutils so that bdist_rpm 
> always runs setup with -O1? Or some other change that could make
> it easy to configure/customize distutils to match the platforms 
> needs ...
We could have a new parameter for bdist_rpm for this so it doesn't 
use sdist optimize parameter, (rpm_optimize) 
and make it True by default for example.
But is this a desired behavior for all rpm-based systems out there ?
> Tarek, I assume you meant you are creating a Fedora 10 
> or RHEL/CentOS 5 VM?
Yes, I am downloading Fedora 9. I assumed it was the best pick to 
work on that problem. (let me know if I should use another version)
 
> I will be glad to help testing and answering further questions
Great ! While I am going to work on Fedora 9 for this problem,
if you have the time to set up a python development 
environment, it would be great to test things on your side too.

Mads,
I am all set now, under a fresh Fedora 10. But it seems that the
generated command for rpm does not work.
it calls:
$ rpm -ba --define _topdir xxx --clean build/xxx/foo.spec
-ba unkown option.
(My Fedora runs rpm 4.6.0rc3)
Anything special to set under Fedora ?
This is strange because bdist_rpm will call -ba, -bs or -bb and
none of them seem to be present in this version of RPM,
while it is present on the standalone RPM I run under MacOsX (5.0)

The command "rpm" is (now, and in rpm.org version) for runtime and
installation only. Rpm building is done by /usr/bin/rpm-build (from the
package with the same name, run "yum install rpm-build"). 
/usr/lib/python2.5/distutils/command/bdist_rpm.py will select rpm-build
if it is available. 
Yes, the fallback is confusing. That is a consequence of the rpm fork
and probably out of scope for this issue ;-)

Ok, looks good now, thanks a lot.
But I am unable to reproduce the problem.
The rpm is built fine using my test and the provided example, 
with or without the -O1 option...
I have tried with Python trunk and Fedora's Python 2.5
Would you mind trying with the current Distutils trunk on your side ?

> But I am unable to reproduce the problem.
> 
> The rpm is built fine using my test and the provided example, 
> with or without the -O1 option...
IIUC, the problem occurs not on building the RPM, but on trying to
install it.

I am now able to reproduce it, thanks to Mads help
for the record, on a fresh Fedora 10, you need to do:
$ yum groupinstall "Development Tools"
$ yum install rpmdevtools
Then the problem will occur like described.
Moving to the next stage : fixing Distutils for that problem

The patch is ready. I'll ask at Distutils-SIG what people think about
the force-optimize option name, then I'll commit it for 2.7 and 3.1

before I commit this patch I will do this one : #5378
to be able to lower down the output of the rpm/rpmbuild command calls

Done in r70049 and r70051.
Thanks for everyone's help !

See comment 1 in
https://bugzilla.redhat.com/show_bug.cgi?id=236535
It might be that this patch still doesn't solve the problem, namely if
there are non-library .py files in the distribution. They get compiled
by brp-python-bytecompile, but the byte code files are not listed.
Apparently, the solution is to either %define
_unpackaged_files_terminate_build 0, or redefine %__os_install_post (to
what value?). Not sure which one is better.

Tarek, I think you committed too quickly. Please consider my comments.

I've read the thread while working in the patch,
and comment #28 explains that brp-python-bytecompile is called over 
the rpms and requires pyo files within the file list.
plus:
http://fedoraproject.org/wiki/Packaging/Python#Including_pyos
Now, if some defines in the spec file will prevent any call of
brp-python-bytecompile to break, it's better for sure. 
Digging in this, to see if there's a better fix then.

> I've read the thread while working in the patch,
> and comment #28 explains that brp-python-bytecompile is called over 
> the rpms and requires pyo files within the file list.
Sure, I'm not objecting. I claim that the patch might be insufficient,
and that brp-python-bytecompile produces bytecode files that bdist_rpm
didn't mention.

Martin,
What is the goal of bdist_rpm? I haven't seen that stated explicitly
anywhere, but I assume the goal is to make a fair attempt to easily
create usable RPMs for some software already using distutil,
acknowledging that it might not work in all cases (because some projects
do strange (buggy?) things) and that the RPMs probably can't be used in
distributions directly (because they probably have their own rules and
requirements).
The applied patch makes it possible for bdist_rpm to work in _some_
situations on Fedora. IMHO that is +1.
Yes, this patch might not be enough to make it work with *.py "__main__"
files. IMHO that is a less critical issue. Personally I have never seen
bdist_rpm fail for this reason. (But "often" for other reasons.)
An advantage of this patch is that it just fixes the existing approach
to work in more situations.
Disabling _unpackaged_files_terminate_build would IMHO be a bad
solution. That would cause "successful" RPM builds which doesn't include
all the files distutil installed. And FWIW I don't understand how
__os_install_post could solve the problem.
If you want another approach: Why use a filelist at all? Yes, it is
needed if the RPM is built "in place", but these days (at least on
Fedora) RPMs are always built in an empty RPM_BUILD_ROOT. So everything
found in RPM_BUILD_ROOT has been installed by distutils, and that
includes all the files and directories the RPM should contain. For 2.5 a
simplified patch for this is:
 # files section
 spec_file.extend([
 '',
- '%files -f INSTALLED_FILES',
+ '%files',
 '%defattr(-,root,root)',
+ '/',
 ])
 
That will also make the RPM own all directories in the path to its
files. That is bad in a distribution but might be OK for bdist_rpm. To
avoid that we could continue to use "-f INSTALLED_FILES" but generate
the file list with a simple "find" command in the %install section and
remove well-known paths such as /usr/lib/python*/site-packages,
/usr/bin, /etc and whatever we could come up with from the list.
This approach might work for almost all (sufficiently wellformed)
packages using distutil and will redefine bdist_rpm to "put all files in
a an RPM instead of installing them directly, so that they can be
removed by uninstalling the RPM". For example it works for logilab.astng
and logilab.pylint which didn't work before.

> What is the goal of bdist_rpm? I haven't seen that stated explicitly
> anywhere, but I assume the goal is to make a fair attempt to easily
> create usable RPMs for some software already using distutil,
> acknowledging that it might not work in all cases (because some projects
> do strange (buggy?) things) and that the RPMs probably can't be used in
> distributions directly (because they probably have their own rules and
> requirements).
Correct.
> The applied patch makes it possible for bdist_rpm to work in _some_
> situations on Fedora. IMHO that is +1.
I agree. I had another complaint about that patch, though, which is
the addition of an option for not including .pyo files. I still like
to see addition of this option reverted.
> Disabling _unpackaged_files_terminate_build would IMHO be a bad
> solution. That would cause "successful" RPM builds which doesn't include
> all the files distutil installed. And FWIW I don't understand how
> __os_install_post could solve the problem.
IIUC, setting
%define __os_install_post %{___build_post}
should prevent invocation of brp-python-bytecompile.

> IIUC, setting
>
> %define __os_install_post %{___build_post}
>
> should prevent invocation of brp-python-bytecompile.
Ok. But preventing invocation of brp-python-bytecompile is IMHO not a
solution. brp-python-bytecompile is needed for the reasons mentioned in
http://fedoraproject.org/wiki/Packaging/Python#Including_pyos .

> Ok. But preventing invocation of brp-python-bytecompile is IMHO not a
> solution. brp-python-bytecompile is needed for the reasons mentioned in
> http://fedoraproject.org/wiki/Packaging/Python#Including_pyos .
What reason specifically are you referring to? That it creates and
packages .pyo files? bdist_rpm already does that (now), so that reason
won't apply to bdist_rpm.

Ok, if you will keep bdist_rpm's new .pyo creation then you are right.
FWIW I think it is a bit of an ugly hack and would prefer another solution.
BTW: The "brp-python-bytecompile creates usr/bin/*.pyo" issue has been
resolved, https://bugzilla.redhat.com/show_bug.cgi?id=182498#c8 

> BTW: The "brp-python-bytecompile creates usr/bin/*.pyo" issue has been
> resolved, https://bugzilla.redhat.com/show_bug.cgi?id=182498#c8
Ok. Perhaps it isn't needed to exclude it, then.

> I agree. I had another complaint about that patch, though, which 
> is the addition of an option for not including .pyo files. 
> I still like to see addition of this option reverted.
I'll remove this option then, and make -O1 hardcoded

done in r70094 and r70096 

> done in r70094 and r70096
Thanks!