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

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

Open
clalancette opened this issue Apr 9, 2021 · 2 comments
Open

Comments

@clalancette
Copy link
Contributor

clalancette commented Apr 9, 2021

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

cottsay commented Apr 9, 2021

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

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):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants