-
Notifications
You must be signed in to change notification settings - Fork 19
Add coverity support to workflow #26
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,3 +60,39 @@ jobs: | |
-D AUTH_LEGACY=OFF \ | ||
-G Ninja | ||
ninja -C build-auth wsbrd | ||
- name: Download Coverity | ||
if: github.ref == 'refs/heads/main' | ||
env: | ||
COVERITY_SCAN_TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }} | ||
run: | | ||
wget -qO- "https://scan.coverity.com/download/cxx/linux64" \ | ||
--post-data "token=${COVERITY_SCAN_TOKEN}&project=wisun-br-linux" \ | ||
| tar -xz --one-top-level=/opt/coverity/ --strip-components=1 | ||
ln -sfn /opt/coverity/bin/cov* /usr/local/bin | ||
coverity --version | ||
- name: Compile with Coverity | ||
if: github.ref == 'refs/heads/main' | ||
run: | | ||
cmake -S . \ | ||
-B build-cov \ | ||
-D COMPILE_WSRD=ON \ | ||
-D COMPILE_DEVTOOLS=ON \ | ||
-D COMPILE_DEMOS=ON \ | ||
-D CMAKE_C_FLAGS=-Werror \ | ||
-G Ninja | ||
cov-build --dir cov-int ninja -C build-cov | ||
- name: Upload Coverity Scan Results | ||
if: github.ref == 'refs/heads/main' | ||
env: | ||
COVERITY_SCAN_TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }} | ||
COVERITY_SCAN_EMAIL: ${{ secrets.COVERITY_SCAN_EMAIL }} | ||
COVERITY_SCAN_VERSION: ${{ github.sha }} | ||
Comment on lines
+86
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to use env vars? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: I now understand the need for secrets, but is it necessary for the SHA? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not necessary, there is plenty of other ways to do it, the aim here was reusability by other teams working on open sources projects. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we want something generic and reusable, the good way is to write an action. Maybe in the future we could have a Silabs action for Coverity if we have common steps. btw, there seems to be an official Coverity GitHub action, I don't know how usable it is: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer relying on simple commands directly over using GitHub Actions. GA abstract too much, making it feel like "magic" or "black box". For me, it's the same logic as with Jenkins plugin: the less you rely on it, the happier you are. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point, it is nice to view the Workflow as a list of commands that one can execute locally by copy-pasting. I don't know much about Coverity, but it looks quite similar to SonarQube in being "magic / black box". |
||
run: | | ||
tar czf cov-int.tgz cov-int | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does "int" mean here? |
||
curl --fail \ | ||
--form token="$COVERITY_SCAN_TOKEN" \ | ||
--form email="$COVERITY_SCAN_EMAIL" \ | ||
--form [email protected] \ | ||
--form version="$COVERITY_SCAN_VERSION" \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we use something more human friendly than a SHA? The output of |
||
--form description="Coverity scan results for commit $COVERITY_SCAN_VERSION" \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this description necessary? It currently does not provide much information since the version is already provided. I could not even find where this description can be read on the frontend 😅 |
||
"https://scan.coverity.com/builds?project=wisun-br-linux" |
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 this deserves a separate workflow.
Also do we even want to automatically run Coverity on every PR/push? (I don't know the deal that we have or how much it costs)
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 will let you choose on this.
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.
Can you move this into a separate workflow then? This will make it easy to tweak the run condition without affecting the regular build.
Also we will be able to use jobs.<job_id>.if instead of jobs.<job_id>.steps[*].if.