Lines-changed-only seems not to work for clang-tidy
What events trigger your workflow?
on:
pull_request:
branches: [master]
paths: ['**.c', '**.h', '**.am']
push:
paths: ['**.c', '**.h', '**.am']
What OS does your workflow use?
runs-on: ubuntu-latest
container: ubuntu:22.04
How is cpp-linter-action configured?
- uses: cpp-linter/cpp-linter-action@v2
id: linter
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
with:
style: '' # Empty for no format check
tidy-checks: '' # Use .clang-tidy config file.
version: ${{ env.LLVM_VERSION }}
# only 'update' a single comment in a pull request's thread.
thread-comments: ${{ github.event_name == 'pull_request' && 'update' }}
lines-changed-only: true
tidy-review: true
passive-reviews: true
What was the unexpected behavior?
I'm testing the action and it seems to detect correctly the line changed in the file I did update to trigger some error.
The job is running the command:
/usr/bin/clang-tidy-14 --line-filter=[{"name": "src/source-af-packet.c", "lines": [[1, 2], [932, 935], [940, 943]]}] --extra-arg= --fix-errors src/source-af-packet.c
which returns no warning despite the introduction of invalid code in these places.
When I run a derivated same command on my system:
clang-tidy --line-filter='[{"name": "src/source-af-packet.c", "lines": [[1, 2], [932, 935], [940, 943]]}]' --extra-arg= src/source-af-packet.c
I got no error too but if I do
clang-tidy --line-filter='[{"name": "source-af-packet.c", "lines": [[1, 2], [932, 935], [940, 943]]}]' --extra-arg= src/source-af-packet.c
then I got the correct behavior.
Is there an option to skip the source directory in the action ?
Failing job url: https://github.com/regit/suricata/actions/runs/14709331910/job/41277429460
[!tip]
- clang-tidy always works better when you give it a compile_commands.json (AKA compilation database).
- cpp-linter works better with source paths and clang-tidy when the compilation database's path is explicitly defined via our
databaseoption.
Peculiar observations
It is odd that the line-filter uses a path different from the path to the analyzed source (clang-tidy --line-filter='[{"name": "source-af-packet.c", ... src/source-af-packet.c). If you're implicitly using a compilation data base (letting clang-tidy auto-detect one), then the src/ part of the path might be getting excluded there which would explain why your customized command works.
It is also weird that --extra-arg is passed to clang-tidy without a value. I think this is a bug somehow.
Tip
- clang-tidy always works better when you give it a compile_commands.json (AKA compilation database).
- cpp-linter works better with source paths and clang-tidy when the compilation database's path is explicitly defined via our
databaseoption.Peculiar observations
It is odd that the
line-filteruses a path different from the path to the analyzed source (clang-tidy --line-filter='[{"name": "source-af-packet.c", ... src/source-af-packet.c). If you're implicitly using a compilation data base (letting clang-tidy auto-detect one), then thesrc/part of the path might be getting excluded there which would explain why your customized command works.It is also weird that
--extra-argis passed to clang-tidy without a value. I think this is a bug somehow.
Yes, I'm using a compilation database and the src directory is indeed not present in the file definition
{ "directory": "/home/eric/git/suricata/src", "file": "source-af-packet.c", "output": "source-af-packet.o", "arguments": "lot of things"}
I'm responsible of the generation of the compile_commands.json (via a Makefile target). I can see if I can tune that and get it work both in editor and in the clang-tidy command.
Is there an option to skip the source directory in the action ?
If everything needed (including compilation database) is located in src/ then you could tell cpp-linter to cd src with
- uses: cpp-linter/cpp-linter-action@v2
with:
repo-root: src
or using github actions syntax
- uses: cpp-linter/cpp-linter-action@v2
working-directory: src
Oh. Looks like repo-root does not behave well with lines-changed-only. I see the new run has warnings when looking for the changed files returned from GH REST API.
WARNING:CPP Linter:Could not find src/app-layer-ftp.c! Did you checkout the repo?
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Oh. Looks like repo-root does not behave well with lines-changed-only. I see the new run has warnings when looking for the changed files returned from GH REST API.
Began tracking this upstream in cpp-linter/cpp-linter#146
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.