Add test dependency on rosidl_typesupport_c_packages #110
Conversation
Since we are generating interfaces for testing. Signed-off-by: Jacob Perron <jacob@openrobotics.org>
Signed-off-by: Jacob Perron <jacob@openrobotics.org>
jacobperron
changed the title
| <test_depend>rosidl_typesupport_connext_c</test_depend> | ||
| <test_depend>rosidl_typesupport_introspection_c</test_depend> | ||
| <test_depend>rosidl_typesupport_fastrtps_c</test_depend> | ||
| <!-- end of group dependencies added for bloom --> |
There was a problem hiding this comment.
Q: should we also add <group_depend>rosidl_typesupport_c_packages</group_depend> ?
There was a problem hiding this comment.
What for? Above we already buildtool_export depend on rosidl_typesupport_c which has such a group dependency.
There was a problem hiding this comment.
Because it seems like it is necessary for this package to build (specifically the tests). It seems more correct than relying on it as a transitive dependency.
There was a problem hiding this comment.
But a group dependency is not limited to tests but acts as a build as well as run dependency.
There was a problem hiding this comment.
Yeah, so maybe it's best to leave it at the separate test_depend tags.
There was a problem hiding this comment.
Should instead https://github.com/ros2/rosidl_typesupport/blob/3c15da21132af47d73a3fea00f2d99352e647f11/rosidl_typesupport_c/package.xml#L19-L20 be modified to include rosidl_typesupport_fastrtps_c and this package woulld only test_depend on rosidl_typesupport_c (as it also does buildtool_export_depend on)?
There was a problem hiding this comment.
Maybe I'm wrong, but I don't think the issue is that rosidl_typesupport_fastrtps_c is missing. I think the issue is that no C typesupports are depended on here (I presume the build depends of rosidl_typesupport_c don't get installed when building rosidl_generator_py).
I guess adding one or more <build_export_depend> tags for C typesupport packages in rosidl_typesupport_c would fix the build though.
There was a problem hiding this comment.
I guess adding one or more
<build_export_depend>tags for C typesupport packages inrosidl_typesupport_cwould fix the build though.
That doesn't seem right from the perspective of that package.
How about we keep the patch as it is right now and see if it fixes the build as desired?
There was a problem hiding this comment.
Sure. Considering that the PR job passes (when past PR jobs failed) I'm fairly confident it will work 🤞
delete-merged-branch
bot
deleted the
jacob/default_generators_test_depend
branch
Since we are generating interfaces for testing.
This is aiming to fix build failures on build.ros2.org, for example: http://build.ros2.org/job/Fbin_uF64__rosidl_generator_py__ubuntu_focal_amd64__binary/7