ABI/API Compliance Checker in github workflow

fujitatomoya commented 3 weeks ago

Feature request

Feature description

Integrate ABI/API compliance checker in github workflow to check ABI compatibility for every PR to see if the fix can be backported automatically with label.

Background

Currently, for every PR, we (developers and maintainers) check the ABI compatibility if the PR is backportable to already available distribution. Sometimes it could be hard to tell with looking at the source code for sure, this also depends on the developers's skill and experience. (I personally run ABI compliance checker to make sure for this.)

Basically all bug fixes should be backported to the downstream distribution, but sometimes we miss and do that need-to-do or requested basis. (we can find comments requesting or asking for the backport to released distribution.)

To avoid this burden for developers and maintainers and keep the high quality for ROS distribution, it would be nice to have ABI compliance checker automatically run against the PR, so that we will be able to know the fix is ABI compatible with the tag or label provided by the workflow.

Note

This ABI Compatible or Backport-able tag is an additional information for maintainers, says this does not mean we have to backport the fix to already released distros. Although ABI is compatible, different behavior or significant different user-experience should be avoided breaking the user space.

Implementation considerations

we can integrate https://github.com/lvc/abi-compliance-checker to github workflow. (or any other tools or recommendation?)
build target branch and PR branch or repo to check the ABI compatibility for every commits.
new label ABI compatible needs to be introduced and tagged by workflow.

I am creating this issue on rclcpp (because that is where i can find this situation most.) but the following core repositories can have the same ABI checker CI.

flowchart TD
    A[PR / Every Commit] -->|workflow| B[ABI Compliance Check]
    B --> C{ABI Compatible?}
    C -->|Yes| D[Tag / Label]
    D --> E[Code Review]
    C -->|No / Not Required| E[Code Review / Approval]
    C -->|ABI Compatible Required?| A[PR / Every Commit]
    E -->|Mergifyio| F[Backport Process]

after all, i think this will do some good for ROS users, developers and maintainers to accelerate the development process and ROS quality.

Tomoya,

ros-discourse commented 3 weeks ago

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/abi-api-compliance-checker-in-github-workflow/38141/1

clalancette commented 2 weeks ago

I think that this is a good idea, though we'll have to be careful with it. In particular, in rolling we allow ABI breaks at any time, while the released distributions don't allow ABI breaks. We'll have to have configuration for that.

In addition, we have some ABI testing already available on the ROS buildfarm, though we haven't tested or enabled it since Galactic. In particular, on your rosdistro repositories, you can add test_abi: true, which will do...something (see https://github.com/ros/rosdistro/blob/73d2581901d80429f1c9e371d2bf5de500d1d7dd/foxy/distribution.yaml#L4628). So I think we should start by seeing what happens if we enable that (also, maybe @j-rivero , @nuclearsandwich , or @cottsay can shed some light on what that does, if anything).

ros2 / rclcpp