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

Plain CMake packages don't get put on ROS_PACKAGE_PATH #143

Closed
jbohren opened this issue Feb 10, 2015 · 6 comments
Closed

Plain CMake packages don't get put on ROS_PACKAGE_PATH #143

jbohren opened this issue Feb 10, 2015 · 6 comments

Comments

@jbohren
Copy link
Contributor

jbohren commented Feb 10, 2015

Currently, if you build a plain CMake package with a package.xml file and <build_type>cmake</build_type>, that package doesn't get put on the ROS_PACKAGE_PATH after sourcing the result space.

Is this by design, or is it an oversight?

@wjwwood
Copy link
Member

wjwwood commented Feb 10, 2015

Neither catkin_tools nor catkin sets the ROS_PACKAGE_PATH. It is set by roslib:

https://github.com/ros/ros/blob/indigo-devel/core/roslib/env-hooks/10.ros.sh.em#L11-L33

So if the plain CMake packages don't depend on and find_package(...) at least roslib then they don't get on the RPP. I think this makes sense in most cases because they are not ROS packages.

@jbohren
Copy link
Contributor Author

jbohren commented Feb 10, 2015

Except if these packages get installed, then they do end up on the ROS_PACKAGE_PATH. This means you can do the following:

source /opt/ros/hydro/setup.bash
sudo apt-get install ros-hydro-orocos-kdl
mkdir -p ws/src
git clone [email protected]:orocos/orocos_kinematics_dynamics.git ws/src/orocos_kinematics_dynamics
cd ws
catkin build
source devel/setup.bash
roscd orocos_kdl
pwd # /opt/ros/hydro/share/orocos_kdl

This can be really confusing since this catkin workspace is supposed to overlay the previous ones. I understand that this is because roscd is using ROS_PACKAGE_PATH and catkin is using CMAKE_PREFIX_PATH but this still seems like it's broken somehow.

@wjwwood
Copy link
Member

wjwwood commented Feb 12, 2015

catkin should not have any knowledge of ROS_PACKAGE_PATH, so it's not possible to do this in the tools. If, in your package.xml of your plain CMake package, you depend on roslib it would be put on the ROS_PACKAGE_PATH.

Based on the example you give, I would call that an inconsistency with roscd. It should maybe be looking on the CMAKE_PREFIX_PATH (or the CATKIN_PREFIX_PATH had that ever taken off) for the package.xml's since there is nothing which says that package.xml's should be in the ROS_PACKAGE_PATH. If it did that it would have found your overlaid version of orocos_kinematics_dynamics.

I would actually prefer roscd to not change directories to package.xml's which have a built_type of cmake since they are not ROS packages. However, that excludes ROS packages from being plain CMake. The real problem here is there is no good way to distinguish a ROS package from just a catkin package or a plain cmake package, i.e. "has a package.xml" does not imply "is a ROS package". In fact the best way is probably that it directly or indirectly depends on roslib, but that would be pretty hard to determine at runtime efficiently.

@jbohren
Copy link
Contributor Author

jbohren commented Feb 12, 2015

Based on the example you give, I would call that an inconsistency with roscd. It should maybe be looking on the CMAKE_PREFIX_PATH (or the CATKIN_PREFIX_PATH had that ever taken off) for the package.xml's since there is nothing which says that package.xml's should be in the ROS_PACKAGE_PATH. If it did that it would have found your overlaid version of orocos_kinematics_dynamics.

This would make sense. It would be nice if we could resolve these inconsistencies.

I would actually prefer roscd to not change directories to package.xml's which have a built_type of cmake since they are not ROS packages.

That statement appears really strange to me.

@wjwwood
Copy link
Member

wjwwood commented Feb 13, 2015

That statement appears really strange to me.

Especially if you take it out of context 😛. I go on to say that would say that plain CMake packages would not be considered ROS packages ever, which is a constraint that is technically unnecessary and poorly defined at the moment. Then I also say that what it really means to be a ROS package is that you depend on roslib, directly or otherwise, but that it would be hard for roscd to check that.

Maybe a more declarative list will illustrate my perspective better:

  • roscd should only change to directories of ROS packages.
  • ROS packages are packages which depend directly or indirectly on roslib.
  • roscd doesn't currently do that which sucks.
  • roscd can easily check whether or not a package is a catkin package or plain cmake, but it cannot easily check the recursive dependencies of the package for roslib.
  • I also believe there are no plain CMake packages which are ROS packages.
  • So having roscd ignore plain CMake package.xml's is a more efficient solution than checking the recursive dependencies.

One slightly better check would be for roscd to ignore plain CMake package.xml's unless they directly depend on roslib. That would then be an efficient check which only fails to find plain CMake ROS packages if they do not directly depend on roslib. But since there are currently no plain CMake packages which are also ROS packages that I am aware of, this is all sort of optimizing for a use case that doesn't exist.

We can discuss the merits of any of the above bullets, but hopefully that's a little clearer than my last post.

@wjwwood
Copy link
Member

wjwwood commented Feb 13, 2015

I would also say that for ROS 2, I have a plan to fix this, which also removes the need for the ROS_PACKAGE_PATH. You can read my proposal here:

https://github.com/wjwwood/package_resource_indexer

It might be something we could roll out into ROS 1, but I don't have plans to in the near term. Basically the definition of a ROS package on the file system would be anything that installs both a package.xml in <prefix>/share/<package_name> and an empty file named <prefix>/resouce_index/ros_package/<package_name>. This can efficiently be checked by a few ls like commands.

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

No branches or pull requests

2 participants