Switch tf2_eigen to use package.xml format 2. #216
Conversation
This is to modernize it a bit to make it more compatible with ROS 2. While we are in here, remove the run/exec dependency on cmake_modules, which doesn't seem to be needed at runtime. Signed-off-by: Chris Lalancette <[email protected]>
tf2_eigen/package.xml
Outdated
| <run_depend>geometry_msgs</run_depend> | ||
| <run_depend>tf2</run_depend> | ||
| <run_depend>cmake_modules</run_depend> | ||
| <exec_depend>geometry_msgs</exec_depend> |
There was a problem hiding this comment.
geometry_msgs and tf2 also needs the <build_export_depend> because of the includes in the headers. http://docs.ros.org/kinetic/api/catkin/html/howto/format2/migrating_from_format_1.html#run-depend
But since they overlap with the build depends it can be collapsed to just a <depend> .
There was a problem hiding this comment.
Right, good point. Will fix that.
tf2_eigen/package.xml
Outdated
| @@ -8,13 +8,13 @@ | |||
| <license>BSD</license> | |||
|
|
|||
| <buildtool_depend>catkin</buildtool_depend> | |||
|
|
|||
| <build_depend>cmake_modules</build_depend> | |||
There was a problem hiding this comment.
This needs <build_export_depend> too since it's in the header and will be needed downstream.
There was a problem hiding this comment.
Can you explain this a bit further so I understand? From what I can tell, tf2_eigen.h includes:
#include <tf2/convert.h>
#include <Eigen/Geometry>
#include <geometry_msgs/PointStamped.h>
#include <geometry_msgs/PoseStamped.h>
So all of those would need build_export_depend (as you point out below). Why would we also need to build_export_depend cmake_modules?
There was a problem hiding this comment.
I think you're right. All the EIGEN variables will be evaluated so it won't need them downstream.
This makes sure they are exec_depend, build_depend, and build_export_depend, all three of which are needed. Also make eigen a build_export_depend, since it is included in the exported header file. Signed-off-by: Chris Lalancette <[email protected]>
|
Thanks :). |
This is to modernize it a bit to make it more compatible
with ROS 2. While we are in here, remove the run/exec dependency
on cmake_modules, which doesn't seem to be needed at runtime.
Signed-off-by: Chris Lalancette [email protected]