Skip to content

Add distro compatibility policy - #3751

Draft
nbbrooks wants to merge 4 commits into
moveit:mainfrom
nbbrooks:nbbrooks/distro-compat-policy
Draft

Add distro compatibility policy#3751
nbbrooks wants to merge 4 commits into
moveit:mainfrom
nbbrooks:nbbrooks/distro-compat-policy

Conversation

@nbbrooks

Copy link
Copy Markdown
Collaborator

Description

Document the in-source patterns and process moveit2 uses to keep main building on every active ROS 2 distro, plus the lifecycle for adding and removing distros from the CI matrix. Three topics:

  1. Supporting multiple distros on main — five sub-patterns covering C++ source divergence (RCLCPP_VERSION_GTE), package.xml conditional deps, launch-loaded file overrides, CMake feature detection, and MIGRATION.md updates.
  2. Handling dependencies without a bloom package — CI source-build via per-distro .repos files (preferred), with a CMake-side optional feature-flag fallback.
  3. CI matrix policy — add / graduate / drop lifecycle.

Each pattern carries a cleanup rule keyed on a distro EOL trigger, so the conditional code paths don't accumulate indefinitely.

Why now

We're accumulating multi-distro divergences in main without a written convention. Recent precedents that motivated this:

  • #3567 uses the RCLCPP_VERSION_GTE pattern for C++ source divergence.
  • moveit/moveit_resources#211 introduces format-3 condition= deps and a launch-time YAML override helper.

Capturing these as a single documented policy lets future divergences land consistently and gives reviewers a checklist.

Checklist

  • Code is auto-formatted (documentation-only)
  • Tutorials/documentation — not applicable; this is a contributor doc
  • MIGRATION.md — not applicable; this is the policy that requires future MIGRATION.md entries
  • Tests — not applicable
  • GUI screenshots — not applicable

Document the in-source patterns and process moveit2 uses to keep main
building on every active ROS 2 distro, plus the lifecycle for adding
and removing distros from the CI matrix. Three topics:

1. Supporting multiple distros on main — five sub-patterns covering C++
   source divergence (RCLCPP_VERSION_GTE), package.xml conditional deps,
   launch-loaded file overrides, CMake feature detection, and MIGRATION
   .md updates.
2. Handling dependencies without a bloom package — CI source-build via
   per-distro .repos files (preferred), with a CMake-side optional
   feature-flag fallback.
3. CI matrix policy — add / graduate / drop lifecycle.

Pulls together patterns already established in
  - moveit2#3567 (C++ source via RCLCPP_VERSION_GTE)
  - moveit2#3727 (Boost::system removal under Boost 1.89+)
  - moveit2#3743 (rolling-resolute experimental CI job)
  - moveit_resources#211 (package.xml condition= attribute)

so future divergences are handled consistently rather than ad hoc.

Add a pointer from CONTRIBUTING.md.

@rhaschke rhaschke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explicitly writing that down. I only have a few minor comments:

  • Place the file in doc/


## Topic 2: Handling dependencies without a bloom package

When a moveit2 dependency is **released** for a distro (entry exists in `rolling/distribution.yaml`) but **not yet bloomed** to the apt binary repo, CI fails at `rosdep install`. Two complementary approaches handle this. **Prefer §2.1.**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a package exists in rosdistro, it was bloomed. However, it might exist in ros-testing only, because it requires a sync. I suggest dropping the mention of bloom (here and below).

Suggested change
When a moveit2 dependency is **released** for a distro (entry exists in `rolling/distribution.yaml`) but **not yet bloomed** to the apt binary repo, CI fails at `rosdep install`. Two complementary approaches handle this. **Prefer §2.1.**
When a moveit2 dependency is not yet released for a distro, CI will fail at `rosdep install`. Two complementary approaches handle this. **Prefer §2.1.**

Comment on lines +214 to +217
if(osqp_FOUND)
add_subdirectory(online_signal_smoothing)
target_compile_definitions(my_target PRIVATE MOVEIT_HAVE_OSQP)
endif()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add an else branch to inform users about the omitted feature (as required below).

Suggested change
if(osqp_FOUND)
add_subdirectory(online_signal_smoothing)
target_compile_definitions(my_target PRIVATE MOVEIT_HAVE_OSQP)
endif()
if(osqp_FOUND)
add_subdirectory(online_signal_smoothing)
target_compile_definitions(my_target PRIVATE MOVEIT_HAVE_OSQP)
else()
message(STATUS "Dropping online_signal_smoothing as osqp wasn't found")
endif()

