Bug 558997 - brp-python-bytecompile does not enter correct paths in python bytecode
Summary: brp-python-bytecompile does not enter correct paths in python bytecode
Alias: None
Product: Fedora
Classification: Fedora
Component: rpm
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Panu Matilainen
QA Contact: Fedora Extras Quality Assurance
Depends On:
Blocks: Python3F13
TreeView+ depends on / blocked
Reported: 2010-01-26 21:21 UTC by Toshio Ernie Kuratomi
Modified: 2010-02-03 10:55 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-02-03 10:55:44 UTC
Type: ---

Attachments (Terms of Use)
Update brp-python-bytecompile to embed the proper paths (2.85 KB, application/octet-stream)
2010-01-26 21:21 UTC, Toshio Ernie Kuratomi
no flags Details
New version -- tested on py2/py3 package (2.87 KB, text/plain)
2010-01-27 00:19 UTC, Toshio Ernie Kuratomi
no flags Details
brp-python-bytecompile that warns and continues (3.04 KB, text/plain)
2010-01-27 01:38 UTC, Toshio Ernie Kuratomi
no flags Details
brp-python-bytecompile that stops build on SyntaxError (3.16 KB, text/plain)
2010-01-30 00:02 UTC, Toshio Ernie Kuratomi
no flags Details

Description Toshio Ernie Kuratomi 2010-01-26 21:21:04 UTC
Created attachment 386918 [details]
Update brp-python-bytecompile to embed the proper paths

Description of problem:
Currently, python modules built for F13 generate .pyc and .pyo files with /site-packages/filename.py embedded in them.  Instead, they should generate files with /usr/lib/python2.6/site-packages/filename.py  (and similar for other paths under site-packages).

These false paths become visible to end users when exceptions are thrown in some circumstances.  I have triggered it for python and python3 when I have the .pyc but not the .py file installed.  You can do something like this:

sudo yum install -y python-setuptools
sudo rm /usr/lib/python2.6/site-packages/pkg_resources.py
echo "import pkg_resources\npkg_resources.require('GobbledyGook >= 1000')\n" > test.py
python ./test.py

Traceback [...]
File "/site-packages/pkg_resources.py", line 644, in require

There may be other ways to trigger this.  When brp-pythn-bytecompile was written, this was an issue that the script purposefully addressed.  Not sure python has become more robust or if those issues still exist.

Version-Release number of selected component (if applicable):

How reproducible:

Steps to Reproduce:
1. rpm -Uvh 'http://koji.fedoraproject.org/koji/getfile?taskID=1946373&name=python-setuptools-0.6.9-5.fc13.noarch.rpm'
2. strings /usr/lib/python2.6/site-packages/pkg_resources.pyc | grep site-packages
Actual results:
Lots of strings of the form:

Expected results:
Lots of strings of the form:

Additional info:
Will attach an untested brp-python-bytecompile script.

It also adds back in some checking for SyntaxErrors that wasn't pushed into the updated script but I'm not quite certain how that works (the comment says it issues a warning; the script appears to exit early if a SyntaxError was encountered... doesn't seem quite right to me.)

Comment 1 Toshio Ernie Kuratomi 2010-01-27 00:19:41 UTC
Created attachment 386948 [details]
New version -- tested on py2/py3 package

New version of brp-python-bytecompile

This version of the script has a few bug fixes.  Tested that it works on a
python2/python3 module package (python-setuptools with a python3 subpackage).  Embeds the expected directory paths.
No private libraries were tested in the package to make sure the default_python call worked.

Also didn't test what happens when SyntaxErrors are found.

Comment 2 Toshio Ernie Kuratomi 2010-01-27 01:12:12 UTC
Tested SyntaxErrors.  The behaviour is that it gives a warning and then exits.  If the syntax error happens in the compilation of .pyc in the site-packages directory then none of the rest of the files will be byte compiled.  This seems wrong to me....  This can be fixed in one of two ways:

1) If a SyntaxError is encountered, fail the build
2) If a SyntaxError is encountered, print a warning and continue

#2 seems more in line with what the comments imply is wanted so I'll change the patch to do that.  I like #1 better though :-)  Perhaps its not the case because we aren't able to differentiate SyntaxErrors from other reasons py.compile might fail?

