-
Couldn't load subscription status.
- Fork 24
[feat] Add ability to specify the compilation database #42
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
Conversation
closes #41
I've been testing this. It works locally, but I get an error on the test repo using a generated database from CMake on the GitHub runner.
INFO:CPP Linter:Running "clang-tidy-10 -checks=boost-*,bugprone-*,performance-*,readability-*,portability-*,modernize-*,clang-analyzer-*,cppcoreguidelines-* --export-fixes=clang_tidy_output.yml -p build src/demo.cpp"
DEBUG:CPP Linter:Output from clang-tidy:
WARNING:CPP Linter:clang-tidy-10 raised the following error(s):
LLVM ERROR: Cannot chdir into "/home/runner/work/test-cpp-linter-action/test-cpp-linter-action/build"!
I'm still investigating if its just a workflow problem. The path seems correct though (duplicated folder names are from git checkout).
It would seem different versions of clang-tidy output something different:
WARNING:CPP Linter:clang-tidy-12 raised the following error(s):
LLVM ERROR: Cannot chdir into "/home/runner/work/test-cpp-linter-action/test-cpp-linter-action/build"!
PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace.
Stack dump:
0. Program arguments: clang-tidy-12 -checks=boost-*,bugprone-*,performance-*,readability-*,portability-*,modernize-*,clang-analyzer-*,cppcoreguidelines-* --export-fixes=clang_tidy_output.yml -p=build src/demo.hpp
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
/lib/x86_64-linux-gnu/libLLVM-12.so.1(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamEi+0x23)[0x7f54b9db8ef3]
/lib/x86_64-linux-gnu/libLLVM-12.so.1(_ZN4llvm3sys17RunSignalHandlersEv+0x50)[0x7f54b9db7210]
/lib/x86_64-linux-gnu/libLLVM-12.so.1(+0xbd955f)[0x7f54b9db955f]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x153c0)[0x7f54c15ef3c0]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcb)[0x7f54b8ce618b]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x12b)[0x7f54b8cc5859]
/lib/x86_64-linux-gnu/libLLVM-12.so.1(+0xb34209)[0x7f54b9d14209]
/lib/x86_64-linux-gnu/libclang-cpp.so.12(_ZN5clang7tooling9ClangTool3runEPNS0_10ToolActionE+0x10cb)[0x7f54c09d704b]
clang-tidy-12[0x87c377]
clang-tidy-12[0x45833d]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0x7f54b8cc70b3]
clang-tidy-12[0x45612e]
I also can't seem to get clang-tidy-13 to work, but again, that's not a problem with these changes.
I have a feeling it may have something to do with the docker image's access to runner's File System.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its ok to merge this, but I'd really like to get the database option to work when specified. Its causing clang-tidy to crash which we have to ignore because of multiple files. To be clear, it does work as a python package executable:
steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v3 - name: Install clang-tools run : sudo apt-get update && sudo apt-get install clang-tidy-${{ matrix.clang-version }} clang-format-${{ matrix.clang-version }} - name: Install linter package run: python -m pip install git+https://github.com/shenxianpeng/cpp-linter-action/@refs/pull/42/head - name: Generate compiler database run: mkdir build && cmake -Bbuild src - name: run linter as package id: linter env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: cpp_linter -s=file -i=build -p=build -V=${{ matrix.clang-version }}
But it doesn't work using:
steps: - uses: actions/checkout@v2 - name: Generate compilation database run: mkdir build && cmake -Bbuild src - name: run linter as action uses: shenxianpeng/cpp-linter-action@refs/pull/42/head id: linter env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: style: file ignore: build database: build version: ${{ matrix.clang-version }}
I think its a File system access thing.
I think its ok to merge this, but I'd really like to get the
databaseoption to work when specified. Its causing clang-tidy to crash which we have to ignore because of multiple files. To be clear, it does work as a python package executable:
steps: - uses: actions/checkout@v2 - uses: actions/setup-python@v1 - name: Install clang-tools run : sudo apt-get update && sudo apt-get install clang-tidy-${{ matrix.clang-version }} clang-format-${{ matrix.clang-version }} - name: Install linter package run: python -m pip install git+https://github.com/shenxianpeng/cpp-linter-action/@refs/pull/42/head - name: Generate compiler database run: mkdir build && cmake -Bbuild src - name: run linter as package id: linter env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: cpp_linter -s=file -i=build -p=build -V=${{ matrix.clang-version }}But it doesn't work using:
steps: - uses: actions/checkout@v2 - name: Generate compilation database run: mkdir build && cmake -Bbuild src - name: run linter as action uses: shenxianpeng/cpp-linter-action@refs/pull/42/head id: linter env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: style: file ignore: build database: build version: ${{ matrix.clang-version }}
I think its a File system access thing.
I cannot reproduce the bug you mentioned. See a github run action result.
If you believe it's a bug, you can open another issue and provide a minimal reproducible example.
@DingXuefeng Your example workflow doesn't actually use a generated build/compile_commands.json.
- name: Generate compilation database run: mkdir build && cmake -Bbuild . -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
What happens when there is a database that exists?
Hi, I can reproduce what you see now.
The problem is due to the fact that compile_commands.json used absolute path, while in the docker they are mapped to /github/workspace folder. I have no good solution now. Maybe ln -sf $GITHUB_WORKSPACE /github/workspace.
Wondering how other github actions handled this.
Wondering how other github actions handled this.
We can use the built-in env var: GITHUB_WORKSPACE. I've already push that change with consideration that it isn't needed locally (executed when not on a runner).
Kudos, SonarCloud Quality Gate passed! Quality Gate passed
Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells
No Coverage information No Coverage information
No Duplication information No Duplication information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @DingXuefeng ! I changed the fix to use the RUNNER_WOKSPACE env var. Tested it again, and everything works as expected.
Well done. thank you both!
* use header's accept for REST API calls * also fix some spelling errors * reduce 3 lines to 1; invoke test in demo * get event payload for any supported event * don't delete comments from the response buffer * skip removing thread comments if none found * try requesting with the accepted format in the header * try using token in the header * Update `version` in README.md (#43) * [feat] Add the ability to specify the compilation database (#42) * add database option * fix existing typos * use header's accept for REST API calls * also fix some spelling errors * reduce 3 lines to 1; invoke test in demo * get event payload for any supported event * don't delete comments from the response buffer * skip removing thread comments if none found * try requesting with the accepted format in the header * try using token in the header * force thread comments disabled on private repos * private flag in the header is a bool, not str * Pleasing pylint * remove duplicated fix * update README about private repos needing a token Co-authored-by: 2bndy5 <2bndy5@gmail.com> Co-authored-by: Peter Shen <xianpeng.shen@gmail.com> Co-authored-by: Xuefeng Ding <xuefeng.ding.physics@gmail.com>
Add ability to specify the compilation database through
clang-format -p xxx.jsonmachinery, so that the head files can be included properly.