Skip to content

Conversation

@bmchalenv
Copy link
Contributor

@bmchalenv bmchalenv commented Jan 28, 2026

Build farm is erroring because of ament_target_dependencies not being recognized. My current assumption is ament_cmake_auto is not properly calling find_package(ament_cmake) and its not loading that macro. This works locally but has seen errors in the Build Farm.

Most other major OSS ROS packages (nav2, ros2-control, etc) avoid the use of ament_cmake_auto so moving away it from it may make this project more stable for releases.

Signed-off-by: Blake McHale <bmchale@nvidia.com>
@greptile-apps
Copy link

greptile-apps bot commented Jan 28, 2026

Greptile Overview

Greptile Summary

Added explicit find_package(ament_cmake REQUIRED) before the existing ament_cmake_auto declaration in CMakeLists.txt.

  • Aligns CMakeLists.txt with package.xml which declares both ament_cmake and ament_cmake_auto as buildtool dependencies
  • Follows ROS 2 best practices by explicitly declaring the base ament_cmake package that provides macros used throughout the file (ament_target_dependencies, ament_package, ament_add_pytest_test, ament_add_gtest, ament_python_install_package)
  • Single line addition with no functional changes to the build process

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • Single line addition that explicitly declares a dependency already implicitly available, matches package.xml declarations, follows ROS 2 best practices, and has no functional impact on the build
  • No files require special attention

Important Files Changed

Filename Overview
greenwave_monitor/CMakeLists.txt Added explicit ament_cmake dependency before ament_cmake_auto to match package.xml declarations and follow ROS 2 best practices

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CMake as CMake Build System
    participant AC as ament_cmake
    participant ACA as ament_cmake_auto
    participant PkgXML as package.xml

    Dev->>CMake: Run colcon build
    CMake->>CMake: Read CMakeLists.txt
    CMake->>PkgXML: Check buildtool_depend declarations
    PkgXML-->>CMake: ament_cmake + ament_cmake_auto declared
    CMake->>AC: find_package(ament_cmake REQUIRED)
    AC-->>CMake: Load ament_cmake macros
    Note over AC: ament_target_dependencies()<br/>ament_package()<br/>ament_add_pytest_test()<br/>ament_add_gtest()
    CMake->>ACA: find_package(ament_cmake_auto REQUIRED)
    ACA-->>CMake: Load ament_cmake_auto macros
    Note over ACA: ament_auto_find_build_dependencies()
    CMake->>ACA: ament_auto_find_build_dependencies()
    ACA->>PkgXML: Read <depend> tags
    PkgXML-->>ACA: rclcpp, std_msgs, diagnostic_msgs, etc.
    ACA->>CMake: Auto-find all dependencies
    CMake->>Dev: Build succeeds with all dependencies resolved
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, no comments

Edit Code Review Agent Settings | Greptile

@bmchalenv bmchalenv marked this pull request as draft January 28, 2026 23:40
@bmchalenv bmchalenv changed the title Add ament_cmake as package in CMakelist Use ament_cmake instead of ament_cmake_auto to improve stability Jan 28, 2026
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

Successfully merging this pull request may close these issues.

1 participant