Update deprecated usage of get_package_share without std::filesystem::path by stephanie-eng · Pull Request #3703 · moveit/moveit2

stephanie-eng · 2026-03-13T19:37:50Z

Description

CI appears to be borked. Maybe this will help.

Checklist

Required by CI: Code is auto formatted using clang-format
Extend the tutorials / documentation reference
Document API changes relevant to the user in the MIGRATION.md notes
Create tests, which fail without this PR reference
Include a screenshot if changing a GUI
While waiting for someone to review your request, please help review another open pull request to support the maintainers

codecov · 2026-03-13T19:58:06Z

Codecov Report

❌ Patch coverage is 52.94118% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.28%. Comparing base (f1135a4) to head (da8f17d).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
moveit_core/utils/src/robot_model_test_utils.cpp	50.00%	6 Missing ⚠️
moveit_ros/planning/rdf_loader/src/rdf_loader.cpp	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            main    #3703       +/-   ##
==========================================
+ Coverage   0.00%   46.28%   +46.28%     
==========================================
  Files        539      726      +187     
  Lines      47592    59495    +11903     
  Branches    6085     7623     +1538     
==========================================
+ Hits           0    27533    +27533     
+ Misses     47592    31794    -15798     
- Partials       0      168      +168

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

nbbrooks · 2026-03-15T15:38:59Z

~~This is to address ament/ament_index#112~~

No, this is to address ament/ament_index#104 in Rolling distro - ament/ament_index#112 is not released to debians yet.

nbbrooks · 2026-03-15T17:30:47Z

@stephanie-eng could you turn on "Allow edits from maintainers" so I can push some changes? For now I duplicated this PR in #3705

stephanie-eng · 2026-03-15T18:40:13Z

Should already be on @nbbrooks

This reverts commit f0b9544. Qt6 migration will be handled in a separate branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

nbbrooks

nbbrooks · 2026-03-23T13:11:25Z

Looks like I missed a QT migration I'll address in a followup - in previous PRs I recall the deprecation check failing on get_package_share deprecations but I do not see this here so LGTM.

  /home/runner/work/moveit2/moveit2/.work/target_ws/src/moveit2/moveit_setup_assistant/moveit_setup_srdf_plugins/src/collision_linear_model.cpp: In function ‘bool moveit_setup::srdf_setup::compareVariants(const QVariant&, const QVariant&)’:
  /home/runner/work/moveit2/moveit2/.work/target_ws/src/moveit2/moveit_setup_assistant/moveit_setup_srdf_plugins/src/collision_linear_model.cpp:279:42: error: ‘Type’ is deprecated: Use QMetaType::Type instead. [-Werror=deprecated-declarations]
    279 |   if (left.userType() == QVariant::Type::Int)
        |                                          ^~~
  In file included from /usr/include/x86_64-linux-gnu/qt6/QtCore/qabstractitemmodel.h:11,
                   from /usr/include/x86_64-linux-gnu/qt6/QtCore/qabstractproxymodel.h:7,
                   from /usr/include/x86_64-linux-gnu/qt6/QtCore/QAbstractProxyModel:1,
                   from /home/runner/work/moveit2/moveit2/.work/target_ws/src/moveit2/moveit_setup_assistant/moveit_setup_srdf_plugins/include/moveit_setup_srdf_plugins/collision_linear_model.hpp:39,
                   from /home/runner/work/moveit2/moveit2/.work/target_ws/src/moveit2/moveit_setup_assistant/moveit_setup_srdf_plugins/src/collision_linear_model.cpp:37:
  /usr/include/x86_64-linux-gnu/qt6/QtCore/qvariant.h:61:70: note: declared here
     61 |     enum QT_DEPRECATED_VERSION_X_6_0("Use QMetaType::Type instead.") Type
        |                                                                      ^~~~
  At global scope:
  cc1plus: note: unrecognized command-line option ‘-Wno-unknown-warning-option’ may have been intended to silence earlier diagnostics
  cc1plus: all warnings being treated as errors
  gmake[2]: *** [CMakeFiles/moveit_setup_srdf_plugins.dir/build.make:90: CMakeFiles/moveit_setup_srdf_plugins.dir/src/collision_linear_model.cpp.o] Error 1
  gmake[1]: *** [CMakeFiles/Makefile2:166: CMakeFiles/moveit_setup_srdf_plugins.dir/all] Error 2
  gmake: *** [Makefile:146: all] Error 2
  ---
  Failed   <<< moveit_setup_srdf_plugins [41.2s, exited with code 2]

…lling The RCLCPP_VERSION_GTE(30, 0, 0) branches added in moveit#3703 for these three files reference a two-argument overload of `ament_index_cpp::get_package_share_directory(package_name, out_path)` that has never existed. Rolling's ament_index_cpp API is `get_package_share_path(package_name)` returning `std::filesystem::path` directly (ament/ament_index#112), and the corresponding header is `get_package_share_path.hpp` — the old `get_package_share_directory.hpp` has been removed. Switch: - Rolling header: `#include <ament_index_cpp/get_package_share_path.hpp>` - Rolling calls: `get_package_share_path(pkg)` returning path directly Pre-Rolling (`#else` branch) keeps the current `get_package_share_directory(pkg)` returning `std::string`. Verified with a full source-build of moveit2 + downstream against `ubuntu:resolute` + ROS Rolling (`ros2-testing` apt). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…lling The RCLCPP_VERSION_GTE(30, 0, 0) branches added in moveit#3703 for these three files reference a two-argument overload of `ament_index_cpp::get_package_share_directory(package_name, out_path)` that has never existed. Rolling's ament_index_cpp API is `get_package_share_path(package_name)` returning `std::filesystem::path` directly (ament/ament_index#112), and the corresponding header is `get_package_share_path.hpp` — the old `get_package_share_directory.hpp` has been removed on the Ubuntu Resolute-based images. Switch: - Feature-test via `__has_include(<ament_index_cpp/get_package_share_path.hpp>)` rather than RCLCPP_VERSION_GTE — the API rename is in ament_index_cpp, not rclcpp, and the two versions can drift (rolling-ci's frozen Noble container has RCLCPP >= 30 but no new header yet). - Rolling: `get_package_share_path(pkg)` returning path directly. - Pre-Rolling: keep `get_package_share_directory(pkg)` returning std::string. Verified with a full source-build of moveit2 + downstream against `ubuntu:resolute` + ROS Rolling (`ros2-testing` apt). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…lling The RCLCPP_VERSION_GTE(30, 0, 0) branches added in moveit#3703 for these three files reference a two-argument overload of `ament_index_cpp::get_package_share_directory(package_name, out_path)` that has never existed. Rolling's ament_index_cpp API is `get_package_share_path(package_name)` returning `std::filesystem::path` directly (ament/ament_index#112), and the corresponding header is `get_package_share_path.hpp` — the old `get_package_share_directory.hpp` has been removed on the Ubuntu Resolute-based images. Guard: RCLCPP_VERSION_GTE(33, 0, 0). The ament_index_cpp rename shipped with rclcpp 33 (Rolling on Ubuntu Resolute). Rolling on Noble was still at rclcpp 30.x with the old header. So the moveit#3703 threshold of 30 was set too early — it was true on both Noble-era rolling (old API) and Resolute-era rolling (new API), which is why the code compiled on Noble-based rolling-ci but chose the wrong branch. Verified with a full source-build of moveit2 + downstream against `ubuntu:resolute` + ROS Rolling (`ros2-testing` apt). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…lling The RCLCPP_VERSION_GTE(30, 0, 0) branches added in moveit#3703 for these three files reference a two-argument overload of `ament_index_cpp::get_package_share_directory(package_name, out_path)` that has never existed. Rolling's ament_index_cpp API is `get_package_share_path(package_name)` returning `std::filesystem::path` directly, and the corresponding header is `get_package_share_path.hpp` — the old `get_package_share_directory.hpp` has been removed on the Ubuntu Resolute-based images. Guard: AMENT_INDEX_CPP_VERSION_GTE(1, 14, 0). Guard by ament_index_cpp's own version, not rclcpp, because: - The API change lives in ament_index_cpp, and its release cadence is independent of rclcpp's. Using rclcpp version as a proxy for another package's feature is fragile. - Concretely: Rolling on Noble shipped ament_index_cpp 1.13.1 (old header) alongside rclcpp 30.1.4. Rolling on Resolute shipped ament_index_cpp 1.14.1 (new header) alongside rclcpp 33.0.x. moveit#3703's RCLCPP_VERSION_GTE(30, 0, 0) threshold triggered on Noble-based Rolling despite the new header not being present there. Drops the now-unused `#include <rclcpp/version.h>` in each file (added by moveit#3703 for the guard we replaced). Verified with a full source-build of moveit2 + downstream against `ubuntu:resolute` + ROS Rolling (`ros2-testing` apt). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…macro The §1.1 draft advised guarding C++ divergence on RCLCPP_VERSION_GTE as a general distro proxy. That's fragile: every ROS 2 package has its own version macro and its own release cadence, so RCLCPP_VERSION only proxies for another package's feature when their versions happen to move in lockstep. Concrete failure moveit#3703 → moveit#3705: `ament_index_cpp` renamed `get_package_share_directory.hpp` → `get_package_share_path.hpp` in 1.14. Rolling's `rclcpp` had already been at ≥ 30 for months before that, so a `RCLCPP_VERSION_GTE(30, 0, 0)` guard triggered on Rolling-on-Noble containers that still shipped ament_index_cpp 1.13.1 — referencing a header that didn't exist yet. The guard was doing nothing useful in the Noble era, then quietly discriminating wrong across the Noble→Resolute transition. Changes: - §1.1 rewritten to advise guarding on the actual feature-carrier package's version macro (`AMENT_INDEX_CPP_VERSION_GTE`, `TF2_VERSION_GTE`, `RCLCPP_VERSION_GTE` for genuine rclcpp changes). Adds a "picking the threshold" checklist, a version snapshot for the frozen `rolling-ci` container vs. current Rolling, and moveit#3703→moveit#3705 as the canonical example. - §1.5 adds a reviewer checklist item: verify the threshold matches the upstream release, and verify the guard uses the correct package's macro. - New Topic 3 section "Auditing existing version guards when Rolling changes base OS" — a concrete grep-and-audit procedure for the CI-matrix PR that adopts each new Rolling base. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…lling ament_index_cpp evolved its API in two steps, and the RCLCPP_VERSION_GTE(30, 0, 0) guards added in moveit#3703 hit the wrong branch on Rolling-on-Noble (where the header still exists but the 1-arg form is now deprecated) and outright fail to compile on Rolling-on-Resolute (where the header no longer exists at all). Guard on ament_index_cpp's own version macro — three tiers: 1.14+ (Rolling on Resolute) <ament_index_cpp/get_package_share_path.hpp> replaced the old header, exposing get_package_share_path(pkg) which returns std::filesystem::path directly. 1.5+ (Jazzy 1.8.4, Kilted 1.11.4, Lyrical 1.13.3, Rolling-on-Noble 1.13.1) Old header still present; use the 2-arg out-param overload get_package_share_directory(pkg, path) which is non-deprecated (matters for the -Werror=deprecated-declarations job on Rolling-on-Noble where the 1-arg form is deprecated). older (Humble 1.4.1) Only the 1-arg get_package_share_directory(pkg) returning std::string is available; wrap in std::filesystem::path(). Guarding on ament_index_cpp's own version (rather than rclcpp's) is important because the two packages release independently — the API change lives in ament_index_cpp, and using RCLCPP_VERSION as a proxy misfires when Rolling's rclcpp advances without a corresponding ament_index_cpp bump (as happened in moveit#3703 → this fix). Drops the now-unused `#include <rclcpp/version.h>` in each file (added by moveit#3703 for the guard we replaced). Verified with a full source-build of moveit2 + downstream against `ubuntu:resolute` + ROS Rolling (`ros2-testing` apt). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…lling (#3705) ament_index_cpp evolved its API in two steps, and the RCLCPP_VERSION_GTE(30, 0, 0) guards added in #3703 hit the wrong branch on Rolling-on-Noble (where the header still exists but the 1-arg form is now deprecated) and outright fail to compile on Rolling-on-Resolute (where the header no longer exists at all). Guard on ament_index_cpp's own version macro — three tiers: 1.14+ (Rolling on Resolute) <ament_index_cpp/get_package_share_path.hpp> replaced the old header, exposing get_package_share_path(pkg) which returns std::filesystem::path directly. 1.5+ (Jazzy 1.8.4, Kilted 1.11.4, Lyrical 1.13.3, Rolling-on-Noble 1.13.1) Old header still present; use the 2-arg out-param overload get_package_share_directory(pkg, path) which is non-deprecated (matters for the -Werror=deprecated-declarations job on Rolling-on-Noble where the 1-arg form is deprecated). older (Humble 1.4.1) Only the 1-arg get_package_share_directory(pkg) returning std::string is available; wrap in std::filesystem::path(). Guarding on ament_index_cpp's own version (rather than rclcpp's) is important because the two packages release independently — the API change lives in ament_index_cpp, and using RCLCPP_VERSION as a proxy misfires when Rolling's rclcpp advances without a corresponding ament_index_cpp bump (as happened in #3703 → this fix). Drops the now-unused `#include <rclcpp/version.h>` in each file (added by #3703 for the guard we replaced). Verified with a full source-build of moveit2 + downstream against `ubuntu:resolute` + ROS Rolling (`ros2-testing` apt). Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

github-project-automation Bot added this to MoveIt Mar 13, 2026

stephanie-eng requested a review from nbbrooks March 13, 2026 20:13

stephanie-eng force-pushed the seng/update-get-package-share branch 3 times, most recently from f9eb433 to f0b9544 Compare March 13, 2026 21:34

Timple reviewed Mar 15, 2026

View reviewed changes

Comment thread moveit_setup_assistant/moveit_setup_framework/include/moveit_setup_framework/utilities.hpp

Timple reviewed Mar 15, 2026

View reviewed changes

Comment thread moveit_ros/planning/rdf_loader/src/rdf_loader.cpp

nbbrooks mentioned this pull request Mar 15, 2026

resolute: complete ament_index_cpp::get_package_share_path migration #3705

Merged

4 tasks

stephanie-eng and others added 5 commits March 22, 2026 11:11

Update deprecated usage of get_package_share

7236a3f

Fix more get_package_shares

3419f48

Update to Qt6 and hope

2697b70

Revert "Update to Qt6 and hope"

c68e134

This reverts commit f0b9544. Qt6 migration will be handled in a separate branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add include guards for ament/ament_index#104

8bef942

nbbrooks force-pushed the seng/update-get-package-share branch from a3d95b1 to 8bef942 Compare March 22, 2026 17:16

