Remove use of ament_target_dependencies: Take 2#3726
Open
nbbrooks wants to merge 1 commit into
Open
Conversation
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
fe0fc9d to
ec8da76
Compare
6 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3726 +/- ##
==========================================
+ Coverage 46.24% 46.25% +0.02%
==========================================
Files 726 726
Lines 59501 59502 +1
Branches 7625 7625
==========================================
+ Hits 27508 27519 +11
+ Misses 31827 31816 -11
- Partials 166 167 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
|
Thanks for starting the work. I just ticketed #3741 |
ament_target_dependencies was deprecated and removed upstream (ament/ament_cmake#614). Replace it with explicit target_link_libraries(... NAMESPACE::TARGET) calls across the codebase, matching the ROS 2 Kilted-and-newer guidance. Two subtleties needed extra care: - ament_target_dependencies(... Boost ...) implicitly linked every Boost component found by ConfigExtras.cmake (chrono, date_time, filesystem, iostreams, program_options, regex, serialization, system, thread). A bare target_link_libraries(... Boost::headers) drops those compiled components and breaks any translation unit that uses non-header-only Boost. Targets that need them now name their components explicitly: - moveit_test_utils: + Boost::regex (robot_model_test_utils.cpp uses boost::split_regex / boost::regex) - moveit_ompl_interface: + Boost::serialization (detail/constraints_library.hpp uses boost::serialization::map) - moveit_ros_benchmarks: + Boost::filesystem + Boost::regex (BenchmarkExecutor.cpp uses both) - The random_numbers::random_numbers imported target was only added upstream in moveit/random_numbers#47 (merged 2025-05-27, "[kilted]"), so it doesn't exist on jazzy or humble and the CMake configure step fails there. moveit_kdl_kinematics_plugin uses the variable form ${random_numbers_LIBRARIES} with a matching target_include_directories instead, so the build works across all supported distros. Also add find_package(rviz2 REQUIRED) to moveit_ros_visualization so that rviz2_VERSION is available for the Qt5/Qt6 selection introduced in moveit#3707, and resolve merge conflicts with that PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
8ca048f to
3b486ce
Compare
Contributor
Author
|
@rhaschke if you have any time to give this a high level look it would be much appreciated! |
There was a problem hiding this comment.
Pull request overview
This PR updates MoveIt 2’s CMake usage to avoid ament_target_dependencies() in preparation for its removal/behavior change in ament_cmake (ament/ament_cmake#614), replacing it with explicit target_link_libraries() calls and adding several “umbrella” INTERFACE targets to simplify downstream linking.
Changes:
- Replace
ament_target_dependencies()with explicittarget_link_libraries()across many packages (favoring imported targets likerclcpp::rclcpp,pluginlib::pluginlib,${*_TARGETS}, etc.). - Add umbrella
INTERFACEtargets (e.g.,moveit_core,moveit_ros_planning,moveit_ros_visualization, etc.) to provide single-target linkage for downstream consumers. - Adjust and expand
find_package()coverage in some top-level CMakeLists to ensure required imported targets/variables exist.
Reviewed changes
Copilot reviewed 99 out of 99 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| moveit_setup_assistant/moveit_setup_srdf_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_setup_assistant/moveit_setup_simulation/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_setup_assistant/moveit_setup_framework/CMakeLists.txt | Explicit link targets; Qt handling adjustments |
| moveit_setup_assistant/moveit_setup_core_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_setup_assistant/moveit_setup_controllers/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_setup_assistant/moveit_setup_assistant/CMakeLists.txt | Explicit link targets; Qt linkage refactor |
| moveit_setup_assistant/moveit_setup_app_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/warehouse/CMakeLists.txt | Explicit link targets; add umbrella INTERFACE target |
| moveit_ros/visualization/trajectory_rviz_plugin/CMakeLists.txt | Explicit link targets for rviz plugin |
| moveit_ros/visualization/rviz_plugin_render_tools/CMakeLists.txt | Explicit link targets for render tools lib |
| moveit_ros/visualization/robot_state_rviz_plugin/CMakeLists.txt | Explicit link targets for rviz plugin |
| moveit_ros/visualization/planning_scene_rviz_plugin/CMakeLists.txt | Explicit link targets for rviz plugin |
| moveit_ros/visualization/motion_planning_rviz_plugin/CMakeLists.txt | Explicit link targets for rviz plugin |
| moveit_ros/visualization/CMakeLists.txt | Add umbrella INTERFACE target for visualization |
| moveit_ros/trajectory_cache/test/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/trajectory_cache/CMakeLists.txt | Explicit link targets; add umbrella INTERFACE target |
| moveit_ros/tests/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/robot_interaction/CMakeLists.txt | Explicit link targets; add umbrella INTERFACE target |
| moveit_ros/planning/trajectory_execution_manager/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/srdf_publisher_node/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/robot_model_loader/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/rdf_loader/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/planning_scene_monitor/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/planning_response_adapter_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/planning_request_adapter_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/planning_pipeline/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/planning_pipeline_interfaces/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/planning_components_tools/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/plan_execution/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/moveit_cpp/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/kinematics_plugin_loader/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/constraint_sampler_manager_loader/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/collision_plugin_loader/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning/CMakeLists.txt | Add umbrella INTERFACE target for planning |
| moveit_ros/planning_interface/planning_scene_interface/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning_interface/move_group_interface/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning_interface/common_planning_interface_objects/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/planning_interface/CMakeLists.txt | Add umbrella INTERFACE target for planning_interface |
| moveit_ros/perception/semantic_world/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/perception/pointcloud_octomap_updater/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/perception/point_containment_filter/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/perception/mesh_filter/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/perception/lazy_free_space_updater/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/perception/depth_image_octomap_updater/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/perception/CMakeLists.txt | Add umbrella INTERFACE target for perception |
| moveit_ros/occupancy_map_monitor/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/moveit_servo/CMakeLists.txt | Replace ament_target_dependencies; add umbrella INTERFACE target |
| moveit_ros/move_group/CMakeLists.txt | Replace ament_target_dependencies; add umbrella INTERFACE target |
| moveit_ros/hybrid_planning/test/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/local_planner/trajectory_operator_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/local_planner/local_planner_component/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/local_planner/local_constraint_solver_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/hybrid_planning_manager/planner_logic_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/hybrid_planning_manager/hybrid_planning_manager_component/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/global_planner/global_planner_plugins/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/global_planner/global_planner_component/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_ros/hybrid_planning/CMakeLists.txt | Add umbrella INTERFACE target for hybrid planning |
| moveit_ros/benchmarks/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_py/src/moveit/moveit_py_utils/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_plugins/moveit_simple_controller_manager/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_plugins/moveit_ros_control_interface/CMakeLists.txt | Replace ament_target_dependencies; add umbrella INTERFACE target |
| moveit_planners/test_configs/prbt_ikfast_manipulator_plugin/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/stomp/test/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/stomp/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/pilz_industrial_motion_planner/test/unit_tests/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/pilz_industrial_motion_planner/test/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/pilz_industrial_motion_planner/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/pilz_industrial_motion_planner_testutils/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/ompl/ompl_interface/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/ompl/CMakeLists.txt | Add umbrella INTERFACE target for OMPL planners |
| moveit_planners/chomp/chomp_motion_planner/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_planners/chomp/chomp_interface/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_kinematics/test/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_kinematics/srv_kinematics_plugin/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_kinematics/kdl_kinematics_plugin/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_kinematics/ikfast_kinematics_plugin/templates/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_kinematics/CMakeLists.txt | Add umbrella INTERFACE target for kinematics |
| moveit_kinematics/cached_ik_kinematics_plugin/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/utils/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/transforms/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/trajectory_processing/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/robot_trajectory/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/robot_state/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/robot_model/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/planning_scene/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/planning_interface/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/online_signal_smoothing/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/kinematics_metrics/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/kinematics_base/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/kinematic_constraints/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/exceptions/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/dynamics_solver/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/distance_field/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/constraint_samplers/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/collision_distance_field/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/collision_detection/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/collision_detection_fcl/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/collision_detection_bullet/CMakeLists.txt | Replace ament_target_dependencies with target_link_libraries |
| moveit_core/CMakeLists.txt | Add umbrella INTERFACE target for moveit_core + dependency exports |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+55
| moveit_setup_srdf_plugins::moveit_setup_srdf_plugins | ||
| pluginlib::pluginlib | ||
| ${QT_CORE} | ||
| ${QT_WIDGETS} | ||
| rclcpp::rclcpp) |
Comment on lines
+68
to
+72
| moveit_setup_srdf_plugins::moveit_setup_srdf_plugins | ||
| pluginlib::pluginlib | ||
| ${QT_CORE} | ||
| ${QT_WIDGETS} | ||
| rclcpp::rclcpp) |
Comment on lines
+69
to
+73
| moveit_ros_visualization::moveit_ros_visualization | ||
| pluginlib::pluginlib | ||
| ${QT_CORE} | ||
| ${QT_WIDGETS} | ||
| rclcpp::rclcpp |
Comment on lines
+20
to
+21
| rviz_ogre_vendor::OgreMain | ||
| rviz_ogre_vendor::OgreOverlay) |
rhaschke
reviewed
Jun 8, 2026
rhaschke
left a comment
rhaschke
left a comment
Contributor
There was a problem hiding this comment.
At first sight, there are many more redundant dependency declarations - additionally to the one found by copilot.
In general, it seems to work, though.
| eigen_stl_containers::eigen_stl_containers | ||
| pluginlib::pluginlib | ||
| rclcpp::rclcpp | ||
| rmw_implementation::rmw_implementation |
Contributor
There was a problem hiding this comment.
This should be part of rclcpp.
Suggested change
| rmw_implementation::rmw_implementation |
| rmw_implementation::rmw_implementation | ||
| urdf::urdf | ||
| urdfdom::urdf_parser | ||
| urdfdom_headers::urdfdom_headers |
Contributor
There was a problem hiding this comment.
This should be part of urdfdom::urdf_parser
Suggested change
| urdfdom_headers::urdfdom_headers |
| ament_target_dependencies(moveit_collision_detection_bullet SYSTEM BULLET) | ||
| ament_target_dependencies( | ||
| target_include_directories(moveit_collision_detection_bullet SYSTEM | ||
| PUBLIC ${BULLET_INCLUDE_DIRS}) |
Contributor
There was a problem hiding this comment.
Doesn't bullet provide a target as well?
| moveit_collision_detection moveit_utils) | ||
| moveit_collision_detection | ||
| moveit_utils | ||
| ${BULLET_LIBRARIES} |
Contributor
There was a problem hiding this comment.
Doesn't bullet provide a target as well?
Suggested change
| ${BULLET_LIBRARIES} | |
| Bullet::Bullet |
|
I tried giving this a go, but currently I am stuck with osqp in order to build MoveIt on Rolling+Resolute. I've started migrating MoveIt to osqp 1.0, but there's one structure left to migrate which didn't end up in the library's public API. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Address removal of ament_target_dependencies in ament/ament_cmake#614
Continuation of #3708
Checklist