-
Notifications
You must be signed in to change notification settings - Fork 128
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
Add missing test dependency to package.xml file #678
Conversation
Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
@@ -26,6 +26,7 @@ | |||
<exec_depend>rosidl_parser</exec_depend> | |||
<exec_depend>rcutils</exec_depend> | |||
|
|||
<test_depend>ament_cmake_cppcheck</test_depend> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Theoretically, this shouldn't be necessary; we already have a dependency on ament_lint_common, and that already contains ament_cmake_cppcheck
: https://github.com/ament/ament_lint/blob/master/ament_lint_common/package.xml .
But maybe I'm missing something, so can you explain why you think this is necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw the error in the install job and saw that the line failing is this one:
find_package(ament_cmake_cppcheck REQUIRED) |
Yeah, I saw ament_lint_common
but I'm not sure if it works internally in a different manner for the install jobs.
I think the PR introducing this regression is #662. Tagging @sloretz to see if he has an idea about why this could be happening.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the CI log:
+ rosdep install --from-paths . -r --ignore-src --rosdistro=rolling --default-yes
ERROR: the following packages/stacks could not have their rosdep keys resolved
to system dependencies:
gazebo_ros: No definition of [tinyxml_vendor] for OS version [focal]
gazebo_plugins: No definition of [tf2_ros] for OS version [focal]
gazebo_msgs: No definition of [ament_lint_common] for OS version [focal]
It looks like ament_lint_common
didn't get installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- rosdep install --from-paths . -r --ignore-src --rosdistro=rolling --default-yes
ERROR: the following packages/stacks could not have their rosdep keys resolved
to system dependencies:
gazebo_ros: No definition of [tinyxml_vendor] for OS version [focal]
gazebo_plugins: No definition of [tf2_ros] for OS version [focal]
gazebo_msgs: No definition of [ament_lint_common] for OS version [focal]
Oh, thanks for pointing that out. Note that this is never going to work; while the packages still exist for Rolling-on-Focal, the rosdep database now only serves Rolling-on-Jammy. So I think that is the main issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback, I'll talk with Jose about how to move forward with this. I imagine we should be moving this job to jammy instead.
I think that we can close this PR. Fixes for gazebo_ros_pkgs on Jammy are ready in ros-simulation/gazebo_ros_pkgs#1373 and problems detailed here I think that have to do with the change for Rolling from Focal to Jammy in the rosdep database. |
Closing, reopen if needed please. |
This should fix the build regressions in the
ros2_gazebo_pkgs-ci-default_rolling-focal-amd64
jobs of the buildfarm:https://build.osrfoundation.org/job/ros2_gazebo_pkgs-ci-default_rolling-focal-amd64/96/
With error:
Signed-off-by: Jorge Perez jjperez@ekumenlabs.com