Only a fraction of repos is built in CI

[ahans]

Bazel 8.0.0 flips --incompatible_disallow_empty_glob to true. We have quite a few empty globs. I was about to prepare a PR that fixes them for bazel test //... in the root of the repo, when I figured I should also test in examples. Guess what, that found more issues. And then I realized that we actually don't even check if everything builds. We should change that, shouldn't we? I'd propose to have a meta cc_library as well as a py_library in ros2/test/BUILD.bazel that depends on all cc_ and py_ targets, respectively. A bazel test //... would then also build that. It'd be probably ok to maintain those targets manually. Alternatively, something based on bazel query '//external:*' could probably work as well. Wdyt?

[lalten]

Could also just remove this: https://github.com/mvukov/rules_ros2/blob/b8695fb1a87faabefd8bb66d1fbae93bc9250dd8/.bazelrc#L43-L44

[ahans]

Could also just remove this:

rules_ros2/.bazelrc

Lines 43 to 44 in b8695fb

Don't bother building targets which aren't dependencies of the tests.

test --build_tests_only

Oh, I wasn't even aware we had this. But removing this doesn't help here, because //... still doesn't include the external ros2_* repos.

[ahans]

This could be doing the trick:

bazel query '//external:*' | grep ":ros2_" | sed 's#//external:\(ros2_.*\)#\@\1//...#' | xargs bazel build

In addition to the issues with empty globs, there seems to be a real build failure as well:

ERROR: /home/ahans/.cache/bazel/_bazel_ahans/c02197da98e070ec8a47cb18b5c98285/external/ros2_rclcpp/BUILD.bazel:131:17: Linking external/ros2_rclcpp/librclcpp_components.so failed: (Exit 1): gcc failed: error executing CppLink command (from target @@ros2_rclcpp//:rclcpp_components) /usr/bin/gcc @bazel-out/k8-fastbuild/bin/external/ros2_rclcpp/librclcpp_components.so-2.params

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
ld.lld: error: duplicate symbol: main
>>> defined at component_container.cpp
>>>            bazel-out/k8-fastbuild/bin/external/ros2_rclcpp/_objs/rclcpp_components/component_container.pic.o:(main)
>>> defined at component_container_isolated.cpp
>>>            bazel-out/k8-fastbuild/bin/external/ros2_rclcpp/_objs/rclcpp_components/component_container_isolated.pic.o:(.text+0x0)

ld.lld: error: duplicate symbol: main
>>> defined at component_container.cpp
>>>            bazel-out/k8-fastbuild/bin/external/ros2_rclcpp/_objs/rclcpp_components/component_container.pic.o:(main)
>>> defined at component_container_mt.cpp
>>>            bazel-out/k8-fastbuild/bin/external/ros2_rclcpp/_objs/rclcpp_components/component_container_mt.pic.o:(.text+0x0)
collect2: error: ld returned 1 exit status

[mvukov]

I made and merged https://github.com/mvukov/rules_ros2/pull/433. More fixes to come up in follow-ups. In follow-ups we can add more tests to increase test coverage and make the codebase more reliable. I'm not that interested in bazel query '//external:*' | grep ":ros2_" | sed 's#//external:\(ros2_.*\)#\@\1//...#' | xargs bazel build, CI should only do bazel test //..., as it's doing right now.

[ahans]

I made and merged #433. More fixes to come up in follow-ups.

Thanks, although I would have preferred to work with you on my PR (#432) to find a good solution.

In follow-ups we can add more tests to increase test coverage and make the codebase more reliable. I'm not that interested in bazel query '//external:*' | grep ":ros2_" | sed 's#//external:\(ros2_.*\)#\@\1//...#' | xargs bazel build, CI should only do bazel test //..., as it's doing right now.

What exactly is it you don't like about this approach? A large portion of rules_ros2 is about providing external repos to build ROS 2 components, so I'd say it makes sense to ensure they build successfully. Unit tests make sense when there's more than just providing a *_library. Testing functionality is handled in upstream ROS 2 repos, so I'm not sure doing that here adds that much value. If you don't like the bazel query ... command because that can't be done easily locally, what about the meta targets and dropping the --build_tests_only (TBH, I think it's a sane default that bazel test //... also implies building, not sure why you would opt-out of this here)?

[lalten]

Perspectively Workspace builds will be replaced with Bzlmod anyways. It would then make sense to externalize many of the provided ros2 repos into their own modules then. Putting everything into the BCR will test their builds. One step in between could be to move these modules into their own .bazelignore'd dirs, add MODULE.bazel (And Workspace?) there, and test that they build (plus some integration test?). rules_rust is doing that with their "extensions" like Bindgen today.