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>
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