In either case, distutils and setuptools packages should be byte compiling themselves in the %build section of the spec file (according to Guidelines and the rpmdevtools python spec template) so we should be catching SyntaxErrors earlier in the build.

Comment 3 Toshio Ernie Kuratomi 2010-01-27 01:38:24 UTC
Created attachment 386963 [details]
brp-python-bytecompile that warns and continues

Last revision, this version will warn and continue on SyntaxErrors.

Comment 4 Panu Matilainen 2010-01-27 08:03:43 UTC
1) was what originally went in, but it triggered some unforeseen problems (always the case, sigh :) with python template files, jython-related packages and whatnot. So the quick-n-dirty solution was to just warn and bail out for now.

As there's no way to automatically determine whether a .py file fails compilation for a legitimate reason or not, the best option would probably be defaulting to 1) but permitting those packages to opt for 2) through a spec override to stumble through thouse special cases.

Comment 5 Toshio Ernie Kuratomi 2010-01-27 18:49:04 UTC
Okay.  I've reworked the script, macros and redhat/macros from rpm-redhat-config to almost do this.  The one piece I'm missing is how to get brp-python-bytecompile to actually fail the build.  I tried exit 1 but that doesn't do it.  Any help?

Comment 6 Panu Matilainen 2010-01-28 22:20:44 UTC
Hmm, exit 1 is certainly supposed to fail the build, and does here for an artificial test-case. Can you attach the updated script so I can have a look?

Comment 7 Dave Malcolm 2010-01-28 22:57:45 UTC
The current state of https://fedoraproject.org/wiki/PackagingDrafts/Python3#TODO says that 
  "Must get this bug fixed so byte compilation does not stop midway through if there's a SyntaxError bug 558997"

Does this aspect work yet in F13's rpm?  (trying to get the packaging guidelines done)

Comment 8 Toshio Ernie Kuratomi 2010-01-29 17:35:12 UTC
Found my bug.  Working on this today.

Comment 9 Toshio Ernie Kuratomi 2010-01-30 00:02:18 UTC
Created attachment 387680 [details]
brp-python-bytecompile that stops build on SyntaxError

Comment 10 Toshio Ernie Kuratomi 2010-01-30 00:05:49 UTC
Here's the changes to /usr/lib/rpm/redhat/macros to enable this:

     %{!?__debug_package:/usr/lib/rpm/redhat/brp-strip %{__strip}} \
     /usr/lib/rpm/redhat/brp-strip-static-archive %{__strip} \
     /usr/lib/rpm/redhat/brp-strip-comment-note %{__strip} %{__objdump} \
-    /usr/lib/rpm/brp-python-bytecompile \
+    /usr/lib/rpm/brp-python-bytecompile %{_python_bytecompile_errors_terminate_build} \
     /usr/lib/rpm/redhat/brp-python-hardlink \
     %{!?__jar_repack:/usr/lib/rpm/redhat/brp-java-repack-jars} \

+# Should bytecompilation errors terminate a build?
+%_python_bytecompile_errors_terminate_build   1

Comment 11 Panu Matilainen 2010-02-03 10:16:54 UTC
Didn't test it yet, but one immediate problem spotted: changing the order of arguments for brp-python-bytecompile creates a pesky and unnecessary incompatibility issue between rpm <-> redhat-rpm-config versions. Better leave $1 for optional python interpreter path and add the "strict" mode as $2, defaulting to former behavior where it doesn't abort the build. That way it'll work regardless of the rpm version, otherwise it'd need some brittle release-specific require on rpm version.

Oh and there are a bunch of now incorrect "XXX TODO: parametrize the exit code, only warn for now" comments.

I dont mind doing these final tweaks, unless you have either objections or insist on doing them yourself :)

Comment 12 Panu Matilainen 2010-02-03 10:55:44 UTC
Oh well, I went ahead and changed the arg order and removed the old comments. Should be fixed now in rawhide as of rpm-4.8.0-5.fc13 and also applied upstream. Thanks for your work on this Toshio!

Note You need to log in before you can comment on or make changes to this bug.