package.xml: depend on bullet

[scpeters]

DART is using bullet for collision detection, but bullet wasn't listed in the package.xml file.

[scpeters]

One of the test instances failed, but I don't know why.

In file included from /home/travis/build/dartsim/dart/dart/collision/fcl/FCLCollisionNode.cpp:37:
In file included from /home/travis/build/dartsim/dart/dart/collision/fcl/FCLCollisionNode.h:44:
In file included from /usr/include/fcl/collision.h:42:
In file included from /usr/include/fcl/collision_object.h:43:
In file included from /usr/include/fcl/ccd/motion_base.h:45:
In file included from /usr/include/fcl/BV/RSS.h:43:
In file included from /usr/local/include/boost/math/constants/constants.hpp:18:
In file included from /usr/local/include/boost/lexical_cast.hpp:159:
In file included from /usr/local/include/boost/numeric/conversion/cast.hpp:34:
In file included from /usr/local/include/boost/numeric/conversion/numeric_cast_traits.hpp:28:
/usr/local/include/boost/numeric/conversion/detail/numeric_cast_traits.hpp:12:14: fatal error: 
      'boost/numeric/conversion/detail/preprocessed/numeric_cast_traits_common.hpp'
      file not found
    #include <boost/numeric/conversion/detail/preprocessed/numeric_cast_...
             ^
1 error generated.

[mxgrey]

Travis-CI seems to have a recurring issue where it attempts to use the wrong version of boost once in a while. I've restarted the tests; they should come out fine now.

[jslee02]

Yes, it keep repeating this failure. Usually rebuilding resolves it.

Beside on the Travis-CI issue, isn't there a way to mark bullet as a optional dependency?

[mkoval]

Catkin has no concept of optional dependencies. Instead, the convention is to split the code that has each dependency into a separate Catkin package. This is one reason why many projects consist of many small packages, instead of one large package with optional dependencies.

This is, in my opinion, not a great convention. But there's not much we can do it about it.

[scpeters]

I noticed this because I have a system install of bullet with double precision in /usr/local and I was also building bullet in a catkin workspace with dart using single precision. Without the package.xml change, they both built at the same time, so that dart found the double precision version in /usr/local with pkg-config, but tried to link against the one in the catkin workspace. It's definitely an edge case, but hopefully you don't mind the change.

[mxgrey]

I think in the future we have plans to split DART's homogeneous library setup into a set of smaller libraries where each of the smaller libraries will be split out based on their external dependencies. We'll be keeping everything in a single repo with the same source tree setup that we have now, but we'll have more granularity in the build system. At that point, we may want to make a separate Catkin package manifest for the dart-bullet extension library. That being said, I vaguely remember Catkin being strict about keeping separate packages in separate directories, so I don't know for sure if this would work.

[mkoval]

That being said, I vaguely remember Catkin being strict about keeping separate packages in separate directories, so I don't know for sure if this would work.

It may be possible to work around this with inventive use of add_subdirectory in CMake and "stub" Catkin packages that just invoke the main CMakeLists.txt file with different options. However, I'm not sure if this is worth the trouble.

It's critical that no dependencies are missing in the package.xml file because this could, like @scpeters reported, can lead to packages being built in the wrong order (e.g. DART before Bullet). However, there is not much harm in exhaustively listing all optional dependencies in the package.xml file because Catkin ignores unknown dependencies when determining the build order.

There are only two downsides:

1. the ROS buildfarm would add those dependencies to the Debian package (this is not an issue for DART, which uses it's own Travis:CI instance)
2. rosdep install will install those dependencies by default

Both of these are pretty minor issues, in my opinion.

[jslee02]

👍 I don't have objection to add bullet dependency to Catkin package manifest for DART 5.1. Once we reorganize DART libraries as proposed in #477, we can consider how to set up separate Catkin packages. But it seems fine to me for now.

[jslee02]

It seems to be okay to merge this. Just waiting another day for more comments.