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

Version 0.6.15 breaks Python packages which are not in a subfolder with the same name #756

Closed
jarvisschultz opened this issue Sep 22, 2015 · 5 comments
Labels

Comments

@jarvisschultz
Copy link

We have a non-ROS Python package called trep, we also have a release repository called trep-release. In the release repository, we use the patches variable in tracks.yaml to include the non-ROS Python module as a third-party package. In an attempt to follow our understanding of associated naming conventions as closely as possible, the Python package is called trep and the corresponding ROS package is called python-trep.

Since the merging of PR #751, our builds are now failing on the buildfarm during the catkin_python_setup.cmake target. The error is

CMake Error at /opt/ros/indigo/share/catkin/cmake/catkin_python_setup.cmake:96 (message):
  The package name 'trep' differs from the basename of the path 'src' in
  project 'python_trep'

The full log is available here.

So I guess my questions are:

  1. Did PR Fix package dir location #751 intentionally disable ROS packages including third party Python modules with different names?
  2. If so, should we just change our package to name to be trep instead of python_trep? Or is there some other way that we can fix this issue?
@dirk-thomas
Copy link
Member

I don't think the problem is in the difference between the Python package name trep and ROS package name python_trep.

The recent PR fixed a problem where the Python package name and the directory for it were extracted / interpreted incorrectly. Before the above error message did not trigger for your package because of that problem.

I tried to use your package with the previous version of catkin and found one issue which is related to this one. When using catkin_make install your package works fine. But when using catkin_make and sourcing the environment of the resulting devel space I am unable to load your Python package correctly. While import trep succeeds I can not access any symbol in there.

For the devel space catkin generates a relaying python script (https://github.com/ros/catkin/blob/indigo-devel/cmake/templates/__init__.py.in). As far as I can see this currently only works when the folder name is equal to the Python package name. Since this is not the case for your package trep (it is in the folder src) the devel space redirection does not work and that is what the error message is covering.

So I think your package did not work in the devel space before but the intended error message was not printed due to the problem fixed in the PR.

The question now is can we make the relaying script more flexible to cover your use case or do you have to move the code in the package into a subfolder with the Python package name (e.g. src/trep)? I will look into if and how option one is feasible...

@dirk-thomas dirk-thomas changed the title Version 0.6.15 breaks upstream third party package with different name Version 0.6.15 breaks Python packages which are not in a subfolder with the same name Sep 22, 2015
@jarvisschultz
Copy link
Author

You are absolutely correct. Changing all of the relevant code to be in a trep/ subdirectory instead of the src/ subdirectory, and updating the setup.py to reflect this change solves the build issue that I originally described. It is worth noting that, I think, this particular package will never be usable in the devel/ space because it uses a Python extension module that is only compiled when running catkin_make install.

Fortunately for us, we do have the ability to edit the upstream repository. So moving everything to a trep/ subdir is definitely a reasonable workaround.

However, seeing that Python's built in packaging tools (distutils, setuptools, or whatever) allow using a different directory than the module name, it seems that it would be nice if the relaying script could also handle this situation.

@dirk-thomas
Copy link
Member

I looked into the relay script (https://github.com/ros/catkin/blob/indigo-devel/cmake/templates/__init__.py.in) and since it uses the sys.path I can't easily change it to work with non-package name folders. Therefore I don't see a "fix" within catkin to handle that case better.

I am sorry for the inconvenience but please go ahead and update your package.

@dirk-thomas
Copy link
Member

@asmodehn
Copy link
Contributor

I just saw this issue recently.
If I understand the problem correctly, I believe that the fix to this kind of problem should be inside the setup.py...

Looking at this version :
https://github.com/MurpheyLab/trep/blob/f5875cf5413f28f1d3e9fe81072abcce7ce6fcff/setup.py
I would expect package_dir = {'trep': 'src'} to work with catkin the same way that it works with distutils.

It just says that your package __init__.py is in the src/ folder, and not src/trep like catkin <0.6.15 understood. Actually '':'src' in there looks to me like a workaround for old catkin.

Reference: https://docs.python.org/2/distutils/setupscript.html#listing-whole-packages
Disclaimer: I haven't tested this on trep. Just thought it was worth mentioning in case someone else ends up here...

skohlbr added a commit to team-vigir/vigir_ocs_common that referenced this issue Jan 31, 2016
argenos pushed a commit to b-it-bots/mas_cob that referenced this issue Feb 19, 2018
argenos pushed a commit to b-it-bots/mas_cob that referenced this issue Feb 19, 2018
argenos pushed a commit to b-it-bots/mas_cob that referenced this issue Feb 19, 2018
argenos pushed a commit to b-it-bots/mas_simulation that referenced this issue Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants