Errors in package.xml doesn't stop action, failed dependency fullfilment revealed later at build

[michalfita]

Description

  + rosdep install -r --from-paths src/iodztm29wur/ros2_dummy_tools --ignore-src --skip-keys 'rti-connext-dds-5.3.1 rti-connext-dds-6.0.1 ' --rosdistro iron -y
  Error(s) in package '/__w/ros2_dummy_tools/ros2_dummy_tools/ros_ws/src/iodztm29wur/ros2_dummy_tools/./package.xml':
  Error(s):
  - The generic dependency on 'rclcpp_components' is redundant with: build_depend
  + true

This failure in invocation of bash -c,./install_rosdeps.sh iron continues w/o stopping.

Later CMake fails as cannot find config files for dependencies required.

Expected Behavior

Action shall stop on incorrect package.xml and let CI report this as error.

Actual Behavior

As shown above, it carries on despite mistake in packages.xml then fails CMake with cryptic:

 CMake Error at CMakeLists.txt:66 (find_package):
    By not providing "Findnaporet_foobar_msgs.cmake" in CMAKE_MODULE_PATH this
    project has asked CMake to find a package configuration file provided by
    "naporet_foobar_msgs", but CMake did not find one.
  
    Could not find a package configuration file provided by
    "naporet_foobar_msgs" with any of the following names:
  
      naporet_foobar_msgsConfig.cmake
      naporet_foobar_msgs-config.cmake
  
    Add the installation prefix of "naporet_foobar_msgs" to CMAKE_PREFIX_PATH
    or set "naporet_foobar_msgs_DIR" to a directory containing one of the above
    files.  If "naporet_foobar_msgs" provides a separate development package or
    SDK, be sure it has been installed.
  
  
  -- Configuring incomplete, errors occurred!
  See also "/__w/ros2_dummy_tools/ros2_dummy_tools/ros_ws/build/naporet_dummy_tools/CMakeFiles/CMakeOutput.log".
  ---
  --- stderr: naporet_dummy_tools
  CMake Error at CMakeLists.txt:66 (find_package):
    By not providing "Findnaporet_foobar_msgs.cmake" in CMAKE_MODULE_PATH this
    project has asked CMake to find a package configuration file provided by
    "naporet_foobar_msgs", but CMake did not find one.
  
    Could not find a package configuration file provided by
    "naporet_foobar_msgs" with any of the following names:
  
      naporet_foobar_msgsConfig.cmake
      naporet_foobar_msgs-config.cmake
  
    Add the installation prefix of "naporet_foobar_msgs" to CMAKE_PREFIX_PATH
    or set "naporet_foobbar_msgs_DIR" to a directory containing one of the above
    files.  If "naporet_foobar_msgs" provides a separate development package or
    SDK, be sure it has been installed.
  
  
  ---
  Failed   <<< naporet_dummy_tools [3.91s, exited with code 1]

To Reproduce

1. Leave repeated dependency in packages.xml
2. Run CI

System (please complete the following information)

• OS: Ubuntu Jammy, Ubuntu Noble
• ROS 2 Distro: Humble, Iron, Jazzy, Rolling

** For ROS 2 bugs - please attach a VCS.repos or Dockerfile if possible to help us reproduce the environment **

Additional context

We're heavy user of your actions, we rely on their trustworthiness. This problem fails the trust and takes time of inexperienced engineers to figure out what they did wrong.

[christophebedard]

This is because of the || true here:

action-ros-ci/src/action-ros-ci.ts

Line 205 in 2209690

)}" --rosdistro $DISTRO -y || true`;

.

There's a reason why we have that, but I can't remember exactly why right now. I'll look into it later this week. Maybe this could depend on an action input.

[christophebedard]

I'm having a bit of a hard time understanding what's going on. Can you provide more information about the error in the package.xml? What's missing/wrong? The error about rclcpp_components seems inconsequential. A full reproducer might be helpful if the use-case is complex.

From the build failure, it looks like naporet_foobar_msgs isn't found, which could be because (1) your package.xml doesn't declare a dependency on it or (2) because the package is not available (source code in workspace, or binary package). This would not catch (1) but might catch (2). However, we introduced the rosdep-check option for this kind of check. See https://github.com/ros-tooling/action-ros-ci#skip-rosdep-install. It runs rosdep check, but it currently only does so if skip-rosdep-install is set to true. This was meant to catch missing dependencies in environments that should already include all dependencies, e.g., Docker image. We could decouple it from skip-rosdep-install.

[michalfita]

It's simple: my package.xml had the same dependency (rclcpp_components) in <depend>...</depend> and <build_depend>...</build_depend> tags (merge artifact).

[christophebedard]

Huh, I just assume that, while not ideal/correct, it's fine, since I see redundant dependency declarations all the same. However, you are right, REP-149 does say that you shouldn't combine <depend> and <build_depend> (emphasis mine):

<depend> (multiple)
Declares a rosdep key or ROS package name that this package needs for multiple reasons. A <depend> tag is equivalent to specifying <build_depend>, <build_export_depend> and <exec_depend>, all on the same package or key. The <depend> tag cannot be used in combination with any of the three equivalent tags for the same package or key name.

I guess "cannot" there is interpreted and reported as an error by rosdep install. I agree that this should really fail with rosdep and not with the build, which I'm assuming fails because rosdep just didn't install anything, so there's missing dependencies. This is indeed a bit confusing.

So I went looking through the git history, and the || true has been there since the very first implementation: https://github.com/ros-tooling/action-ros-ci/blame/b08ee66c5236c0e9df3b57a86f851e9ca66956af/src/action-ros2-ci.ts#L66-L72. The comment specifically says:

    // For "latest" builds, rosdep often misses some keys, adding "|| true", to
    // ignore those failures, as it is often non-critical.

Some options:

1. Remove || true
   • While that may be the cleanest option, I'm hesitant to change this in case it does break existing workflows. Maybe we could just bump v0.3 -> v0.4 to make the change more evident, though.
2. Optionally use || true based on an action input
   • This would work, but it's yet another option
3. Rely on rosdep check: could you check if rosdep check also reports this error?
   • This would require allowing users to set rosdep-check to true even if skip-rosdep-install is not true

What do you think?

[michalfita]

You're asking about my expert informed opinion, why I'm just dumb forced user of ROS2 (which I don't even like) and I accidentally found this failed on CI, instead of spending hours cracking my head why CMake fails on missing config option while the build works locally.

My uninformed bet based on hunch coming from experience would be to remove || true to stop masking problems.