search

ros-tooling / setup-ros

Github Action to set up ROS 2 on hosts
Apache License 2.0
87 stars 41 forks source link

documentation about required-ros-distributions is unclear #494

Open christian-rauch opened 2 years ago

christian-rauch commented 2 years ago

Description

The documentation for required-ros-distributions mentions:

required-ros-distributions installs the desktop variant for that distribution. This option is not required, and should probably be avoided in most workflows

If I do not use required-ros-distributions then ROS has to be built from source:

The default behavior is to only install development tools. No ROS binary distribution is installed in this case. This setup should be used when ROS is built entirely from source.

Actual Behavior

According to the documentation, one can either build ROS entirely from source or install a binary distribution that already includes all dependencies of ros-$ROS_DISTRO-desktop.

required-ros-distributions is also still used in many examples, e.g. https://github.com/ros-tooling/setup-ros#iterating-on-all-ros-distributions-for-all-platforms.

Expected Behavior

To check correct dependency resolution, it must be possible to set up the binary ROS repo without installing any ROS packages, except generic packages such as colcon.

Further, required-ros-distributions should not be used in examples to not install the huge set of dependencies from ros-$ROS_DISTRO-desktop. Those are rarely required to run CI on a package and a package should define its own dependencies anyway.

christophebedard commented 2 years ago

We discussed this behaviour somewhat recently in https://github.com/ros-tooling/setup-ros/issues/439#issuecomment-1025247201 and added the part you quoted in #464

To check correct dependency resolution, it must be possible to set up the binary ROS repo without installing any ROS packages, except generic packages such as colcon.

This is already the case if you do not set required-ros-distributions, which is only used to install ros-${ROS_DISTRO}-desktop: https://github.com/ros-tooling/setup-ros/blob/dd88a18a51b173834c899cfc6cd7295a7fdeeba4/src/setup-ros-linux.ts#L152-L154

Further, required-ros-distributions should not be used in examples to not install the huge set of dependencies from ros-$ROS_DISTRO-desktop. Those are rarely required to run CI on a package and a package should define its own dependencies anyway.

I agree! I'd say this is mostly a documentation/examples issues then: as you mentioned, most/basic examples should be changed to not use required-ros-distributions. PRs are always welcome, otherwise I'll get to it soon.

christian-rauch commented 2 years ago

Ok, then just removing required-ros-distributions is doing what I need.

What is the purpose of required-ros-distributions then? Why would someone install ros-$ROS_DISTRO-desktop in the CI? Can't we just remove that option entirely? :-)

christophebedard commented 2 years ago

What is the purpose of required-ros-distributions then? Why would someone install ros-$ROS_DISTRO-desktop in the CI?

I think it was originally intended to speed up CI, since then ROS 2 does not need to get built from source every time. However, as mentioned in https://github.com/ros-tooling/setup-ros/issues/439#issuecomment-1025247201, installing the desktop variant takes quite a long time, and users should just rely on action-ros-ci to use rosdep for dependencies.

Can't we just remove that option entirely? :-)

Not sure about completely removing it. We might have users that don't use action-ros-ci and use setup-ros to install the desktop variant. We can change all examples to not use it, and only mention it at the bottom of the README with a clear disclaimer.