ros-infrastructure / bloom

A release automation tool which makes releasing catkin (http://ros.org/wiki/catkin) packages easier.
Other
58 stars 94 forks source link

bloom installing python bytecode #199

Open piyushk opened 11 years ago

piyushk commented 11 years ago

Not sure how bad this is, but I saw the following errors while running a pre-release test locally. The cmvision package contains a few custom messages.

E: ros-hydro-cmvision: package-installs-python-bytecode opt/ros/hydro/lib/python2.7/dist-packages/cmvision/__init__.pyc
E: ros-hydro-cmvision: package-installs-python-bytecode opt/ros/hydro/lib/python2.7/dist-packages/cmvision/msg/_Blob.pyc
E: ros-hydro-cmvision: package-installs-python-bytecode opt/ros/hydro/lib/python2.7/dist-packages/cmvision/msg/_Blobs.pyc
E: ros-hydro-cmvision: package-installs-python-bytecode opt/ros/hydro/lib/python2.7/dist-packages/cmvision/msg/__init__.pyc
wjwwood commented 11 years ago

Sorry, I'm not sure what this has to do with bloom? Is it just that you saw this error while following a bloom tutorial or?...

I need more context to even know what might be going on here.

piyushk commented 11 years ago

I was running a pre-release test on cmvision while following this tutorial: http://wiki.ros.org/bloom/Tutorials/PrereleaseTest

After git-buildpackage succeeds, lintian produces the errors as seen in the above bug report. I guess all this means is the bytecode is also making its way into the system deb (instead of just the python files). I'm not sure if this has any adverse side effects or not, and wasn't able to get many hits on google as to whether this was a bug.

wjwwood commented 11 years ago

Did you address this in the tutorial? Can this be closed?

wjwwood commented 11 years ago

Actually this was a different issue than the one we were discussing right? I'm not sure but this might have something to do with how we use setup.py in catkin? @dirk-thomas

piyushk commented 11 years ago

@wjwwood Yes. This is a different issue. I've just closed https://github.com/ros-infrastructure/bloom/issues/200.

Also, my google skills were sub-par yesterday. This askubuntu thread talks about the problem above: http://askubuntu.com/questions/243787/why-py-and-not-pyc-or-pyo

wjwwood commented 10 years ago

I believe this will require us to not install .pyc/.pyo files or strip them from the installed files and then use dh_python2 --compile-all and/or dh_python3 --compile-all to have the debian automatically get a postinst rule for compiling the python files for the appropriate python at install time.

130s commented 10 years ago

I believe this will require us to not install .pyc/.pyo files or strip them from the installed files and then use dh_python2 --compile-alland/or dh_python3 --compile-all to have the debian automatically get a postinst rule for compiling the python files for the appropriate python at install time.

Not sure if this is the right place to go on this topic, but @wjwwood just from curiosity, do you have an idea whether what you just mentioned is planned to be implemented somewhere? We had a discussion about installed .pyc files too with someone from a robot manufacturer.

wjwwood commented 10 years ago

I think we will try to implement it for Indigo, but that will be time permitting.

130s commented 10 years ago

I see. One more question; which repository is the right one to watch about it? ros-infrastructure/jenkins_scripts? Thanks in advance!

tfoote commented 10 years ago

@130s This repo, this ticket. It's going to need to be in the debian templates bloom uses.

po1 commented 10 years ago

Nice to know that this issue is acknowledged. I might have a few cycles to update bloom's templates with this in upcoming weeks, if it is still relevant.

wjwwood commented 10 years ago

I think it is more complicated than updating templates, we have to figure out how to separate installation of python code from the byte compiling step, which last I remember might involve changes to catkin to allow that.

Then again, I could be wrong it can be done completely in the rules file template.

po1 commented 10 years ago

In a mailing list that I can't find anymore, some debian devs advocated that the bytecode compilation step should not be prevented, as it can provide valuable QA feedback, and that the byte-compiled files should simply be removed during packaging, and re-compiled during the postinst stage.

I have played a bit with it today, and I edited bloom's templates to make it do exactly that. A patch will follow shortly.

wjwwood commented 10 years ago

Yes, reading this issue again, that seems to be the consensus here too. Cool, thanks for the help.

po1 commented 10 years ago

So far, I have the following:

--- a/bloom/generators/debian/templates/control.em
+++ b/bloom/generators/debian/templates/control.em
@@ -2,11 +2,11 @@ Source: @(Package)
 Section: misc
 Priority: extra
 Maintainer: @(Maintainer)
-Build-Depends: debhelper (>= @(debhelper_version).0.0), @(', '.join(BuildDepends))
+Build-Depends: debhelper (>= @(debhelper_version).0.0), python, @(', '.join(BuildDepends))
 Homepage: @(Homepage)
 Standards-Version: 3.9.2

 Package: @(Package)
 Architecture: any
-Depends: ${shlibs:Depends}, ${misc:Depends}, @(', '.join(Depends))
+Depends: ${shlibs:Depends}, ${misc:Depends}, ${python:Depends}, @(', '.join(Depends))
 Description: @(Description)
--- a/bloom/generators/debian/templates/rules.em
+++ b/bloom/generators/debian/templates/rules.em
@@ -8,7 +8,7 @@

 # Uncomment this to turn on verbose mode.
 export DH_VERBOSE=1
-export DH_OPTIONS=-v --buildsystem=cmake
+#export DH_OPTIONS=-v --buildsystem=cmake
 # TODO: remove the LDFLAGS override.  It's here to avoid esoteric problems
 # of this sort:
 #  https://code.ros.org/trac/ros/ticket/2977
@@ -17,7 +17,7 @@ export LDFLAGS=
 export PKG_CONFIG_PATH=@(InstallationPrefix)/lib/pkgconfig

 %:
-       dh  $@@
+       dh  $@@ -v --buildsystem=cmake --with python2

 override_dh_auto_configure:
        # In case we're installing to a non-standard location, look for a setup.sh
@@ -57,3 +57,6 @@ override_dh_auto_install:
        # set things like CMAKE_PREFIX_PATH, PKG_CONFIG_PATH, and PYTHONPATH.
        if [ -f "@(InstallationPrefix)/setup.sh" ]; then . "@(InstallationPrefix)/setup.sh"; fi && \
        dh_auto_install
+
+override_dh_python2:
+       dh_python2 -v -p @(Package) @(InstallationPrefix)

Notes:

wjwwood commented 10 years ago

That seems reasonable, but I would need to be tested in our current infrastructure to make sure it doesn't break anything.

I understand the need to add the explicit build dependency on Python, but it is over matching on packages which do not use catkin and do not have Python code.

po1 commented 10 years ago

I understand the need to add the explicit build dependency on Python, but it is over matching on packages which do not use catkin and do not have Python code.

Yes, that was my concern. Maybe a templatized version of it, depending on whether bloom can find a setup.py in the source tree would be better. Also, that involves a build dependency, but the tool might be clever enough to not add any runtime dependency of none is needed. If this would be acceptable, I'll investigate it.

As for the infrastructure, I am testing it right now on a partial replica of the ROS buildfarm for Raspbian, and so far it seems that it hasn't caused any trouble.