ros / geometry2

A set of ROS packages for keeping track of coordinate transforms.
190 stars 275 forks source link

added missing package dependencies of tf2/utils #509

Closed MarcelSoler closed 1 year ago

MarcelSoler commented 3 years ago

tf2/utils.h depends on tf2_geometry_msgs https://github.com/ros/geometry2/blob/noetic-devel/tf2/include/tf2/impl/utils.h#L18 So it should be added as a package dependency.

mikepurvis commented 3 years ago

@tfoote This is needed for the Nix build as well, since it's very strict about dependencies.


Oh ugh, I see the real issue here— it's a mutual build-depend since tf2_geometry_msgs already depends on on tf2. So an actual architectural change is needed in order to properly resolve this.

Maybe since that header doesn't actually get compiled in the tf2 package, the proper fix is just for it to be a build-export/run depend?

tfoote commented 3 years ago

Yeah, we can't add this as it will induce a circular dependency. The build-export dependency is probably the right dependency. Since it's a header only I think the build-export may actually be enough without the run dependency.

mikepurvis commented 3 years ago

The catkin_package(CATKIN_DEPENDS xxx) macro can only accept build_depends, though, right? The real issue here is dependent packages that need to explicitly and unexpectedly pull in tf2_geometry_msgs to get this header— a public example I hit was tf2_2d from Locus:

https://github.com/locusrobotics/tf2_2d/blob/bf6d20a53ae5cbdde22c35e790917faea25189ac/CMakeLists.txt#L2-L11

This works fine when building against a base with a merged include, but when built in a strict environment like Nix, the explicit dep on tf2_geometry_msgs must be patched in.

mikepurvis commented 3 years ago

tf2_2d has been resolved as of https://github.com/locusrobotics/tf2_2d/pull/4.

This ticket should probably be closed as there are other tickets like #275 and #170 which have discussed this at length already.