Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added missing package dependencies of tf2/utils #509

Conversation

MarcelSoler
Copy link

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.

@MarcelSoler MarcelSoler requested a review from tfoote as a code owner May 4, 2021 15:12
@mikepurvis
Copy link
Member

mikepurvis commented Jun 30, 2021

@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
Copy link
Member

tfoote commented Jul 8, 2021

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
Copy link
Member

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
Copy link
Member

tf2_2d has been resolved as of locusrobotics/tf2_2d#4.

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

ooeygui pushed a commit to ms-iot/geometry2 that referenced this pull request Oct 12, 2022
* Drop PyKDL dependency

Signed-off-by: Florian Vahl <[email protected]>
Co-authored-by: Chris Lalancette <[email protected]>
@MarcelSoler MarcelSoler deleted the MarcelSoler/fix_utils_dependencies branch January 10, 2023 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants