Dependency from rmw_cyclonedds_cpp on fastrtps via rmw_dds_common

[rotu]

I think this created a build mess by introducing a dependency on FastRTPS.

colcon graph --packages-up-to rmw_cyclonedds_cpp --packages-above fastrtps   
fastrtps                         +**.......
rosidl_typesupport_fastrtps_cpp   +*..***..
rosidl_typesupport_fastrtps_c      +*..**..
rosidl_typesupport_c                +****..
rosidl_generator_py                  + **..
rosidl_typesupport_cpp                +**..
rosidl_default_generators              + * 
rosidl_default_runtime                  +*.
rmw_dds_common                           +*
rmw_cyclonedds_cpp                        +

Originally posted by @rotu in ros2/rmw_cyclonedds#145 (comment)

[dirk-thomas]

The graph shows all dependencies considered for the topological order. That includes group dependencies.

There is no direct dependency on FastRTPS in this package which you can verify by looking at the package manifest.

So you might want to take a step back describing what the problem is you are having and then finding the actual reason for it.

[rotu]

There is a transitive dependency on FastRTPS.

Before ros2/rmw_cyclonedds#145, fastrtps wasn't being built as part of rmw_cyclonedds_cpp CI: https://github.com/ros2/rmw_cyclonedds/runs/596733361?check_suite_focus=true

Afterwards, it was: https://github.com/ros2/rmw_cyclonedds/runs/609354227?check_suite_focus=true#step:4:3049

[dirk-thomas]

The order comes from the fact that this package contains messages and therefore depends on all type support packages. There is no explicit dependency on FastRTPS. You can easily verify that by building RMW CycloneDDS in a workspace without FastRTPS.

[ivanpauno]

@rotu it doesn't add a dependency.
rmw_dds_common depends on rosidl_default_generators, which has a group dependency in typesupport packages.

For that reason, if rosidl_typesupport_fastrtps_c or rosidl_typesupport_fastrtps_cpp is present in your workspace, colcon will build them before rmw_dds_common (and of course, it will build fastrtps before too, as it's a dependency of those packages).
But that doesn't mean they are a required dependency.

If you want to avoid building fastrtps in the github actions used in rmw_cyclonedds, what you can do is to add an AMENT_IGNORE file in rosidl_typesupport_fastrtps, or avoid checking out it.

[rotu]

Okay. It seems like I'm confusing two concepts: the list of "required dependencies" and the build order in which those packages must be built. What is the proper way to tell whether something is a "required dependency" versus an optional one?

@thomas-moulard It seems action-ros-ci makes the same mistake:
https://github.com/ros-tooling/action-ros-ci/blob/b22b3bdcfe5914903491fc7ee9a7a94efa33e2c5/src/action-ros-ci.ts#L221-L229

It also seems action-ros-ci doesn't have the same logic to disable RMW packages, which perhaps should be moved to the rmw package in some way instead of being living in CI:
https://github.com/ros2/ci/blob/9ce633a9411e964e2c54c2940da4e566388efbd0/ros2_batch_job/__main__.py#L699-L743

[ivanpauno]

required dependency

IIUC, group dependencies (which are marked with group_depend tag), are always optional.
I don't know if we have another special tag for optional dependencies, I think we don't.

I'm not sure of how the ros ci actions can be configured, but having a way of ignoring specific packages/repositories sounds like a good idea.

[rotu]

group dependencies (which are marked with group_depend tag), are always optional.

That would be a good thing to clarify in https://www.ros.org/reps/rep-0149.html#dependency-groups and/or add an 'optional' flag on dependencies.

having a way of ignoring specific packages/repositories sounds like a good idea

I think that makes sense. Right now there's not spot for "additional colcon args" like there is on Travis.

For multiple RMW backends, there exists RMW_IMPLEMENTATION_DISABLE_RUNTIME_SELECTION. It also might be a good idea to either share this option with rosidl or add a new option to change only use certain typesupports.

[dirk-thomas]

It also might be a good idea to either share this option with rosidl or add a new option to change only use certain typesupports.

This is not something we are planning to implement. The user chooses what they want by what is in a workspace (and not ignored during the build).

Since there doesn't seem to be an issue in this repository I am going to close the ticket. Please feel free to continue commenting.

[rotu]

This is not something we are planning to implement.

What's the rationale for allowing building without dynamic RMW selection and why doesn't it apply to typesupports?

Oops. I didn't realize that excluding other implementations from rmw_implementation does not exclude them from --packages-up-to rmw_implementation so the benefits of such a feature would not be as great as I hoped.

The user chooses what they want by what is in a workspace (and not ignored during the build).

What would be the correct way to tell colcon build to ignore all packages not required rmw_cyclonedds?

[dirk-thomas]

What would be the correct way to tell colcon build to ignore all packages not required rmw_cyclonedds?

Something like --packages-ignore-regex .*fastrtps.* .*connext.*

[rotu]

What would be the correct way to tell colcon build to ignore all packages not required rmw_cyclonedds?

Something like --packages-ignore-regex .*fastrtps.* .*connext.*

That ignores fastrtps and connext, it doesn't filter based on rmw_cyclonedds_cpp's package requirements. In general, it seems like a pretty core feature of a build tool to be able to figure out which packages some package depends on...

[dirk-thomas]

it seems like a pretty core feature of a build tool to be able to figure out which packages some package depends on...

Please feel free to describe how colcon should determine that based on the available information from the package manifests.

[rotu]

A package's required dependencies seems to be the transitive closure of its Dependency tags, excluding its Group dependency tags (which as @ivanpauno said above, are always optional).

[dirk-thomas]

Group dependencies are not necessarily optional. There are cases where you e.g. need at least one of package from the group in order to work properly. That semantic isn't encoded in the manifest because the community wasn't able to reach a consensus when this was proposed (I don't have a link to that discussion at hand).

Therefore I don't think an option to just ignore group dependencies is sufficient.

[ivanpauno]

Group dependencies are not necessarily optional.

This is a great point.

rmw_dds_common depends on rosidl_default_generators, which has a group dependency in typesupport packages.

The group dependency mentioned above needs AT LEAST one package.
For rmw_cyclonedds_cpp, only rosidl_typesupport_introspection_cpp is needed.
For rmw_fastrtps_cpp, only rosidl_typesupport_fastrtps_cpp is needed.

I don't think that information can be encapsulated in a package.xml (i.e. I don't think that ros-infrastructure/rep#252 can provide a solution).
IMO, the only way to have a minimal build for one specific rmw implementation is to delete the others from the workspace.

[rotu]

You're right that my suggested fixes for ros-infrastructure/rep#252 would not fully encode the dependency "at least one of...".

It would enable replacing this part of the build process:

rosdep install ... --skip-keys "console_bridge fastcdr fastrtps libopensplice67 libopensplice69 rti-connext-dds-5.3.1 urdfdom_headers"

with something like:

rosdep install ... --disable-optional

It is possible to do this today with e.g. <member_of_group condition='($DISABLE_OPTIONAL != 1) or ($ENABLE_CONNEXT == 1)'>.

You're right that today, the only way to have a minimal build is to delete packages. I don't think we should settle for that status quo.

[rotu]

FYI: here's the proposed fix in the cyclone rmw.
ros2/rmw_cyclonedds#168