search

ros-infrastructure / ros_buildfarm

ROS buildfarm based on Docker
Apache License 2.0
81 stars 96 forks source link

Always install workspace package in ROS 2 devel jobs #955

Closed cottsay closed 2 years ago

cottsay commented 2 years ago

This will revert devel jobs to the previous behavior while leaving the CI jobs workspace-less.

As an alternative, we could look through the resolved list of packages and only install the workspace package if we see any other packages in the list that start with ros-$ROSDISTRO-*. It's a bit of a violation, though, because rosdep is responsible for transforming package names into debian names.

Fixes: #950

nuclearsandwich commented 2 years ago

As an alternative, we could look through the resolved list of packages and only install the workspace package if we see any other packages in the list that start with ros-$ROSDISTRO-*. It's a bit of a violation, though, because rosdep is responsible for transforming package names into debian names.

I think we could do that. Just as we could also check if the only rosdistro resolved rosdep keys are in that bootstrap layer below ros_workspace: ament_package, ament_cmake_core, and ros_workspace itself. But given that ROS 1 jobs unconditionally install catkin for workspace packages, I think we can do the same for ROS 2 distributions. In the very long term I'd like to make this bootstrap layer part of the specification somehow so that we can define what's in it and how it should behave more globally rather than needing to do it ad-hoc through each tool.

cottsay commented 2 years ago

I think we could do that. Just as we could also check if the only rosdistro resolved rosdep keys are in that bootstrap layer below ros_workspace: ament_package, ament_cmake_core, and ros_workspace itself. But given that ROS 1 jobs unconditionally install catkin for workspace packages, I think we can do the same for ROS 2 distributions. In the very long term I'd like to make this bootstrap layer part of the specification somehow so that we can define what's in it and how it should behave more globally rather than needing to do it ad-hoc through each tool.

All of this sounds reasonable. I think that this change is a good mitigation until we're ready to give some of that a try.