-
Notifications
You must be signed in to change notification settings - Fork 125
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
Splitted rosidl_generator_c and rosidl_generator_cpp in two: rosidl_generator_x and rosidl_runtime_x #443
Comments
It would make it easier to review these PRs if you could reference at least one specific source line in the packages where new dependencies are added to clarify why they are added. It might also be good to clarify - when runtime packages are added - why generators are still needed. |
This was my fault I didn't remove the rosidl_generator_c/cpp dependencies. I revisited all the changes and removed this dependencies |
I have been checking rcl, rcl_lifecycle, rcl_action, rclcpp and rclcpp_action all of them make use of rosidl_generator ( with the new packages should be rosidl_runtime) but I think this dependecy is not need it, the reason why it's because rosidl_generator/rosidl_runtim is used where a message packages it's being used too. Some examples: RCLCPP: https://github.com/ros2/rclcpp/blob/master/rclcpp_action/CMakeLists.txt#L36 @wjwwood and @dirk-thomas what do you think? |
Please double check that with all the PRs the actual goal (avoiding a runtime dependency on any generator package) has been achieved. |
I have been reproducing the failure in my local MAC OS machine. if I used |
The Without that dependency CMake tries to build the test before the code generator was run which creates the used header files. |
The packages depend on
The packages depend on
|
I guess the dependencies still need to be removed from the two introspection packages then to achieve the stated goal, right? |
The packages depend on
The packages depend on
|
The packages depend on
The packages depend on
|
Please update all PRs to update / revert changes to dependency types of the following kind, e.g. ros2/rmw_dds_common#10: Since the added |
Also please update the list of all related PRs somewhere in this ticket - in a single place (rather than having to scroll through the whole thread). |
These warnings on Windows were already fixed, but I didn't update the branch in the PR
|
In MacOS
Windows
|
This issue is to track all the PRs that are need it to compile ros2.repos with the changes introduced in this PR split #442. We will have these packages:
The text was updated successfully, but these errors were encountered: