Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ABI/API Compliance Checker in github workflow #2555

Open
fujitatomoya opened this issue Jun 8, 2024 · 2 comments
Open

ABI/API Compliance Checker in github workflow #2555

fujitatomoya opened this issue Jun 8, 2024 · 2 comments
Labels

Comments

@fujitatomoya
Copy link
Collaborator

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

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

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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants