Skip to content
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

documentation about required-ros-distributions is unclear #494

Open
christian-rauch opened this issue May 30, 2022 · 3 comments
Open

documentation about required-ros-distributions is unclear #494

christian-rauch opened this issue May 30, 2022 · 3 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed

Comments

@christian-rauch
Copy link

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.

@christian-rauch christian-rauch added the bug Something isn't working label May 30, 2022
@christophebedard christophebedard added the documentation Improvements or additions to documentation label May 30, 2022
@christophebedard
Copy link
Member

christophebedard commented May 30, 2022

We discussed this behaviour somewhat recently in #439 (comment) 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:

for (const rosDistro of utils.getRequiredRosDistributions()) {
await apt.runAptGetInstall([`ros-${rosDistro}-desktop`]);
}

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
Copy link
Author

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
Copy link
Member

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 #439 (comment), 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.

@christophebedard christophebedard added the help wanted Extra attention is needed label May 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants