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

add coverity support for github actions #128

Merged
merged 2 commits into from
Feb 3, 2022
Merged

Conversation

jeking3
Copy link
Collaborator

@jeking3 jeking3 commented Jan 28, 2022

This change adds Coverity Scan integration.
It requires two GitHub Secrets in your repository.

I created a Coverity Scan project for boost-ci so we can test this is isolation.
I added GitHub Secrets COVERITY_SCAN_NOTIFICATION_EMAIL and COVERITY_SCAN_TOKEN to the repo.

Note that there is no way to disable a matrix run at the job level (easily). I configured it so coverity does not run on pull requests, only on pushes into develop, master, and feature/* branches by default. Therefore, in the checks for this pull request you will see a Coverity job that completes, but it does not submit anything as the Coverity step is skipped in pull requests. I also pushed a feature branch called feature/coverity, which does push results. Here's the fully enabled job in that branch push: https://github.com/boostorg/boost-ci/runs/5013565237?check_suite_focus=true, and here's a link to Coverity where it uploaded: https://scan.coverity.com/projects/boostorg-boost-ci?tab=overview

To eliminate the coverity job on pull requests I would have to copy most of the posix job steps into their own coverity job, and I figured that level of step code duplication without GHA support for YAML anchors would be less than awesome. So this is the best compromise.

I also did more work to allow ccache to be fully disabled; this is important because I could not get coverity to work correctly with ccache running before the compiler (I tried at least 20 times with different options and whatnot).

@jeking3 jeking3 added the enhancement New feature or request label Jan 28, 2022
@jeking3 jeking3 self-assigned this Jan 28, 2022
@codecov
Copy link

codecov bot commented Jan 28, 2022

Codecov Report

Merging #128 (c53cae1) into master (39c21a0) will not change coverage.
The diff coverage is n/a.

❗ Current head c53cae1 differs from pull request most recent head df147c7. Consider uploading reports for the commit df147c7 to get more accurate results

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #128   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines           16        16           
  Branches         7         7           
=========================================
  Hits            16        16           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39c21a0...df147c7. Read the comment docs.

Copy link
Collaborator

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments.
BTW: Where is the yamllint used actually? Should we have a boost-ci exclusive job running that?

Also good idea about the if: env.B2_USE_CCACHE condition for the cache action. Missed that.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
ci/github/coverity.sh Outdated Show resolved Hide resolved
ci/github/coverity.sh Outdated Show resolved Hide resolved
ci/build.sh Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@jeking3
Copy link
Collaborator Author

jeking3 commented Jan 28, 2022

You will see a number of force pushes to this PR so I can test the changes.

@jeking3
Copy link
Collaborator Author

jeking3 commented Jan 28, 2022

I used yamllint to make sure the ci.yml file was okay structurally. We could have a CI job for it if we want but I did not add it.

@jeking3
Copy link
Collaborator Author

jeking3 commented Jan 28, 2022

I'm using a separate branch to prove out changes so this doesn't keep running CI.

@jeking3
Copy link
Collaborator Author

jeking3 commented Jan 28, 2022

@Flamefire got this passing with uuid again, I think I addressed concerns. Let me know!

Copy link
Collaborator

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry forgot to submit the review.
A few minor things, check what you like to do.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.yamllint Outdated Show resolved Hide resolved
ci/github/coverity.sh Outdated Show resolved Hide resolved
ci/github/coverity.sh Outdated Show resolved Hide resolved
ci/github/coverity.sh Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@jeking3 jeking3 changed the title add coverity support for github actions, fix .yamllint add coverity support for github actions Jan 31, 2022
@jeking3
Copy link
Collaborator Author

jeking3 commented Feb 1, 2022

I simplified the changes. The feature branch push resulted in a Coverity Scan upload:

https://scan.coverity.com/projects/boostorg-boost-ci?tab=overview

I could not get a job to be skipped from a matrix input. The if statement at the job level cannot see the matrix object. So this is the best we're going to have for now.

@jeking3
Copy link
Collaborator Author

jeking3 commented Feb 1, 2022

@Flamefire ready for another review

Copy link
Collaborator

@Flamefire Flamefire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Almost done. Found 2 remaining issues (1 only stylistic) and added my thoughts to the coverity-not-skip-issue.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Flamefire Flamefire merged commit 00f6024 into master Feb 3, 2022
@jeking3 jeking3 deleted the jeking3/github-coverity branch February 4, 2022 19:27
@Flamefire Flamefire mentioned this pull request Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants