Support colcon workspace with bullet and ode

[scpeters]

We have been using colcon to build gazebo and ignition packages from source in a workspace, similar to using catkin-tools with a catkin workspace. A cool feature of colcon is that it can automatically identify cmake dependencies based on the cmake project name and build packages in the workspace in the correct order without creating package.xml files.

• https://github.com/colcon/colcon-cmake/blob/master/colcon_cmake/package_identification/cmake.py#L132-L135

If a package.xml file is present in a package, it will use that instead of the automatic detection, so you can build a mixed workspace of plain cmake and ROS1 or ROS2 packages.

When attempting to use colcon in a workspace with ign-physics, dartsim, bullet, and ODE, it was not properly identifying the dependencies. I believe the following two steps are necessary:

• adding depend tags for BULLET_PHYSICS and ODE to the dartsim package.xml file
  • https://github.com/bulletphysics/bullet3/blob/master/CMakeLists.txt#L7
  • https://bitbucket.org/odedevs/ode/src/695ba8c6aa4a725b610514725dcb9ea77683236d/CMakeLists.txt#lines-3
• changing the cmake project name and package.xml package name to DART to match the generated DARTConfig.cmake

I'm not sure if there are cpack / rosdistro / rosdep implications of changing these things, but I think it helps with colcon. Here's a workspace file for testing:

• https://bitbucket.org/osrf/gazebodistro/raw/%20physics_dart_bullet_ode/ign-physics1.yaml

cc @mxgrey @jslee02 @azeey

---

Before creating a pull request

• Document new methods and classes
• Format new code files using clang-format

Before merging a pull request

• Set version target by selecting a milestone on the right side
• Summarize this change in CHANGELOG.md
• Add unit test(s) for this change

[mxgrey]

Unfortunately there are some breaking changes that would be required for DART/dartsim to comply with the assumptions made by colcon's package information inference rules. I think it'll make sense to put these changes into dartsim-7, but the changes could pose significant problems for current users of DART/dartsim-6.

Instead of relying on colcon's inference rules, maybe we should explicitly put a colcon.pkg file into the DART codebase? Obviously this would be annoying as it would introduce a third source of package metadata into the source code (the other two being cmake and package.xml), but we would only use it as a stop-gap measure until dartsim-7, at which point we can make our cmake fully compliant with colcon's assumptions, and then delete the colcon.pkg file altogether.

[scpeters]

a colcon.pkg file sounds like the right thing to do, at least in the short term. I'll experiment with that tomorrow.

[mjcarroll]

Colcon also has a mechanism for providing metadata so that you don't necessarily have to add colcon.pkg files to the packages.

It would be reasonable to add the information there as well: https://github.com/colcon/colcon-metadata-repository

[scpeters]

Here is the content of an equivalent colcon.pkg file:

{
  "name": "DART",
  "dependencies": ["BULLET_PHYSICS", "ODE"],
}

[scpeters]

I reverted the changes to CMakeLists.txt and package.xml and added a colcon.pkg file instead.

[scpeters]

@mjcarroll wrote:

Colcon also has a mechanism for providing metadata so that you don't necessarily have to add colcon.pkg files to the packages.

It would be reasonable to add the information there as well: https://github.com/colcon/colcon-metadata-repository

I guess we could add the "dependencies": ["BULLET_PHYSICS", "ODE"], line to the metadata repository, but I think we would still need the colcon.pkg here if we want to change the project name used by colcon so that downstream cmake software that uses find_package(DART) will recognize this dependency automatically.

[jslee02]

Does this file support comments in the file? If so, it'd be great to leave a brief comment how this file is used for.

[mjcarroll]

It's YAML, so comments prefixed with # are acceptable here.

[codecov]

Codecov Report

Merging #1389 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1389   +/-   ##
=======================================
  Coverage   57.27%   57.27%           
=======================================
  Files         366      366           
  Lines       27432    27432           
=======================================
  Hits        15711    15711           
  Misses      11721    11721