…sections

Document the unified per-distro branch CI policy established by:
- Topic 3 §"Per-distro branches": which workflows live on per-distro
  branches (ci/docker/docker_lint/format only) vs main-only
  (prerelease/sonar/stale/tutorial_docker).
- Topic 3 §"Forking a new per-distro branch": the exact bootstrap
  changes when a new ROS distro lands. Captures the lessons from the
  kilted-branch retrofit (moveit#3769) where ci.yaml/docker.yaml/tutorial
  workflows were left targeting rolling/main after the fork from main.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46.30%. Comparing base (77c5894) to head (8d3ff57).
⚠️ Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3751      +/-   ##
==========================================
+ Coverage   46.23%   46.30%   +0.08%     
==========================================
  Files         726      726              
  Lines       59501    59508       +7     
  Branches     7624     7623       -1     
==========================================
+ Hits        27505    27550      +45     
+ Misses      31829    31791      -38     
  Partials      167      167              

☔ View full report in Codecov by Harness.
📢 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 and others added 2 commits July 9, 2026 14:59
…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#3703moveit#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#3703moveit#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>
…istros

Adds humble, jazzy, kilted, lyrical columns to the rclcpp /
ament_index_cpp version table. Sourced from `apt-cache policy`
on ubuntu:{jammy,noble,resolute} against packages.ros.org.

Also flags the Rolling-on-Noble ↔ Lyrical inversion: Rolling's rclcpp
was at 30.1.4 when the frozen `rolling-ci` snapshot was taken, but
Lyrical branched later at 32.0.1. A guard threshold set with only
"current Rolling" in mind can catch or miss Lyrical unintentionally
depending on the value.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
nbbrooks added a commit to nbbrooks/moveit2 that referenced this pull request Aug 2, 2026
…Plugin

OSQP shipped v1.0 with a redesigned C API. Rolling is expected to migrate
to the moveit-org fork of osqp_vendor (which vendors OSQP v1.0.0 under
cmake_minimum_required(3.16), naturally CMake-4-clean on Resolute), while
Humble/Jazzy/Kilted stay on tier4/osqp_vendor's apt-shipped 0.2.0 (v0.6.x).
Guard the two API paths so a single main branch supports both — matches
the distro-compat-guard policy in moveit#3751.

Version detection: CMake looks at the include directory the osqp::osqp
target resolved to and checks whether it contains v1's osqp_api_types.h.
That definition (MOVEIT_OSQP_V1) is passed PUBLIC into the compile so
consumers of moveit_acceleration_filter (notably the test binary) pick
the same branch as the .so — the guard controls a private member type
of AccelerationLimitedPlugin so a v1/v0.6 mismatch across TU boundaries
would be UB. Notes:

- osqp_VERSION cannot drive this — OSQP's shipped osqp-config-version.cmake
  sets PACKAGE_VERSION "0.0.0" (packaging bug, both major streams).
- Neither v0.6 nor v1 exposes a numeric preprocessor version macro
  (v0.6's OSQP_VERSION is a string literal; v1's numeric macros are in
  a non-installed private/version.h).
- Probing INTERFACE_INCLUDE_DIRECTORIES ties the compile-time branch
  selection to the same install CMake found, so a build environment with
  BOTH v0.6 (apt) and v1.0 (source-overlay) picks self-consistently.
- The .hpp keeps a header-probe fallback so LSP/clang-tidy standalone
  (which don't run through our CMakeLists) still get a plausible value.

API-shape guards inside acceleration_filter.{hpp,cpp}:
- OSQPWorkspace* ↔ OSQPSolver* (member type on AccelerationLimitedPlugin)
- v0.6 c_int + csc typedefs are aliased to v1's OSQPInt + OSQPCscMatrix
  at namespace scope so most of the file reads in v1 naming
- OSQPData aggregate populated in the OSQPDataWrapper constructor on
  v0.6 only; v1 doesn't have it
- updateA calls osqp_update_A (v0.6) vs osqp_update_data_mat (v1)
- updateData calls osqp_update_bounds (v0.6) vs osqp_update_data_vec (v1)
- osqp_setup takes (&workspace, &data, &settings) on v0.6 vs
  (&solver, &P, q, &A, l, u, m, n, &settings) on v1
- OSQPSettings.warm_start on v0.6 vs warm_starting on v1

Field names on csc/OSQPCscMatrix (n, m, i, p, x, nzmax, nz) are identical
so CSCWrapper stays as one implementation. osqp_solve and solution access
(->solution->x[0]) are also identical between versions.

Also introduces moveit2_rolling.repos pointing osqp_vendor at
moveit/osqp_vendor's OSQP v1.0.0 merge commit so rolling-ci actually
compiles the v1 code path (without this the v1 branch would only see
first compile on the Resolute buildfarm). Coordinates with moveit#3760 which
introduces the same file for a different reason — whichever lands
second reconciles.

Verified end-to-end in a fresh ubuntu:resolute + ROS Rolling container:
- With moveit/osqp_vendor@main (OSQP v1.0.0): moveit_core builds clean,
  4/4 AccelerationFilterTest cases pass
- With tier4/osqp_vendor@0.2.0 (OSQP v0.6.2): moveit_core builds clean,
  4/4 AccelerationFilterTest cases pass

Refs moveit#3786, moveit#3760.
nbbrooks added a commit that referenced this pull request Aug 2, 2026
…Plugin (#3806)

OSQP shipped v1.0 with a redesigned C API. Rolling is expected to migrate
to the moveit-org fork of osqp_vendor (which vendors OSQP v1.0.0 under
cmake_minimum_required(3.16), naturally CMake-4-clean on Resolute), while
Humble/Jazzy/Kilted stay on tier4/osqp_vendor's apt-shipped 0.2.0 (v0.6.x).
Guard the two API paths so a single main branch supports both — matches
the distro-compat-guard policy in #3751.

Version detection: CMake looks at the include directory the osqp::osqp
target resolved to and checks whether it contains v1's osqp_api_types.h.
That definition (MOVEIT_OSQP_V1) is passed PUBLIC into the compile so
consumers of moveit_acceleration_filter (notably the test binary) pick
the same branch as the .so — the guard controls a private member type
of AccelerationLimitedPlugin so a v1/v0.6 mismatch across TU boundaries
would be UB. Notes:

- osqp_VERSION cannot drive this — OSQP's shipped osqp-config-version.cmake
  sets PACKAGE_VERSION "0.0.0" (packaging bug, both major streams).
- Neither v0.6 nor v1 exposes a numeric preprocessor version macro
  (v0.6's OSQP_VERSION is a string literal; v1's numeric macros are in
  a non-installed private/version.h).
- Probing INTERFACE_INCLUDE_DIRECTORIES ties the compile-time branch
  selection to the same install CMake found, so a build environment with
  BOTH v0.6 (apt) and v1.0 (source-overlay) picks self-consistently.
- The .hpp keeps a header-probe fallback so LSP/clang-tidy standalone
  (which don't run through our CMakeLists) still get a plausible value.

API-shape guards inside acceleration_filter.{hpp,cpp}:
- OSQPWorkspace* ↔ OSQPSolver* (member type on AccelerationLimitedPlugin)
- v0.6 c_int + csc typedefs are aliased to v1's OSQPInt + OSQPCscMatrix
  at namespace scope so most of the file reads in v1 naming
- OSQPData aggregate populated in the OSQPDataWrapper constructor on
  v0.6 only; v1 doesn't have it
- updateA calls osqp_update_A (v0.6) vs osqp_update_data_mat (v1)
- updateData calls osqp_update_bounds (v0.6) vs osqp_update_data_vec (v1)
- osqp_setup takes (&workspace, &data, &settings) on v0.6 vs
  (&solver, &P, q, &A, l, u, m, n, &settings) on v1
- OSQPSettings.warm_start on v0.6 vs warm_starting on v1

Field names on csc/OSQPCscMatrix (n, m, i, p, x, nzmax, nz) are identical
so CSCWrapper stays as one implementation. osqp_solve and solution access
(->solution->x[0]) are also identical between versions.

Also introduces moveit2_rolling.repos pointing osqp_vendor at
moveit/osqp_vendor's OSQP v1.0.0 merge commit so rolling-ci actually
compiles the v1 code path (without this the v1 branch would only see
first compile on the Resolute buildfarm). Coordinates with #3760 which
introduces the same file for a different reason — whichever lands
second reconciles.

Verified end-to-end in a fresh ubuntu:resolute + ROS Rolling container:
- With moveit/osqp_vendor@main (OSQP v1.0.0): moveit_core builds clean,
  4/4 AccelerationFilterTest cases pass
- With tier4/osqp_vendor@0.2.0 (OSQP v0.6.2): moveit_core builds clean,
  4/4 AccelerationFilterTest cases pass

Refs #3786, #3760.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants