Skip to content

Fix broken ROS2 C++ build in CI due to ament#2580

Merged
lsk567 merged 1 commit into
masterfrom
fix-ros2-cpp-build
Jan 21, 2026
Merged

Fix broken ROS2 C++ build in CI due to ament#2580
lsk567 merged 1 commit into
masterfrom
fix-ros2-cpp-build

Conversation

@lsk567

@lsk567 lsk567 commented Jan 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes the ROS2 C++ build integration by correcting how reactor-cpp exports its CMake configuration for the ament/colcon ecosystem. Also fixes a pre-existing bug in the C++ coverage collection CI step.

Problem

The ROS2 C++ test was failing because downstream packages (the generated LF ROS2 packages) could not find the reactor-cpp library via CMake's find_package(reactor-cpp REQUIRED).

Root cause: The reactor-cpp library was being built and installed correctly by colcon, but its CMake export configuration wasn't set up properly for the ament/colcon ecosystem. Specifically:

  • The install(EXPORT ...) command was in lib/CMakeLists.txt, but ament requires it to be in the root CMakeLists.txt alongside ament_package() for proper integration
  • The export lacked a NAMESPACE, which meant the generated CMake config file didn't create proper imported targets
  • The LF code generator's find_package(reactor-cpp) couldn't locate the package because ament_auto_find_build_dependencies() doesn't automatically find packages installed in sibling directories of a colcon workspace

Fix

Three coordinated changes were needed:

A. reactor-cpp CMakeLists.txt (root):

if (REACTOR_CPP_USE_AMENT)
  # Moved from lib/CMakeLists.txt - must be here for ament compatibility
  install(EXPORT ${LIB_TARGET} DESTINATION share/${LIB_TARGET}/cmake NAMESPACE ${LIB_TARGET}::)
  ament_export_targets(${LIB_TARGET} HAS_LIBRARY_TARGET)
  ament_export_include_directories(include)
  ament_export_libraries(${LIB_TARGET})
  ament_package()
endif()

B. reactor-cpp lib/CMakeLists.txt:

  • Removed the install(EXPORT ...) call (moved to root)

C. CppRos2PackageGenerator.kt (LF code generator):

// Added HINTS to find reactor-cpp in the colcon workspace install directory
find_package(reactor-cpp REQUIRED HINTS ${CMAKE_INSTALL_PREFIX}/../reactor-cpp/share/reactor-cpp/cmake)

// Changed to use namespaced target
target_link_libraries(${LF_MAIN_TARGET} reactor-cpp::reactor-cpp)

The key insight was that ament's build system requires the export installation and ament_package() to be in the same CMakeLists.txt file, and using namespaced targets (reactor-cpp::reactor-cpp) ensures proper linkage through CMake's imported target mechanism.

Additional Fix: C++ Coverage Collection

Fixed a pre-existing bug in the CI workflow where the coverage extraction step was looking for the wrong directory path:

  • Was: test/Cpp/src-gen/reactor-cpp-default/*
  • Now: test/Cpp/src-gen/reactor-cpp/*

Test plan

  • ROS2 C++ integration tests pass in CI
  • Regular C++ integration tests pass in CI
  • C++ coverage collection works correctly

🤖 Generated with Claude Code

@lsk567 lsk567 added the bugfix label Jan 20, 2026
@lsk567 lsk567 force-pushed the fix-ros2-cpp-build branch from eb01458 to 25cc564 Compare January 21, 2026 09:39
@lsk567 lsk567 changed the title Fix ROS2 C++ build by using consistent reactor-cpp package name Fix broken ROS2 C++ build in CI due to ament Jan 21, 2026
lsk567 added a commit to lf-lang/reactor-cpp that referenced this pull request Jan 21, 2026
This commit fixes the ROS2/colcon build integration by properly configuring
CMake exports for the ament ecosystem.

Changes:
- Add ament detection when not building as LF subproject
- Move install(EXPORT) from lib/CMakeLists.txt to root CMakeLists.txt
  (required for ament compatibility - must be alongside ament_package())
- Add NAMESPACE to export for proper imported target creation
- Add ament_export_targets(), ament_export_include_directories(), and
  ament_export_libraries() for full ament integration

The key insight is that ament's build system requires the export installation
and ament_package() to be in the same CMakeLists.txt file. Using namespaced
targets (reactor-cpp::reactor-cpp) ensures proper linkage through CMake's
imported target mechanism.

Related: lf-lang/lingua-franca#2580

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit fixes two issues:

1. ROS2 C++ Build Integration

   The ROS2 C++ test was failing because downstream packages could not find
   reactor-cpp via find_package(). The root cause was improper CMake export
   configuration for the ament/colcon ecosystem.

   Changes to reactor-cpp (submodule update):
   - Add ament detection when not building as LF subproject
   - Move install(EXPORT) to root CMakeLists.txt (required for ament - must
     be alongside ament_package())
   - Add NAMESPACE to export for proper imported target creation
   - Add ament_export_targets(), ament_export_include_directories(), and
     ament_export_libraries()

   Changes to CppRos2PackageGenerator.kt:
   - Add HINTS to find_package(reactor-cpp) to locate it in colcon workspace
   - Use namespaced target reactor-cpp::reactor-cpp for linking

2. C++ Coverage Collection (pre-existing bug fix)

   The coverage extraction step was looking for the wrong directory path:
   - Was: test/Cpp/src-gen/reactor-cpp-default/*
   - Now: test/Cpp/src-gen/reactor-cpp/*

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@lsk567 lsk567 force-pushed the fix-ros2-cpp-build branch from f70e3c9 to 5238187 Compare January 21, 2026 17:25
@lsk567 lsk567 requested a review from edwardalee January 21, 2026 17:26

@edwardalee edwardalee left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, this was a complicated one. Thanks for fixing!

@lsk567 lsk567 added this pull request to the merge queue Jan 21, 2026
Merged via the queue into master with commit 39b1221 Jan 21, 2026
26 checks passed
@lsk567 lsk567 deleted the fix-ros2-cpp-build branch January 21, 2026 20:03
lsk567 added a commit that referenced this pull request Jan 22, 2026
This reverts commit 39b1221, reversing
changes made to a8eb573.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 23, 2026
@edwardalee edwardalee added exclude Exclude from change log and removed bugfix labels Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

exclude Exclude from change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants