Remove use of ament_target_dependencies: Take 2#3726
Merged
nbbrooks merged 8 commits intoJun 16, 2026
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.01%
==========================================
Files 726 726
Lines 59513 59512 -1
Branches 7623 7623
==========================================
+ Hits 27516 27519 +3
+ Misses 31831 31826 -5
- 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 |
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
8ca048f to
3b486ce
Compare
Collaborator
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.
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.
|
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. |
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
becac50 to
13d4006
Compare
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
31d55c1 to
2177c4a
Compare
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
937de58 to
d4a3bf5
Compare
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>
Link the imported Qt targets (Qt${QT_VERSION_MAJOR}::Core/::Widgets)
instead of the undefined ${QT_CORE}/${QT_WIDGETS} variables, remove a
duplicate rviz_ogre_vendor::OgreMain, and drop redundant transitive
dependencies (rmw_implementation, urdfdom_headers) already provided by
rclcpp and urdfdom::urdf_parser.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The rviz2-version-based Qt selection already finds Qt before any rviz or moveit dependency, so the second find_package(Qt6) block was redundant and could pull in Qt6 even on the Qt5 path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add Qt to moveit_setup_assistant's exported dependencies so downstream consumers of its targets resolve the Qt imported targets. Drop the unused Boost::regex/Boost::filesystem links (and their includes) from benchmarks, which uses std::regex/std::filesystem. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…loader Link Boost::date_time in moveit_ros_benchmarks and moveit_ompl_interface, which use boost::posix_time but only linked Boost::headers after the ament_target_dependencies(... Boost) removal (the old call pulled in all Boost components). The Boost::date_time imported target is already found via ConfigExtras.cmake / the top-level find_package. Drop the redundant class_loader::class_loader link from moveit_plan_execution and moveit_kinematics_plugin_loader; pluginlib::pluginlib already re-exports class_loader, and these targets only use the pluginlib wrapper (<pluginlib/class_loader.hpp>), not class_loader directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ament_export_dependencies(Qt${QT_VERSION_MAJOR}) emits find_dependency(Qt6)
in the package config, but find_package(Qt6) without COMPONENTS does not
define the Qt6::Core / Qt6::Widgets imported targets that the exported
targets link against. Downstream find_package(moveit_setup_framework) /
find_package(moveit_setup_assistant) would then fail to resolve those
targets.
Export the standalone Qt${QT_VERSION_MAJOR}Core / Qt${QT_VERSION_MAJOR}Widgets
packages instead, whose configs define Qt6::Core / Qt6::Widgets. The
component find_package already sets Qt6Core_FOUND / Qt6Widgets_FOUND, so
ament_export_dependencies accepts them.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ders moveit_robot_interaction and moveit_ros_occupancy_map_monitor use tf2_ros (tf2_ros::Buffer) in their public headers but only received it transitively. Link tf2_ros::tf2_ros (plus tf2/tf2_eigen, already used in robot_interaction's sources) explicitly and export it via THIS_PACKAGE_INCLUDE_DEPENDS so downstream consumers of those headers resolve the dependency. Also link tf2_ros into the occupancy_map_server executable, which uses tf2_ros::TransformListener/Buffer directly. Clarify the moveit_ros_move_group umbrella INTERFACE target comment: it exposes the capabilities API (moveit_move_group_capabilities_base); moveit_move_group_default_capabilities is a runtime pluginlib plugin and is intentionally excluded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…gs_TARGETS}
tf2_geometry_msgs is a plain C++ library package (not a rosidl interface
package), exporting a single imported target tf2_geometry_msgs::tf2_geometry_msgs
via ament_export_targets(). Linking that namespaced target is the idiomatic,
documented form for a library, and it fails loudly if the target is ever
missing -- whereas an undefined ${..._TARGETS} variable expands to empty and
silently drops the dependency. The _TARGETS variable form is reserved for
rosidl interface (msg/srv/action) packages, which generate multiple
typesupport targets with no single aggregate target on all distros.
No behavioral change: both forms resolve to the same target today.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
nbbrooks
force-pushed
the
pr-remove-ament-target-dependencies
branch
from
755be16 to
65d0256
Compare
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