Fix Rolling build: replace ament_target_dependencies + guard the ament_index_cpp header rename - #156
Merged
Conversation
nbbrooks
added a commit
that referenced
this pull request
Aug 3, 2026
This repo ships a single `ros2` branch to five distros but only tested two,
and neither of those gives a Resolute signal:
humble 4.1.2-1 tested (humble-source image)
jazzy 4.1.2-1 NOT tested
kilted 4.1.2-2 NOT tested
lyrical 4.1.2-3 NOT tested
rolling 4.1.2-2 tested (rolling-source image)
The rolling job pins moveit/moveit2:rolling-source. That image is currently
stale -- moveit2's docker.yaml has failed on every run since ~2026-07-13
(its Dockerfile hardcodes clang-format-14, which Ubuntu Resolute does not
package), so the `source` job is skipped and the published tag predates the
Rolling base-OS move to Resolute. The last CI run on this branch was
2026-06-23, also before the transition. So the green badge on `ros2` says
nothing about Resolute.
MoveIt is released on jazzy, kilted and lyrical, so those can be covered
today with plain binary debs -- no source build and no dependency on the
moveit2 image pipeline. lyrical is Ubuntu Resolute, which makes it the first
job in this repo that exercises Resolute at all.
Two changes are needed to let source-image and bare-OS jobs share a matrix:
- DOCKER_IMAGE moves out of the static job env into a conditional step, since
jobs building on a bare OS image must leave it unset. Same pattern as
moveit2's own ci.yaml.
- AFTER_SETUP_UPSTREAM_WORKSPACE_EMBED is replaced by industrial_ci's
UNDERLAY, conditioned on the image being a -source image. The old snippet
sourced ~/ws_moveit/install/setup.bash unconditionally, a path that only
exists inside the moveit2 source images, so it failed immediately on any
bare-OS job. This adopts the form moveit_task_constructor already uses:
UNDERLAY: ${{ endsWith(matrix.env.IMAGE, '-source') && '/root/ws_moveit/install' || '' }}
which also drops a latent typo -- the old line ended `set - u` (with a
space), which sets a positional parameter rather than re-enabling `-u`.
CACHE_PREFIX and the job name fall back to <distro>-<repo> for entries with
no IMAGE.
Expect lyrical to fail initially: CMakeLists.txt still calls
ament_target_dependencies, which was removed in ament_cmake 2.7.3+. That is
the breakage reported in PickNikRobotics/rviz_visual_tools#294 and fixed by
#156. This change makes it visible rather than silently untested.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nbbrooks
force-pushed
the
nbbrooks/rolling-compat
branch
from
August 3, 2026 02:39
8fea0b6 to
d7ae8ce
Compare
…d interactive_markers Rolling removed ament_target_dependencies. Migrate to target_link_libraries with imported targets across the library and demo call sites. Add explicit find_package(interactive_markers) + <depend> + link — no longer transitive through rviz_visual_tools after rviz_visual_tools#277.
…N_GTE ament_index_cpp 1.14+ removed <get_package_share_directory.hpp>. Use the new get_package_share_path (returns std::filesystem::path) when available, fall back to the old API otherwise. Same 3-tier guard pattern moveit2#3705 adopted.
nbbrooks
force-pushed
the
nbbrooks/rolling-compat
branch
from
August 3, 2026 03:16
d7ae8ce to
9aa4f9c
Compare
moveit_visual_tools #includes these packages and uses their API directly,
but declares none of them in package.xml or CMakeLists.txt. All three
resolve today only because moveit_core / moveit_ros_planning happen to
pull them in transitively.
geometric_shapes src/moveit_visual_tools.cpp:67-68 includes
solid_primitive_dims.h + shape_operations.h and calls
shapes::createMeshFromResource /
shapes::constructMsgFromShape at L861-863.
Reported as moveit#155.
moveit_msgs include/moveit_visual_tools/moveit_visual_tools.h:48-52
includes five moveit_msgs headers, and the types appear
in public method signatures (publishGrasps,
publishTrajectoryPath, processCollisionObjectMsg, ...).
ament_index_cpp src/moveit_visual_tools_demo.cpp:36-41.
Each is scoped to what it is actually used for rather than added uniformly:
- moveit_msgs is PUBLIC and exported via THIS_PACKAGE_INCLUDE_DEPENDS: it
appears in installed public headers, so downstream consumers of
find_package(moveit_visual_tools) need it resolved.
- geometric_shapes is PRIVATE and deliberately NOT exported. It appears in
no public header -- include/ contains zero references to it or to
shapes::. This is a SHARED library, so a PRIVATE link is not recorded in
the exported target's interface at all (no $<LINK_ONLY:> entry, which
would appear for a STATIC library), and downstream consumers correctly
do not need geometric_shapes. Linking the library target therefore uses
the keyword signature; the demo target keeps the plain form, which is
legal because the plain/keyword rule is per-target.
- ament_index_cpp is linked to the demo executable only and kept out of
THIS_PACKAGE_INCLUDE_DEPENDS; it is not part of the library's interface.
All three still get <depend> in package.xml. A find_package(... REQUIRED)
without a matching <depend> is a build-time dependency rosdep will never
install, so the two halves are not substitutes for each other.
This matters more on this branch than on ros2. The preceding commit replaces
ament_target_dependencies with target_link_libraries, and
ament_target_dependencies was flattening the recursive dependency tree onto
every target -- which is precisely what was papering over these three gaps.
Declaring them is a prerequisite for that migration, not a metadata cleanup.
moveit_ros_occupancy_map_monitor is deliberately not among the three. moveit#153
added find_package(moveit_ros_occupancy_map_monitor REQUIRED) on the base
branch, but nothing here includes an occupancy_map_monitor header directly;
it is reachable only via
moveit/planning_scene_monitor/planning_scene_monitor.hpp:53. Verified against
the built moveit_ros_planning export set: moveit_planning_scene_monitor
carries moveit_ros_occupancy_map_monitor::moveit_ros_occupancy_map_monitor in
its INTERFACE_LINK_LIBRARIES and moveit_ros_planning::moveit_ros_planning
re-exports it, so planning_scene_monitor.hpp compiles under
target_link_libraries with no explicit find_package. That line is left as-is
here; note it has no matching <depend> in package.xml, which is the
find_package/<depend> mismatch this commit avoids for the other three.
moveit_ros_planning::moveit_ros_planning is an INTERFACE target added by
moveit2#3726 (merged 2026-06-16). It exists only on moveit2 main -- every
released MoveIt predates it, so linking it fails at configure time on all
three distros this package ships to:
CMake Error: Target "moveit_visual_tools" links to
moveit_ros_planning::moveit_ros_planning
but the target was not found.
Verified against the released debs; the specific target is present
everywhere, the umbrella nowhere:
distro moveit_ros_planning ::moveit_planning_scene_monitor ::moveit_ros_planning
humble 2.5.9 present absent
jazzy 2.12.4 present absent
kilted 2.14.3 present absent
moveit2 main - present present
It is also the accurate dependency rather than merely the portable one:
planning_scene_monitor.hpp is the only moveit_ros_planning header this
package includes, so the umbrella would over-link all 14 of that package's
libraries.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Closed
Contributor
Author
|
Rolling resolute and Lyrical failures are expected due to missing MoveIt APT releases. |
nbbrooks
added a commit
that referenced
this pull request
Aug 3, 2026
Consolidate the catkin_generate_changelog output: one entry per change, with sub-bullets for the pieces of #156. The generated version inlined every commit body (155 lines) and rewrote cross-repo references such as moveit2#3726 and rviz_visual_tools#277 into links against this repository's issue tracker. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Addresses
1.
ament_target_dependencieswas removedFrom (ament/ament_cmake#614, merged 2026-01-13). Migrated the library and demo targets to
target_link_librarieswith imported targets.2.
ament_index_cpp1.14 removed<get_package_share_directory.hpp>moveit_visual_tools_demo.cppnow selectsget_package_share_path(returnsstd::filesystem::path) when available and falls back otherwise, keyed onAMENT_INDEX_CPP_VERSION_GTE— the carrier package's own version macro, notRCLCPP_VERSION_GTE.Inspired by