search

ros-infrastructure / ros_buildfarm

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

Not all test dependencies seem to be getting installed in Rci jobs #870

Open clalancette opened 3 years ago

clalancette commented 3 years ago

This bug is a bit nebulous because I don't understand all of the details, but I'll explain what I know.

https://build.ros2.org/view/Rci/job/Rci__nightly-cyclonedds_ubuntu_focal_amd64/289/ failed to build because CycloneDDS requires cunit to build and run tests. This is properly declared in the package.xml of CycloneDDS: https://github.com/eclipse-cyclonedds/cyclonedds/blob/3108aff3ee8c8b4cdb418441d1fe0925de3ef273/package.xml#L22 .

If we look carefully at https://build.ros2.org/view/Rci/job/Rci__nightly-cyclonedds_ubuntu_focal_amd64/289/consoleFull#console-section-16 , we see that libcunit-dev is identified as one of the test dependencies. Further, in that same section, it is resolved from a rosdep key to an Ubuntu package name (libcunit1-dev). However, it seems like it is never installed, as there are no mentions of that package ever again in that logging output until CycloneDDS fails to build.

Curiously, if you look at cppunit, it is in a similar situation. It is declared, resolved to an Ubuntu package name, and then never installed.

That's as much as I know right now, but this does seem like a bug somewhere in this pipeline that we should probably track down.

cottsay commented 3 years ago

To summarize a discussion @clalancette and I had on this topic, this is actually intended behavior.

The CI jobs actually build the packages twice. In the first pass, the test dependencies are not present and the tests are disabled using BUILD_TESTING. The second pass first installs the test dependencies, then re-builds with BUILD_TESTING enabled.

This outlines the difference between what should be declared as a <build_depend> in the package.xml, and what should be a <test_depend>. In this particular case, a CMakeLists.txt change was necessary to make the package in question respect the BUILD_TESTING value when looking for dependencies that were only necessary for testing. The dependencies were declared correctly in terms of when they SHOULD have been necessary, but the CMakeLists.txt didn't reflect that.

Without question, this circumstance could be more discoverable. It is definitely not obvious why the test dependencies weren't present during the build. Maybe when the install list is considered, if test dependencies are omitted, they should be dumped into the log and called out as omitted so that when searching the log for the name of the missing package, someone could stumble across the point at which they were discluded from installation.

We should keep this bug open as a reminder until we have changed the messaging in the logs somehow.

clalancette commented 3 years ago

Without question, this circumstance could be more discoverable. It is definitely not obvious why the test dependencies weren't present during the build. Maybe when the install list is considered, if test dependencies are omitted, they should be dumped into the log and called out as omitted so that when searching the log for the name of the missing package, someone could stumble across the point at which they were discluded from installation.

Yeah, maybe we can just change the second output of

Resolved the dependencies to the following binary packages:

to something like:

Resolved the test dependencies to the following binary packages (not installed until second pass):