Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bloom installing python bytecode #199

Open
piyushk opened this issue Sep 2, 2013 · 17 comments · May be fixed by #550
Open

bloom installing python bytecode #199

piyushk opened this issue Sep 2, 2013 · 17 comments · May be fixed by #550

Comments

@piyushk
Copy link

piyushk commented Sep 2, 2013

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
Copy link
Contributor

wjwwood commented Sep 2, 2013

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
Copy link
Author

piyushk commented Sep 2, 2013

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
Copy link
Contributor

wjwwood commented Sep 3, 2013

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

@wjwwood
Copy link
Contributor

wjwwood commented Sep 3, 2013

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
Copy link
Author

piyushk commented Sep 3, 2013

@wjwwood Yes. This is a different issue. I've just closed #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
Copy link
Contributor

wjwwood commented Jan 6, 2014

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
Copy link

130s commented Jan 8, 2014

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
Copy link
Contributor

wjwwood commented Jan 8, 2014

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

@130s
Copy link

130s commented Jan 9, 2014

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

@tfoote
Copy link
Member

tfoote commented Jan 9, 2014

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

@po1
Copy link
Contributor

po1 commented Jul 3, 2014

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
Copy link
Contributor

wjwwood commented Jul 3, 2014

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
Copy link
Contributor

po1 commented Jul 3, 2014

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
Copy link
Contributor

wjwwood commented Jul 3, 2014

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

@po1
Copy link
Contributor

po1 commented Jul 3, 2014

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:

  • I had some trouble with DH_OPTIONS, dh_python2 was failing with them, so I had to put these options directly on the dh $@ line
  • This adds an explicit build dependency to python, which may otherwise not be necessary when building a non-catkin project

@wjwwood
Copy link
Contributor

wjwwood commented Jul 7, 2014

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
Copy link
Contributor

po1 commented Jul 8, 2014

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants