feat(build-and-test-differential): modify clang-tidy to run only on specified packages

[yhisaki]

Description

Currently, clang-tidy checks are being executed but are not required and are being ignored. However, clang-tidy outputs important warnings that should not be disregarded.

This PR aims to gradually make clang-tidy checks required on a package-by-package basis. With this PR, the following process related to clang-tidy will be executed on GitHub Actions:

1. Get modified packages: Retrieve the packages that have been modified in the PR.
2. Get target packages: Obtain the intersection of packages listed in clang-tidy-required-package.yaml and those retrieved in step 1.
3. Run autoware-clang-tidy on the packages obtained in step 2.

By progressively adding clang-tidy-compliant packages to clang-tidy-required-package.yaml , we can achieve gradual improvement in code quality.

This PR is related to this PR.

Related links

• https://github.com/orgs/autowarefoundation/discussions/4876

How was this PR tested?

Tested on CI.

• succeeded CI
• failed CI

Notes for reviewers

None.

Interface changes

None.

Effects on system behavior

None.

[github-actions]

Thank you for contributing to the Autoware project!

🚧 If your pull request is in progress, switch it to draft mode.

Please ensure:

• You've checked our contribution guidelines.
• Your PR follows our pull request guidelines.
• All required CI checks pass before marking the PR ready for review.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.68%. Comparing base (d4d40cd) to head (276c91a).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #9016      +/-   ##
==========================================
- Coverage   26.68%   26.68%   -0.01%     
==========================================
  Files        1296     1300       +4     
  Lines       95679    95715      +36     
  Branches    39078    39080       +2     
==========================================
+ Hits        25530    25538       +8     
- Misses      67494    67522      +28     
  Partials     2655     2655              

Flag	Coverage Δ		*Carryforward flag
differential	17.86% <ø> (?)
total	26.68% <ø> (ø)		Carriedforward from 92dd416

*This pull request uses carry forward flags. Click here to find out more.

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

[YamatoAndo]

@yhisaki hi, I can't review this PR, so please ask someone else to review it.

[xmfcx]

About this file as a whole, what about removing the commented out packages?

This way the file will be cleaner. Right now it is hard to see the "not commented out" packages.

We can just keep adding new packages as they come through.

[xmfcx]

This way, the autoware_ prefixing job won't affect it that much.

[veqcc]

Agreed 👍

[xmfcx]

Ok thanks, I'll approve once the commented out packages are removed from this file 👍

[veqcc]

@yhisaki
Could you update clang-tidy-required-packages.yaml file?

[veqcc]

@yhisaki
I have create a new, minimum .clang-tidy-ci configuration, so please use it and list up the packages which pass the minimum check.
autowarefoundation/autoware#5465

[veqcc]

@xmfcx
I have one question on clang-tidy CI: Is there any other repository which uses autowarefoundation/autoware-github-actions/clang-tidy?
If not, what about changing autowarefoundation/autoware-github-actions/clang-tidy as follows:

• delete target-files interface and use only target-packages
• use run-clang-tidy instead of clang-tidy, which is more suitable for a big project

[xmfcx]

You can find all the references here:

• https://github.com/search?q=org%3Aautowarefoundation%20autowarefoundation%2Fautoware-github-actions%2Fclang-tidy&type=code

I'm not sure about usages outside our organization.

You are referring to https://github.com/lmapii/run-clang-tidy right?

Could you create a PR to https://github.com/autowarefoundation/autoware-github-actions create a new version for the clang tidy for this.

And it would be a breaking change, we will need to update the major version for the entire actions in the repository since they are not versioned individually.

Once the major version is incremented, any other further references to any of the actions from the repository will need to be incremented if we make a change to them. This is because we refer to them with @v1 everywhere. Example:

autoware.universe/.github/actions/build-and-test-differential/action.yaml

Line 43 in d8087f4

uses: autowarefoundation/autoware-github-actions/get-modified-packages@v1

[xmfcx]

It might be a good idea to divide https://github.com/autowarefoundation/autoware-github-actions into multiple repositories.

At least, whenever a major change is required for an action, we can put that particular action into its own repository and version it independently.

What do you think @mitsudome-r @HansRobo ?

I think the costs for the new repo are:

• repository settings
  • general settings
  • rulesets
    • status checks will refer to actions below
  • team management
• actions
  • release action?
  • precommit
  • test-composite-actions
  • new pr action
  • these can be automated with a sync action

[veqcc]

You can find all the references here:
https://github.com/search?q=org%3Aautowarefoundation%20autowarefoundation%2Fautoware-github-actions%2Fclang-tidy&type=code

Thank you for your infomation, and I understand it is a breaking change and needs versioning.
Our policy on making clang-tidy a required CI is a mimimum and rapid start, so if it will take some time, we will use the v1 clang-tidy workflow 👍

You are referring to https://github.com/lmapii/run-clang-tidy right?
Could you create a PR to https://github.com/autowarefoundation/autoware-github-actions create a new version for the clang tidy for this.

No, perhaps. run-clang-tidy is a part of llvm-project:
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py
I will make a PR later!

[HansRobo]

It might be a good idea to divide autowarefoundation/autoware-github-actions into multiple repositories.

At least, whenever a major change is required for an action, we can put that particular action into its own repository and version > it independently.

I agree.
It's something we'll need to do eventually as we grow, so if this is the opportunity, we should go ahead with it.

There will be maintenance costs, but I can help with some of that, and most of it can be automated.
However, a lot of the start-up work will have to be done by the AWF maintainers.

By dividing the repository, the simplicity of permission management may be lost, but this can be managed by creating a team of GitHub Actions maintainers on AWF GitHub organization and linking the divided repositories to that team.

(In line with this, awf/autoware-spell-check-dict can also be linked to this team for management.)

[mitsudome-r]

It might be a good idea to divide https://github.com/autowarefoundation/autoware-github-actions into multiple repositories.

At least, whenever a major change is required for an action, we can put that particular action into its own repository and version it independently.

That sounds like a good idea to me.

[yhisaki]

While this PR specifies which ROS2 packages to tidy, I want to significantly rewrite it to specify which files should NOT be tidied.

In my plan, I would create a file like .clang-tidy-ignore and specify files to ignore in the following format:

path/to/ignore1/*
path/to/ignore2/ignored.cpp

The reasons for this change are:

1. This approach is common and used in various projects (e.g. apache/arrow, zeek/spicy, andreasfertig/cppinsights)
2. This is a more versatile approach.

Let me know if you have any thoughts or opinions.

[xmfcx]

@yhisaki it makes sense, let's do it as you've suggested 👍