Use package format 3 with conditional dependency on catkin. #536
Conversation
catkin isn't used or available in ROS 2. The recommended dependency on catkin for non-ROS packages released on the build farm is covered by the universally injected dependency on the ros_workspace package.
I've also proposed this change upstream in flexible-collision-library/fcl#536 if that merges then we can drop this overlay.
There was a problem hiding this comment.
Can you review this, @jamiesnape ?
Reviewable status: 0 of 1 files reviewed, all discussions resolved
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @nuclearsandwich)
|
@nuclearsandwich For octomap, there was a community request to add an additional entry to add the library to the ament_index. We also added a conditional dependency on |
|
Thanks for this PR! Can you move the octomap discussion to another PR or issue? |
|
Apologies for being unclear - my question was whether extending on this PR similar to how it's been accomplished for Octomap in the two snippets linked above is desired/required to amend (1) the package.xml touched in this PR to include a conditional dependency on ament_cmake [which based on the original post may be redundant] and (2) to add a registration to the ament_index in the CMakeLists [which may be better as a release patch]. Downstream projects have requested these in the past. If @nuclearsandwich can confirm whether this is necessary (or not), I am happy to make a PR |
Registering fcl in the ament resource index would move it closer to the behavior of "native" ROS 2 packages which simplifies usage for ROS 2 consumers of fcl. However this has also re-sparked a discussion of which packages MUST be registered in an ament index versus which packages MAY be registered. At best registering in the ament index is an expedient and may not ultimately be the recommended path for non-ROS packages however I do not yet have any guidance on an alternative recommended path. If you're willing to contribute a change similar to https://github.com/OctoMap/octomap/blob/bdb1c3757e370db93457d710772ab85e8bd44dd4/octomap/CMakeLists.txt#L75-L77 that seems like a good "little copying". It would be up to the fcl maintainers to accept it or not. |
REP-136 recommends that third-party (non-ROS) packages add a runtime dependency on catkin so that the setup scripts provided by the catkin package which set up a ROS environment are installed along with that package. In ROS 2, those scripts are provided by the ros_workspace package and bloom transparently injects a dependency on ros_workspace into every released package. It would be possible to use ament_cmake (or more likely the individual ament_cmake_core package to optionally provide the package.xml and ament resource index registration but since what you have is working fine I think just removing the unecessary dependency is a good next step. As mentioned in flexible-collision-library/fcl#536 (comment) we haven't produced a definitive recommendation for third party packages in ROS 2 but I've added it to a meeting agenda and we'll document the conclusions of that discussion and share them as well.
I'd like to release fcl into ROS 2, starting with ROS 2 Rolling.
catkin isn't used or available in ROS 2. The recommended dependency on catkin for non-ROS packages released on the build farm is covered by the universally injected dependency on the ros_workspace package.
This change is