Summary
While reviewing ROS 2 package dependency metadata, I noticed that moveit_visual_tools appears to use geometric_shapes directly, but geometric_shapes does not seem to be declared in moveit_visual_tools/package.xml.
This may make the package rely on moveit_core or moveit_ros_planning to provide geometric_shapes transitively.
Evidence
Direct usage
moveit_visual_tools directly includes geometric_shapes in C++ source or public headers:
src/moveit_visual_tools.cpp:67: #include <geometric_shapes/solid_primitive_dims.h>
src/moveit_visual_tools.cpp:68: #include <geometric_shapes/shape_operations.h>
The package uses dependency-owned types or APIs from those headers:
src/moveit_visual_tools.cpp:861: shapes::Shape* mesh = shapes::createMeshFromResource(mesh_path); // make sure its prepended by file://
src/moveit_visual_tools.cpp:862: shapes::ShapeMsg shape_msg; // this is a boost::variant type from shape_messages.h
src/moveit_visual_tools.cpp:863: if (!mesh || !shapes::constructMsgFromShape(mesh, shape_msg))
Current package.xml
I could not find a direct declaration of geometric_shapes in moveit_visual_tools/package.xml.
The package currently declares the following intermediate dependency/dependencies, which make geometric_shapes reachable transitively:
<depend>moveit_core</depend>
<depend>moveit_ros_planning</depend>
Observed during build/test
Build/test file-access tracing observed some events associated with geometric_shapes during build. Representative accessed paths include:
geometric_shapes/share/ament_index/resource_index/package_run_dependencies/geometric_shapes
geometric_shapes/share/geometric_shapes/cmake/geometric_shapesConfig-version.cmake
geometric_shapes/share/geometric_shapes/cmake/geometric_shapesConfig.cmake
geometric_shapes/share/geometric_shapes/cmake/ament_cmake_export_targets-extras.cmake
Transitive path
moveit_visual_tools -> moveit_core -> geometric_shapes
moveit_visual_tools -> moveit_ros_planning -> moveit_core -> geometric_shapes
moveit_visual_tools -> moveit_ros_planning -> moveit_ros_occupancy_map_monitor -> geometric_shapes
moveit_visual_tools -> moveit_ros_planning -> moveit_ros_occupancy_map_monitor -> moveit_core -> geometric_shapes
Suggested fix
If this direct usage is intentional, would it make sense to add:
<depend>geometric_shapes</depend>
to moveit_visual_tools/package.xml? The corresponding find_package(geometric_shapes REQUIRED) and CMake target dependency should also be added for the target(s) that compile these files, where required by the package's CMake structure.
Notes
This issue does not claim that the package currently fails to build. The concern is that the package directly uses geometric_shapes while relying on a transitive dependency path to make it available. The observation is based on package metadata, concrete source-level use, recursive dependency closure analysis, and build/test file-access tracing.
Could you please confirm whether this dependency is intentionally left implicit through the transitive dependency path shown above, or whether adding an explicit dependency would be appropriate?
I would be happy to open a small PR adding the dependency if that matches the intended package metadata.
Summary
While reviewing ROS 2 package dependency metadata, I noticed that
moveit_visual_toolsappears to usegeometric_shapesdirectly, butgeometric_shapesdoes not seem to be declared inmoveit_visual_tools/package.xml.This may make the package rely on
moveit_coreormoveit_ros_planningto providegeometric_shapestransitively.Evidence
Direct usage
moveit_visual_toolsdirectly includesgeometric_shapesin C++ source or public headers:src/moveit_visual_tools.cpp:67:#include <geometric_shapes/solid_primitive_dims.h>src/moveit_visual_tools.cpp:68:#include <geometric_shapes/shape_operations.h>The package uses dependency-owned types or APIs from those headers:
src/moveit_visual_tools.cpp:861:shapes::Shape* mesh = shapes::createMeshFromResource(mesh_path); // make sure its prepended by file://src/moveit_visual_tools.cpp:862:shapes::ShapeMsg shape_msg; // this is a boost::variant type from shape_messages.hsrc/moveit_visual_tools.cpp:863:if (!mesh || !shapes::constructMsgFromShape(mesh, shape_msg))Current package.xml
I could not find a direct declaration of
geometric_shapesinmoveit_visual_tools/package.xml.The package currently declares the following intermediate dependency/dependencies, which make
geometric_shapesreachable transitively:Observed during build/test
Build/test file-access tracing observed some events associated with
geometric_shapesduringbuild. Representative accessed paths include:geometric_shapes/share/ament_index/resource_index/package_run_dependencies/geometric_shapesgeometric_shapes/share/geometric_shapes/cmake/geometric_shapesConfig-version.cmakegeometric_shapes/share/geometric_shapes/cmake/geometric_shapesConfig.cmakegeometric_shapes/share/geometric_shapes/cmake/ament_cmake_export_targets-extras.cmakeTransitive path
Suggested fix
If this direct usage is intentional, would it make sense to add:
to
moveit_visual_tools/package.xml? The correspondingfind_package(geometric_shapes REQUIRED)and CMake target dependency should also be added for the target(s) that compile these files, where required by the package's CMake structure.Notes
This issue does not claim that the package currently fails to build. The concern is that the package directly uses
geometric_shapeswhile relying on a transitive dependency path to make it available. The observation is based on package metadata, concrete source-level use, recursive dependency closure analysis, and build/test file-access tracing.Could you please confirm whether this dependency is intentionally left implicit through the transitive dependency path shown above, or whether adding an explicit dependency would be appropriate?
I would be happy to open a small PR adding the dependency if that matches the intended package metadata.