Dependecy issues between tf2 and tf2_geometry_msgs

[Crusty82]

If you look closely, you can see that

tf2/include/tf2/impl/utils.h includes <tf2_geometry_msgs> and tf2_geometry_msgs/include/tf2_geometry_msgs/tf2_geometry_msgs.h includes

So there is a cycle there. I am forced to compile these packages via a different build system, and my system dies with a cyclic dependency error. Also note how tf2 sneakily includes tf2_geometry_msgs without actually requiring the package in CMakeLists or package.xml. I fail to understand how this can build in the first place, but it surely doesn't build for me.

[tfoote]

This was flagged in #170 There's a proposed fix to call it out specifically but that explicitly triggers the circular dependency in the main build. #170 so can't be merged. The main solution is that we need to move that implementation.

[Crusty82]

Thanks for replying this quickly Tully, and sorry I missed issue #170. I will locally fix this then by moving functionality from tf2 into tf2_geometry_msgs. Thanks!

[tfoote]

No problem. If you have a clean solution a PR would be appreciated. I think it it's likely that the template specializations in the impl can move to tf2_geometry_msgs and then anyone with that as a dependency could use them the same way they always have.

[trainman419]

I just ran into this too, and I also solved it by moving the functions that manipulate geometry_msgs types into tf2_geometry_msgs.

I can submit my changes as a PR if you'd like, but it's an API change and since I'm only testing against a small part of ROS and I'm not using cmake to build ROS, so I can't confirm that this fix is valid for the rest of the ROS ecosystem.

[tfoote]

@trainman419 If you have the moved implementation could you add a backwards compatible API placeholder that forwards to the new location and gives a deprecation warning? That deprecated API will still cause the dependency issue, but we can remove it in a future rosdistro and we can put a very specific warning that hopefully people will find the workaround quickly if they run into it.

[mintar]

:+1:

we can remove it in a future rosdistro

That rosdistro better be Noetic, since it's the last one! :)

[jspricke]

We got bugged by this in Debian as well: https://bugs.debian.org/916479. @trainman419 do you plan to send the PR soon?

[trainman419]

Looks like my changes didn't end up being necessary for our project, and didn't make it into our repository. I will not be submitting a PR for these changes (I don't have any way to test the changes requested by @tfoote , either).

[ayrton04]

This is causing one of our dev jobs to fail (though somehow the binary jobs are fine):

https://build.ros.org/job/Mdev__tf2_2d__ubuntu_bionic_amd64/1/consoleFull

17:55:25 /opt/ros/melodic/include/tf2/impl/utils.h:18:10: fatal error: tf2_geometry_msgs/tf2_geometry_msgs.h: No such file or directory
17:55:25  #include <tf2_geometry_msgs/tf2_geometry_msgs.h>
17:55:25           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Since tf2 doesn't (and can't) have tf2_geometry_msgs in its dependencies, tf2_geometry_msgs are not included in the build. I suppose the easiest thing for us to do is just include tf2_geometry_msgs as a dependency directly.

[tfoote]

Yes, if you include tf2_geometry_msgs as a dependency it will be fine. You should do this if you use utils.h which is the only place that uses this. It's header only and should probably be refactored out.

[jspricke]

@tfoote I did the refactoring in #357 would be great to get some feedback on this.