Merge branch 'main' into seng/update-get-package-share

da8f17d

nbbrooks approved these changes Mar 23, 2026

View reviewed changes

nbbrooks enabled auto-merge (squash) March 23, 2026 13:09

nbbrooks disabled auto-merge March 23, 2026 13:09

nbbrooks changed the title ~~Update deprecated usage of get_package_share~~ Update deprecated usage of get_package_share without std::filesystem::path Mar 23, 2026

nbbrooks merged commit 17b0dcf into moveit:main Mar 23, 2026
9 of 11 checks passed

github-project-automation Bot moved this to ✅ Done in MoveIt Mar 23, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update deprecated usage of get_package_share without std::filesystem::path#3703

Update deprecated usage of get_package_share without std::filesystem::path#3703
nbbrooks merged 6 commits into
moveit:mainfrom
stephanie-eng:seng/update-get-package-share

stephanie-eng commented Mar 13, 2026

Uh oh!

codecov Bot commented Mar 13, 2026 •

edited

Loading

Uh oh!

nbbrooks commented Mar 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

nbbrooks commented Mar 15, 2026

Uh oh!

stephanie-eng commented Mar 15, 2026

Uh oh!

nbbrooks left a comment

Uh oh!

nbbrooks commented Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

stephanie-eng commented Mar 13, 2026

Description

Checklist

Uh oh!

codecov Bot commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

nbbrooks commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nbbrooks commented Mar 15, 2026

Uh oh!

stephanie-eng commented Mar 15, 2026

Uh oh!

nbbrooks left a comment

Choose a reason for hiding this comment

Uh oh!

nbbrooks commented Mar 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Mar 13, 2026 •

edited

Loading

nbbrooks commented Mar 15, 2026 •

edited

Loading