Add a GitHub workflow to submit builds to Coverity Scan
cover Add a GitHub workflow to submit builds to Coverity Scan
Coverity [https://scan.coverity.com/] is a powerful static analysis tool that helps prevent vulnerabilities. It is free to use by open source projects, and Git benefits from this, as well as Git for Windows. As is the case with many powerful tools, using Coverity comes with its own set of challenges, one of which being that submitting a build is quite laborious.
The help with this, the Git for Windows project has an Azure Pipeline for several years already to automate submitting builds to Coverity Scan:
https://dev.azure.com/git-for-windows/git/_build/index?definitionId=35
It is time to move this automation off of Azure Pipelines, and I thought that the Git project itself might as well benefit from this workflow.
Since Coverity build submissions require access (and a token to authenticate), this workflow is skipped by default. To enable it, the repository variable
[https://docs.github.com/en/actions/learn-github-actions/variables]ENABLE_COVERITY_SCAN_FOR_BRANCHES needs to be added. Its value needs to be a JSON string array containing the branch names, e.g. ["master", "next"]. Further, two repository secrets
[https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions]
need to be set: COVERITY_SCAN_EMAIL and COVERITY_SCAN_TOKEN.
An example run in the Git for Windows project can be admired here:
https://github.com/git-for-windows/git/actions/runs/6272393351/job/17033838405
To prove out that it would also work with the git Coverity project and building on operating systems other than Windows, I added two throw-away commits disabling the actual submission of the build to Coverity Scan (and also the main.yml CI to save on electrons) and pushed the branch to my fork. The ubuntu-latest run
[https://github.com/dscho/git/actions/runs/6272014876/job/17032859462], the windows-latest run
[https://github.com/dscho/git/actions/runs/6272014876/job/17032859234] and the macos-latest run
[https://github.com/dscho/git/actions/runs/6272014876/job/17032710138] all worked as expected.
This patch series is based on that Azure Pipeline, the support code in
https://github.com/git-for-windows/build-extra/blob/0e0b919073fb/please.sh#L835-L968,
and is very loosely inspired by
https://lore.kernel.org/git/4590e1381feb8962cadf2b40b22086531d662ef8.1692675172.git.me@ttaylorr.com/
(but you may not know it from comparing the patches because they look so vastly different). The reason why this patch series is so different is quite sad because I got very excited about the simplicity of using the GitHub Action vapier/coverity-scan-action. On paper, this Action looks really neat, but its implementation left me wanting, in particular because it does not even work (cov-configure must be called these days, and that Action simply does not, causing the entire build to fail), lacks support for Windows and macOS, fails to cache the Coverity Tool if the build fails for reasons unrelated to downloading & extracting the tool, and the activity in its issue tracker suggests to me that it is neither used nor maintained actively.
This patch series is based on v2.42.0, but would apply literally everywhere because it adds a new file and modifies no existing one.
Johannes Schindelin (6):
ci: add a GitHub workflow to submit Coverity scans
coverity: cache the Coverity Build Tool
coverity: allow overriding the Coverity project
coverity: support building on Windows
coverity: allow running on macOS
coverity: detect and report when the token or project is incorrect
.github/workflows/coverity.yml | 159 +++++++++++++++++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 .github/workflows/coverity.yml
base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1588%2Fdscho%2Fcoverity-workflow-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1588/dscho/coverity-workflow-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1588
Signature · 1 line
gitgitgadget
PATCH 1/6 ci: add a GitHub workflow to submit Coverity scans
From: Johannes Schindelin <[email protected]>
Coverity is a static analysis tool that detects and generates reports on various security and code quality issues.
It is particularly useful when diagnosing memory safety issues which may be used as part of exploiting a security vulnerability.
Coverity's website provides a service that accepts "builds" (which contains the object files generated during a standard build as well as a database generated by Coverity's scan tool).
Let's add a GitHub workflow to automate all of this. To avoid running it without appropriate Coverity configuration (e.g. the token required to use Coverity's services), the job only runs when the repository variable "ENABLE_COVERITY_SCAN_FOR_BRANCHES" has been configured accordingly (see
https://docs.github.com/en/actions/learn-github-actions/variables for details how to configure repository variables): It is expected to be a valid JSON array of branch strings, e.g. ["main", "next"].
In addition, this workflow requires two repository secrets:
- COVERITY_SCAN_EMAIL: the email to send the report to, and
- COVERITY_SCAN_TOKEN: the Coverity token (look in the Project Settings tab of your Coverity project).
Note: The initial version of this patch used vapier/coverity-scan-action to benefit from that Action's caching of the Coverity tool, which is rather large. Sadly, that Action only supports Linux, and we want to have the option of building on Windows, too. Besides, in the meantime Coverity requires cov-configure to be runantime, and that Action was not adjusted accordingly, i.e. it seems not to be maintained actively. Therefore it would seem prudent to implement the steps manually instead of using that Action.
--- .github/workflows/coverity.yml | 56 ++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 .github/workflows/coverity.yml
.github/workflows/coverity.yml new file +56 -0
@@ -0,0 +1,56 @@ +name: Coverity + +# This GitHub workflow automates submitting builds to Coverity Scan. To enable it, +# set the repository variable `ENABLE_COVERITY_SCAN_FOR_BRANCHES` (for details, see +# https://docs.github.com/en/actions/learn-github-actions/variables) to a JSON +# string array containing the names of the branches for which the workflow should be +# run, e.g. `["main", "next"]`. +# +# In addition, two repository secrets must be set (for details how to add secrets, see +# https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions): +# `COVERITY_SCAN_EMAIL` and `COVERITY_SCAN_TOKEN`. The former specifies the +# email to which the Coverity reports should be sent and the latter can be +# obtained from the Project Settings tab of the Coverity project). + +on: + push: + +jobs: + coverity: + if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) + runs-on: ubuntu-latest + env: + COVERITY_PROJECT: git + COVERITY_LANGUAGE: cxx + COVERITY_PLATFORM: linux64 + steps: + - uses: actions/checkout@v3 + - run: ci/install-dependencies.sh + env: + runs_on_pool: ubuntu-latest + + - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + run: | + curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --no-progress-meter \ + --output $RUNNER_TEMP/cov-analysis.tgz \ + --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT" + - name: extract the Coverity Build Tool + run: | + mkdir $RUNNER_TEMP/cov-analysis && + tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis + - name: build with cov-build + run: | + export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" && + cov-configure --gcc && + cov-build --dir cov-int make -j$(nproc) + - name: package the build + run: tar -czvf cov-int.tgz cov-int + - name: submit the build to Coverity Scan + run: | + curl \ + --form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \ + --form email="${{ secrets.COVERITY_SCAN_EMAIL }}" \ + --form [email protected] \ + --form version="${{ github.sha }}" \ + "https://scan.coverity.com/builds?project=$COVERITY_PROJECT"
Signature · 1 line
gitgitgadget
Re: [PATCH 1/6] ci: add a GitHub workflow to submit Coverity scans
Quoted 8 lines · Johannes Schindelin via GitGitGadget
Note: The initial version of this patch used
vapier/coverity-scan-actionto benefit from that Action's caching of the Coverity tool, which is rather large. Sadly, that Action only supports Linux, and we want to have the option of building on Windows, too. Besides, in the meantime Coverity requirescov-configureto be runantime, and that Action was not adjusted accordingly, i.e. it seems not to be maintained actively. Therefore it would seem prudent to implement the steps manually instead of using that Action.
I'm still unsure of the cov-configure thing, as I have never needed it (and the "vapier" Action worked fine for me). But the lack of Windows support is obviously a deal-breaker. I wondered if it might be worth trying to submit a PR to that project to fix it for everybody, but I saw that you commented on their "Windows support" issue, which has been sitting unanswered since it was opened in May. It's possible they might be more responsive to an actual PR, but I agree that it may be simpler to just go our own way here.
.github/workflows/coverity.yml line 25
Quoted 8 lines
.github/workflows/coverity.yml new file +8
-0+jobs: + coverity: + if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) + runs-on: ubuntu-latest + env: + COVERITY_PROJECT: git + COVERITY_LANGUAGE: cxx + COVERITY_PLATFORM: linux64
Ah, now I see why you were bothered by using "git/git" at the project name earlier. That is what I assumed we would use (and certainly I use "peff/git" on the Coverity side), but I forgot that we already have the general "git" name on the Coverity side (which isn't to say we couldn't switch to using the "git/git" name, but I am happy for us to be just "git" there).
.github/workflows/coverity.yml line 37
Quoted 12 lines
.github/workflows/coverity.yml new file +12
-0+ steps: + - uses: actions/checkout@v3 + - run: ci/install-dependencies.sh + env: + runs_on_pool: ubuntu-latest + + - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + run: | + curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --no-progress-meter \ + --output $RUNNER_TEMP/cov-analysis.tgz \ + --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT"
You might want "--fail" or "--fail-with-body" here. I think any server-side errors (like a missing or invalid token or project name) will result in a 401. Having curl reported that as a non-zero exit should stop the Action with a failure, rather than silently continuing.
This is mostly a style suggestion, but I think you can use:
--form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \
--form project="$COVERITY_PROJECT"here, which IMHO is a little more readable than "data". It probably doesn't matter in practice, but I think it would also would handle any encoding for us (though note that if we wanted to be really careful, the TOKEN secret would need shell quoting).
Using --form will use multipart/form-data instead of application/x-www-form-url-encoded, but coverity seems happy with either.
.github/workflows/coverity.yml line 41
Quoted 4 lines
.github/workflows/coverity.yml new file +4
-0+ - name: extract the Coverity Build Tool + run: | + mkdir $RUNNER_TEMP/cov-analysis && + tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis
OK, we are starting without Windows support yet. :)
.github/workflows/coverity.yml line 48
Quoted 7 lines
.github/workflows/coverity.yml new file +7
-0+ - name: build with cov-build + run: | + export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" && + cov-configure --gcc && + cov-build --dir cov-int make -j$(nproc) + - name: package the build + run: tar -czvf cov-int.tgz cov-int
OK, this looks a lot like what my custom rule does (no surprise, since we are all adapting Coverity's instructions).
.github/workflows/coverity.yml line 56
Quoted 8 lines
.github/workflows/coverity.yml new file +8
-0+ - name: submit the build to Coverity Scan + run: | + curl \ + --form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \ + --form email="${{ secrets.COVERITY_SCAN_EMAIL }}" \ + --form [email protected] \ + --form version="${{ github.sha }}" \ + "https://scan.coverity.com/builds?project=$COVERITY_PROJECT"
Likewise. I add:
--form description="$(./git version)"
to mine, but I am not even sure where that ends up (the "version" is probably the most interesting bit, as that is shown on the Coverity project page).
I notice you put the "project" variable in the query string. Can it be a --form, too, for symmetry? (In mine, I seem to have it as _both_, which is probably just a mistake). Not a huge deal either way, but just a small readability thing.
As with above, we'd probably want --fail or --fail-with-body here to detect errors (since otherwise a failed upload goes completely unreported).
-Peff
Re: [PATCH 1/6] ci: add a GitHub workflow to submit Coverity scans
Hi Peff,
Quoted 11 lines · Jeff King
Quoted 8 lines · Johannes Schindelin via GitGitGadget
Note: The initial version of this patch used
vapier/coverity-scan-actionto benefit from that Action's caching of the Coverity tool, which is rather large. Sadly, that Action only supports Linux, and we want to have the option of building on Windows, too. Besides, in the meantime Coverity requirescov-configureto be runantime, and that Action was not adjusted accordingly, i.e. it seems not to be maintained actively. Therefore it would seem prudent to implement the steps manually instead of using that Action.
Quoted 3 lines
I'm still unsure of the cov-configure thing, as I have never needed it (and the "vapier" Action worked fine for me). But the lack of Windows support is obviously a deal-breaker.
It is quite possible that I only verified that cov-configure --gcc needs to be called when running on Windows, and not on Linux, as there were many more deal breakers to convince me that we should _not_ use vapier/coverity-scan-action. Unless we fork it into the git org and start maintaining it ourselves, which is an option to consider.
Quoted 10 lines
.github/workflows/coverity.yml line 37
Quoted 6 lines
.github/workflows/coverity.yml new file +6
-0+ - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + run: | + curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --no-progress-meter \ + --output $RUNNER_TEMP/cov-analysis.tgz \ + --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT"You might want "--fail" or "--fail-with-body" here. I think any server-side errors (like a missing or invalid token or project name) will result in a 401.
Sadly, https://curl.se/docs/manpage.html#-f says this:
This method is not fail-safe and there are occasions where non-successful response codes slip through, especially when authentication is involved (response codes 401 and 407).
401 is the precise case we're hitting when the token or the project name are incorrect.
Having said that, I just tested with this particular host, and curl -f does fail with [exit code 22](https://curl.se/docs/manpage.html#22) as one would desire. So I will make that change.
As to --fail-with-body: it is too new to use (it was [introduced in cURL v7.76.0](https://curl.se/docs/manpage.html#--fail-with-body) and Ubuntu v20.04 [comes with v7.68.0](https://packages.ubuntu.com/search?suite=focal&searchon=names&keywords=curl), i.e. is missing that option).
In any case, in my tests, --fail-with-body did not show anything more than --fail in this instance. Maybe for you it is different?
Quoted 4 lines
This is mostly a style suggestion, but I think you can use:
--form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \ --form project="$COVERITY_PROJECT"
That is how I did things in Git for Windows, but at some stage I copied over code from vapier/coverity-scan-action. It is yet another slight code smell about that Action that it sometimes uses --data and sometimes --form:
https://github.com/vapier/coverity-scan-action/blob/cae3c096a2eb21c431961a49375ac17aea2670ce/action.yml#L89
https://github.com/vapier/coverity-scan-action/blob/cae3c096a2eb21c431961a49375ac17aea2670ce/action.yml#L118
Quoted 2 lines
I notice you put the "project" variable in the query string. Can it be a --form, too, for symmetry?
The instructions at https://scan.coverity.com/projects/git/builds/new (in the "Automation" section) are very clear that project should be passed as a GET variable.
Even if using a POST variable would work, I'd rather stay with the officially-documented way.
Ciao,
Johannes
Re: [PATCH 1/6] ci: add a GitHub workflow to submit Coverity scans
Quoted 10 lines · Johannes Schindelin
Quoted 3 lines
You might want "--fail" or "--fail-with-body" here. I think any server-side errors (like a missing or invalid token or project name) will result in a 401.
Sadly, https://curl.se/docs/manpage.html#-f says this:
This method is not fail-safe and there are occasions where non-successful response codes slip through, especially when authentication is involved (response codes 401 and 407).
Quoted 6 lines
401 is the precise case we're hitting when the token or the project name are incorrect.
Having said that, I just tested with this particular host, and
curl -fdoes fail with [exit code 22](https://curl.se/docs/manpage.html#22) as one would desire. So I will make that change.
The manpage is rather vague about what those "occasions" are, but I think we are probably OK since we are not sending HTTP-level credentials, according to:
https://github.com/curl/curl/commit/cde5e35d9b046b224c64936c432d67c9de8bcc9e
At any rate, even if this did not catch every failure, I think we are better off catching more rather than fewer.
Quoted 8 lines
As to
--fail-with-body: it is too new to use (it was [introduced in cURL v7.76.0](https://curl.se/docs/manpage.html#--fail-with-body) and Ubuntu v20.04 [comes with v7.68.0](https://packages.ubuntu.com/search?suite=focal&searchon=names&keywords=curl), i.e. is missing that option).In any case, in my tests,
--fail-with-bodydid not show anything more than--failin this instance. Maybe for you it is different?
No, I don't think it's worth worrying about here. It could help with debugging if their server returns something generic like a 500 code along with a more detailed reason (we do something similar in git-remote-curl to show failed bodies). But if it's at all more hassle than typing "--with-body" it is not worth the effort.
Quoted 9 lines
Quoted 2 lines
I notice you put the "project" variable in the query string. Can it be a --form, too, for symmetry?
The instructions at https://scan.coverity.com/projects/git/builds/new (in the "Automation" section) are very clear that
projectshould be passed as a GET variable.Even if using a POST variable would work, I'd rather stay with the officially-documented way.
That seems like good reasoning to me.
-Peff
PATCH 2/6 coverity: cache the Coverity Build Tool
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
It would add a 1GB+ download for every run, better cache it.
This is inspired by the GitHub Action vapier/coverity-scan-action, however, it uses the finer-grained restore/save method to be able to cache the Coverity Build Tool even if an unrelated step in the GitHub workflow fails later on.
--- .github/workflows/coverity.yml | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
.github/workflows/coverity.yml +25 -0
@@ -29,16 +29,41 @@ jobs: env: runs_on_pool: ubuntu-latest + # The Coverity site says the tool is usually updated twice yearly, so the + # MD5 of download can be used to determine whether there's been an update. + - name: get the Coverity Build Tool hash + id: lookup + run: | + MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") + echo "hash=$MD5" >>$GITHUB_OUTPUT + + # Try to cache the tool to avoid downloading 1GB+ on every run. + # A cache miss will add ~30s to create, but a cache hit will save minutes. + - name: restore the Coverity Build Tool + id: cache + uses: actions/cache/restore@v3 + with: + path: ${{ runner.temp }}/cov-analysis + key: cov-build-${{ env.COVERITY_LANGUAGE }}-${{ env.COVERITY_PLATFORM }}-${{ steps.lookup.outputs.hash }} - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + if: steps.cache.outputs.cache-hit != 'true' run: | curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --no-progress-meter \ --output $RUNNER_TEMP/cov-analysis.tgz \ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT" - name: extract the Coverity Build Tool + if: steps.cache.outputs.cache-hit != 'true' run: | mkdir $RUNNER_TEMP/cov-analysis && tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis + - name: cache the Coverity Build Tool + if: steps.cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v3 + with: + path: ${{ runner.temp }}/cov-analysis + key: cov-build-${{ env.COVERITY_LANGUAGE }}-${{ env.COVERITY_PLATFORM }}-${{ steps.lookup.outputs.hash }} - name: build with cov-build run: | export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" &&
Signature · 1 line
gitgitgadget
Re: [PATCH 2/6] coverity: cache the Coverity Build Tool
Quoted 8 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
It would add a 1GB+ download for every run, better cache it.
This is inspired by the GitHub Action
vapier/coverity-scan-action, however, it uses the finer-grainedrestore/savemethod to be able to cache the Coverity Build Tool even if an unrelated step in the GitHub workflow fails later on.
Nice. This is the big thing that I think the vapier action was providing us, and it does not look too bad. I have never used actions/cache before, but it all looks plausibly correct to me (and I assume you did a few test-runs to check it).
One note:
.github/workflows/coverity.yml jobs: · line 39
Quoted 8 lines
.github/workflows/coverity.yml +8
-0+ # The Coverity site says the tool is usually updated twice yearly, so the + # MD5 of download can be used to determine whether there's been an update. + - name: get the Coverity Build Tool hash + id: lookup + run: | + MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") + echo "hash=$MD5" >>$GITHUB_OUTPUT
We probably want --fail here, too (and presumably &&-chaining) so that we don't accidentally write a bogus cache entry. Possibly even check that $MD5 isn't blank if we want to be double-paranoid.
That made me wonder: if we do end up with a bogus cache entry, how does one clear it? And it looks like it can be managed directly via
https://github.com/$user/$project/actions/caches. Nice.
-Peff
Re: [PATCH 2/6] coverity: cache the Coverity Build Tool
Hi Peff,
Quoted 11 lines · Jeff King
Quoted 8 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
It would add a 1GB+ download for every run, better cache it.
This is inspired by the GitHub Action
vapier/coverity-scan-action, however, it uses the finer-grainedrestore/savemethod to be able to cache the Coverity Build Tool even if an unrelated step in the GitHub workflow fails later on.
Quoted 5 lines
Nice. This is the big thing that I think the vapier action was providing us, and it does not look too bad.
I have never used actions/cache before, but it all looks plausibly correct to me (and I assume you did a few test-runs to check it).
I use actions/cache extensively, both in GitHub workflows via the Action as well as in custom Actions like setup-git-for-windows-sdk, so I am confident that I am using this tool correctly here, too.
Quoted 12 lines
One note:
.github/workflows/coverity.yml jobs: · line 39
Quoted 8 lines
.github/workflows/coverity.yml +8
-0+ # The Coverity site says the tool is usually updated twice yearly, so the + # MD5 of download can be used to determine whether there's been an update. + - name: get the Coverity Build Tool hash + id: lookup + run: | + MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") + echo "hash=$MD5" >>$GITHUB_OUTPUT
Quoted 1 line
We probably want --fail here, too.
I concur, after verifying that the scary manual page note about authentication issues often not being handled correctly by curl --fail does not affect this particular scenario.
Ciao,
Johannes
PATCH 3/6 coverity: allow overriding the Coverity project
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
By default, the builds are submitted to the git project at
https://scan.coverity.com/projects/git.
The Git for Windows project would like to use this workflow, too, though, and needs the builds to be submitted to the git-for-windows Coverity project.
To that end, allow configuring the Coverity project name via the repository variable, you guessed it, COVERITY_PROJECT. The default if that variable is not configured or has an empty value is still git.
--- .github/workflows/coverity.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
.github/workflows/coverity.yml +4 -1
@@ -11,6 +11,9 @@ name: Coverity # `COVERITY_SCAN_EMAIL` and `COVERITY_SCAN_TOKEN`. The former specifies the # email to which the Coverity reports should be sent and the latter can be # obtained from the Project Settings tab of the Coverity project). +# +# By default, the builds are submitted to the Coverity project `git`. To override this, +# set the repository variable `COVERITY_PROJECT`. on: push:
@@ -20,7 +23,7 @@ jobs: if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) runs-on: ubuntu-latest env: - COVERITY_PROJECT: git + COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }} COVERITY_LANGUAGE: cxx COVERITY_PLATFORM: linux64 steps:
Signature · 1 line
gitgitgadget
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
.github/workflows/coverity.yml name: Coverity · line 16
Quoted 2 lines · Johannes Schindelin via GitGitGadget
.github/workflows/coverity.yml +2
-0+# By default, the builds are submitted to the Coverity project `git`. To override this, +# set the repository variable `COVERITY_PROJECT`.
This may not matter all that much, because I don't expect most people to set this up for their forks (and if we have git/git results that I have access to, I will probably even stop building my peff/git one). But I just wondered if a better default would be the GitHub project name (i.e., $user/git).
It has been long enough that I do not remember all of the setup on the Coverity side, but I assumed there was some "set it up for this GitHub project" button. But maybe I just picked that name myself.
-Peff
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
Hi Peff,
Quoted 7 lines · Jeff King
.github/workflows/coverity.yml name: Coverity · line 16
Quoted 2 lines · Johannes Schindelin via GitGitGadget
.github/workflows/coverity.yml +2
-0+# By default, the builds are submitted to the Coverity project `git`. To override this, +# set the repository variable `COVERITY_PROJECT`.This may not matter all that much, because I don't expect most people to set this up for their forks
Except, of course, Git for Windows. And that has been the entire motivation for me to work on this here patch series in the first place, so it would be rather pointless if I could not override this in the git-for-windows/git fork.
Of course, I could address this differently. I could add a commit on top and rebase that all the time. I'd just as well avoid that though. There is already too much stuff in the Git for Windows fork that I have to rebase so often.
Based on your response, I was on my way to enhance the commit message accordingly, but then I saw this already being there:
The Git for Windows project would like to use this workflow, too, though, and needs the builds to be submitted to the git-for-windows Coverity project.
Would you have any suggestion how that could make the motivation and intention of this patch clearer?
Ciao,
Johannes
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
Quoted 12 lines · Johannes Schindelin
Hi Peff,
Quoted 7 lines · Jeff King
.github/workflows/coverity.yml name: Coverity · line 16
Quoted 2 lines · Johannes Schindelin via GitGitGadget
.github/workflows/coverity.yml +2
-0+# By default, the builds are submitted to the Coverity project `git`. To override this, +# set the repository variable `COVERITY_PROJECT`.This may not matter all that much, because I don't expect most people to set this up for their forks
Quoted 4 lines
Except, of course, Git for Windows. And that has been the entire motivation for me to work on this here patch series in the first place, so it would be rather pointless if I could not override this in the git-for-windows/git fork.
I didn't at all mean that it should not be possible to override it. It was obvious to me you would want to do so for git-for-windows. I meant that the default should be $user/$fork from the Actions environment, rather than just "git". That would be a more appropriate default for people who wanted to run this out of their forks (but again, the part you quoted above is basically "I'm not sure anybody even wants to do that").
Quoted 8 lines
Of course, I could address this differently. I could add a commit on top and rebase that all the time. I'd just as well avoid that though. There is already too much stuff in the Git for Windows fork that I have to rebase so often.
Based on your response, I was on my way to enhance the commit message accordingly, but then I saw this already being there:
Quoted 6 lines
The Git for Windows project would like to use this workflow, too, though, and needs the builds to be submitted to the
git-for-windowsCoverity project.Would you have any suggestion how that could make the motivation and intention of this patch clearer?
I understood your intention. I think you misunderstood mine. :) The commit message is fine as-is, I think.
-Peff
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
Hi Jeff,
Quoted 13 lines · Jeff King
Quoted 11 lines · Johannes Schindelin
Quoted 8 lines · Jeff King
Review
Quoted 3 lines · Johannes Schindelin via GitGitGadget
(quoted diff) +3
-0+# By default, the builds are submitted to the Coverity project +# `git`. To override this, set the repository variable +# `COVERITY_PROJECT`.This may not matter all that much, because I don't expect most people to set this up for their forks
Quoted 6 lines
Quoted 4 lines
Except, of course, Git for Windows. And that has been the entire motivation for me to work on this here patch series in the first place, so it would be rather pointless if I could not override this in the git-for-windows/git fork.
I didn't at all mean that it should not be possible to override it.
Aha! The "This" in "This may not matter all that much" was referring to the git instead of the COVERITY_PROJECT part. That had not been clear to me.
Quoted 2 lines
I meant that the default should be $user/$fork from the Actions environment,
I'd much rather default to the value needed by the Git project than a value that may be needed by any other user. I do aim to support the Git project with this patch series, first and foremost.
Ciao,
Johannes
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
Quoted 6 lines · Johannes Schindelin
Quoted 2 lines
I meant that the default should be $user/$fork from the Actions environment,
I'd much rather default to the value needed by the Git project than a value that may be needed by any other user. I do aim to support the Git project with this patch series, first and foremost.
I appreciate the sentiment.
At the same time, it would be one less thing they need to tweak before starting to use it, and if there are two or more users to do so, it would already have paid off. Developers typically outnumber projects they work on.
I also have to wonder if it might make it more obvious what is going on if you made the default to $user/$fork and have the project override it, which hopefully may make it easier to find out what they need to do for those who want to override it to a different value to suit their need?
Thanks.
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
Quoted 10 lines · Junio C Hamano
At the same time, it would be one less thing they need to tweak before starting to use it, and if there are two or more users to do so, it would already have paid off. Developers typically outnumber projects they work on.
I also have to wonder if it might make it more obvious what is going on if you made the default to $user/$fork and have the project override it, which hopefully may make it easier to find out what they need to do for those who want to override it to a different value to suit their need?
Yeah, that was my thinking (and what I had been proposing).
But I really think it probably doesn't matter that much either way. I would not be surprised if there are zero developers who use this, because of the setup on the coverity side, and the fact that the results are not always immediately actionable.
Even I, who has been running coverity on my local fork for a few years, will probably just switch to using the git.git run and occasionally looking at the results (that creates an extra headache because somebody has to grant acess to the git.git run results to interested parties, but it's also a one-time setup thing).
-Peff
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
Quoted 20 lines · Jeff King
Quoted 10 lines · Junio C Hamano
At the same time, it would be one less thing they need to tweak before starting to use it, and if there are two or more users to do so, it would already have paid off. Developers typically outnumber projects they work on.
I also have to wonder if it might make it more obvious what is going on if you made the default to $user/$fork and have the project override it, which hopefully may make it easier to find out what they need to do for those who want to override it to a different value to suit their need?
Yeah, that was my thinking (and what I had been proposing).
But I really think it probably doesn't matter that much either way. I would not be surprised if there are zero developers who use this, because of the setup on the coverity side, and the fact that the results are not always immediately actionable.
Quoted 5 lines
Even I, who has been running coverity on my local fork for a few years, will probably just switch to using the git.git run and occasionally looking at the results (that creates an extra headache because somebody has to grant acess to the git.git run results to interested parties, but it's also a one-time setup thing).
Sure.
I do not care too much either way, and in a situation like this where the design decision does not have a crucial longer-term impact either way exactly because it is a one-time thing for any user, whoever has invested their work on should have the final say.
Thanks.
Re: [PATCH 3/6] coverity: allow overriding the Coverity project
In reply to Johannes SchindelinQuoted 8 lines · Johannes Schindelin
Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
This may not matter all that much, because I don't expect most people to set this up for their forks
Except, of course, Git for Windows. And that has been the entire motivation for me to work on this here patch series in the first place, so it would be rather pointless if I could not override this in the git-for-windows/git fork.
Quoted 5 lines
Quoted 1 line
I didn't at all mean that it should not be possible to override it.
Aha! The "This" in "This may not matter all that much" was referring to the
gitinstead of theCOVERITY_PROJECTpart. That had not been clear to me.
I think we are on the same page now, but I wanted to elaborate here because this miscommunication interested me. The "this" in what I wrote was actually the "X" in:
This may not matter because of $FOO, but X.
The X got snipped in what you quoted, but was "But I just wondered if a better default would would be...". I certainly did not help with readability by inserting a huge parenthetical phrase and putting in a full-stop and starting the new sentence on "But" (my ninth grade English teacher would be horrified).
I mention it mostly as a note to myself about trying to make sure I keep my writing clear and readable (and a cautionary tale for others!).
-Peff
PATCH 4/6 coverity: support building on Windows
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
By adding the repository variable ENABLE_COVERITY_SCAN_ON_OS with a value, say, ["windows-latest"], this GitHub workflow now runs on Windows, allowing to analyze Windows-specific issues.
--- .github/workflows/coverity.yml | 56 ++++++++++++++++++++++++++++++---- 1 file changed, 50 insertions(+), 6 deletions(-)
.github/workflows/coverity.yml +50 -6
@@ -12,31 +12,61 @@ name: Coverity # email to which the Coverity reports should be sent and the latter can be # obtained from the Project Settings tab of the Coverity project). # +# The workflow runs on `ubuntu-latest` by default. This can be overridden by setting +# the repository variable `ENABLE_COVERITY_SCAN_ON_OS` to a JSON string array specifying +# the operating systems, e.g. `["ubuntu-latest", "windows-latest"]`. +# # By default, the builds are submitted to the Coverity project `git`. To override this, # set the repository variable `COVERITY_PROJECT`. on: push: +defaults: + run: + shell: bash + jobs: coverity: if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) - runs-on: ubuntu-latest + strategy: + matrix: + os: ${{ fromJSON(vars.ENABLE_COVERITY_SCAN_ON_OS || '["ubuntu-latest"]') }} + runs-on: ${{ matrix.os }} env: COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }} COVERITY_LANGUAGE: cxx - COVERITY_PLATFORM: linux64 steps: - uses: actions/checkout@v3 + - name: install minimal Git for Windows SDK + if: contains(matrix.os, 'windows') + uses: git-for-windows/setup-git-for-windows-sdk@v1 - run: ci/install-dependencies.sh + if: contains(matrix.os, 'ubuntu') env: - runs_on_pool: ubuntu-latest + runs_on_pool: ${{ matrix.os }} # The Coverity site says the tool is usually updated twice yearly, so the # MD5 of download can be used to determine whether there's been an update. - name: get the Coverity Build Tool hash id: lookup run: | + case "${{ matrix.os }}" in + *windows*) + COVERITY_PLATFORM=win64 + COVERITY_TOOL_FILENAME=cov-analysis.zip + ;; + *ubuntu*) + COVERITY_PLATFORM=linux64 + COVERITY_TOOL_FILENAME=cov-analysis.tgz + ;; + *) + echo '::error::unhandled OS ${{ matrix.os }}' >&2 + exit 1 + ;; + esac + echo "COVERITY_PLATFORM=$COVERITY_PLATFORM" >>$GITHUB_ENV + echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") echo "hash=$MD5" >>$GITHUB_OUTPUT
@@ -54,13 +84,27 @@ jobs: run: | curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --no-progress-meter \ - --output $RUNNER_TEMP/cov-analysis.tgz \ + --output $RUNNER_TEMP/$COVERITY_TOOL_FILENAME \ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT" - name: extract the Coverity Build Tool if: steps.cache.outputs.cache-hit != 'true' run: | - mkdir $RUNNER_TEMP/cov-analysis && - tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis + case "$COVERITY_TOOL_FILENAME" in + *.tgz) + mkdir $RUNNER_TEMP/cov-analysis && + tar -xzf $RUNNER_TEMP/$COVERITY_TOOL_FILENAME --strip 1 -C $RUNNER_TEMP/cov-analysis + ;; + *.zip) + cd $RUNNER_TEMP && + mkdir cov-analysis-tmp && + unzip -d cov-analysis-tmp $COVERITY_TOOL_FILENAME && + mv cov-analysis-tmp/* cov-analysis + ;; + *) + echo "::error::unhandled archive type: $COVERITY_TOOL_FILENAME" >&2 + exit 1 + ;; + esac - name: cache the Coverity Build Tool if: steps.cache.outputs.cache-hit != 'true' uses: actions/cache/save@v3
Signature · 1 line
gitgitgadget
Re: [PATCH 4/6] coverity: support building on Windows
Quoted 5 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
By adding the repository variable
ENABLE_COVERITY_SCAN_ON_OSwith a value, say,["windows-latest"], this GitHub workflow now runs on Windows, allowing to analyze Windows-specific issues.
Makes sense. I figured we'd key this on COVERITY_PLATFORM itself, but I guess we need to map Actions environments to Coverity platform names, so starting with the Actions names makes sense.
.github/workflows/coverity.yml name: Coverity · line 17
Quoted 3 lines
.github/workflows/coverity.yml +3
-0+# The workflow runs on `ubuntu-latest` by default. This can be overridden by setting +# the repository variable `ENABLE_COVERITY_SCAN_ON_OS` to a JSON string array specifying +# the operating systems, e.g. `["ubuntu-latest", "windows-latest"]`.
OK. I was envisioning that we'd just run on one platform, and maybe git-for-windows would run on another. But it does not hurt to be able to do both from one repo. I'm not sure how Coverity presents that, but it should be able to figure out based on "version" and "platform" fields that they are two builds of the same version (and not, say, one overriding the other as the "latest").
-Peff
PATCH 5/6 coverity: allow running on macOS
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
For completeness' sake, let's add support for submitting macOS builds to Coverity Scan.
--- .github/workflows/coverity.yml | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
.github/workflows/coverity.yml +20 -2
@@ -42,7 +42,7 @@ jobs: if: contains(matrix.os, 'windows') uses: git-for-windows/setup-git-for-windows-sdk@v1 - run: ci/install-dependencies.sh - if: contains(matrix.os, 'ubuntu') + if: contains(matrix.os, 'ubuntu') || contains(matrix.os, 'macos') env: runs_on_pool: ${{ matrix.os }}
@@ -55,10 +55,17 @@ jobs: *windows*) COVERITY_PLATFORM=win64 COVERITY_TOOL_FILENAME=cov-analysis.zip + MAKEFLAGS=-j$(nproc) + ;; + *macos*) + COVERITY_PLATFORM=macOSX + COVERITY_TOOL_FILENAME=cov-analysis.dmg + MAKEFLAGS=-j$(sysctl -n hw.physicalcpu) ;; *ubuntu*) COVERITY_PLATFORM=linux64 COVERITY_TOOL_FILENAME=cov-analysis.tgz + MAKEFLAGS=-j$(nproc) ;; *) echo '::error::unhandled OS ${{ matrix.os }}' >&2
@@ -67,6 +74,7 @@ jobs: esac echo "COVERITY_PLATFORM=$COVERITY_PLATFORM" >>$GITHUB_ENV echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV + echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") echo "hash=$MD5" >>$GITHUB_OUTPUT
@@ -94,6 +102,16 @@ jobs: mkdir $RUNNER_TEMP/cov-analysis && tar -xzf $RUNNER_TEMP/$COVERITY_TOOL_FILENAME --strip 1 -C $RUNNER_TEMP/cov-analysis ;; + *.dmg) + cd $RUNNER_TEMP && + attach="$(hdiutil attach $COVERITY_TOOL_FILENAME)" && + volume="$(echo "$attach" | cut -f 3 | grep /Volumes/)" && + mkdir cov-analysis && + cd cov-analysis && + sh "$volume"/cov-analysis-macosx-*.sh && + ls -l && + hdiutil detach "$volume" + ;; *.zip) cd $RUNNER_TEMP && mkdir cov-analysis-tmp &&
@@ -115,7 +133,7 @@ jobs: run: | export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" && cov-configure --gcc && - cov-build --dir cov-int make -j$(nproc) + cov-build --dir cov-int make - name: package the build run: tar -czvf cov-int.tgz cov-int - name: submit the build to Coverity Scan
Signature · 1 line
gitgitgadget
Re: [PATCH 5/6] coverity: allow running on macOS
Quoted 4 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
For completeness' sake, let's add support for submitting macOS builds to Coverity Scan.
I don't have any real problem with this, and it will check a few extra bits of platform-specific code not covered elsewhere. My big question would be: how much extra does this cost to run each time?
I guess it is not too much (compared to regular CI); my coverity build job ran in 7 minutes, and that is including the download of the tool.
-Peff
Re: [PATCH 5/6] coverity: allow running on macOS
Hi Peff,
Quoted 9 lines · Jeff King
Quoted 4 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
For completeness' sake, let's add support for submitting macOS builds to Coverity Scan.
I don't have any real problem with this, and it will check a few extra bits of platform-specific code not covered elsewhere.
Just to make sure: The patch series, as presented here, will only build on ubuntu-latest by default, unless that is specifically overridden in the repository variables of git/git. It makes most sense to me that way.
Quoted 2 lines
My big question would be: how much extra does this cost to run each time?
I linked the example runs in the cover letter, which record the runtimes of the build with cov-build step as:
ubuntu-latest 5m20s windows-latest 17m40s macos-latest 1m59s
But again, I don't see much sense building git/git for anything else than ubuntu-latest, at least for now.
Ciao,
Johannes
Re: [PATCH 5/6] coverity: allow running on macOS
Quoted 9 lines · Johannes Schindelin
Quoted 5 lines
Quoted 2 lines
For completeness' sake, let's add support for submitting macOS builds to Coverity Scan.
I don't have any real problem with this, and it will check a few extra bits of platform-specific code not covered elsewhere.
Just to make sure: The patch series, as presented here, will only build on
ubuntu-latestby default, unless that is specifically overridden in the repository variables ofgit/git. It makes most sense to me that way.
Yeah, that makes sense to me, too. But then I wondered why we have this macOS code if nobody is going to run it. On the other hand, maybe it eventually will come in handy, and you already did the work, and it is not hurting anybody in the meantime.
-Peff
PATCH 6/6 coverity: detect and report when the token or project is incorrect
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When trying to obtain the MD5 of the Coverity Scan Tool (in order to decide whether a cached version can be used or a new version has to be downloaded), it is possible to get a 401 (Authorization required) due to either an incorrect token, or even more likely due to an incorrect Coverity project name.
Let's detect that scenario and provide a helpful error message instead of trying to go forward with an empty string instead of the correct MD5.
--- .github/workflows/coverity.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+)
.github/workflows/coverity.yml +13 -0
@@ -76,7 +76,20 @@ jobs: echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + -D "$RUNNER_TEMP"/headers.txt \ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1") + http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)" + case "$http_code" in + *200*) ;; # okay + *401*) # access denied + echo "::error::incorrect token or project? ($http_code)" >&2 + exit 1 + ;; + *) # other error + echo "::error::HTTP error $http_code" >&2 + exit 1 + ;; + esac echo "hash=$MD5" >>$GITHUB_OUTPUT # Try to cache the tool to avoid downloading 1GB+ on every run.
Signature · 1 line
gitgitgadget
Re: [PATCH 6/6] coverity: detect and report when the token or project is incorrect
Quoted 10 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
When trying to obtain the MD5 of the Coverity Scan Tool (in order to decide whether a cached version can be used or a new version has to be downloaded), it is possible to get a 401 (Authorization required) due to either an incorrect token, or even more likely due to an incorrect Coverity project name.
Let's detect that scenario and provide a helpful error message instead of trying to go forward with an empty string instead of the correct MD5.
Ah. :) I think using "curl --fail" is probably a simpler solution here.
-Peff
Re: [PATCH 6/6] coverity: detect and report when the token or project is incorrect
Hi Jeff,
Quoted 10 lines · Jeff King
Quoted 8 lines · Johannes Schindelin via GitGitGadget
From: Johannes Schindelin <[email protected]>
When trying to obtain the MD5 of the Coverity Scan Tool (in order to decide whether a cached version can be used or a new version has to be downloaded), it is possible to get a 401 (Authorization required) due to either an incorrect token, or even more likely due to an incorrect Coverity project name.
Quoted 4 lines
Quoted 2 lines
Let's detect that scenario and provide a helpful error message instead of trying to go forward with an empty string instead of the correct MD5.
Ah. :) I think using "curl --fail" is probably a simpler solution here.
Apart from the unintuitive error message. I myself was puzzled and struggled quite a few times until I realized that it was not the token that was incorrect, but the project name.
To help people with a similar mental capacity to my own, I would therefore really want to keep this here patch.
As a compromise, I will switch to using --fail and then looking at the exit code (with 22 indicating authentication issues).
Ciao,
Johannes
Re: [PATCH 6/6] coverity: detect and report when the token or project is incorrect
Quoted 9 lines · Johannes Schindelin
Quoted 4 lines
Quoted 2 lines
Let's detect that scenario and provide a helpful error message instead of trying to go forward with an empty string instead of the correct MD5.
Ah. :) I think using "curl --fail" is probably a simpler solution here.
Apart from the unintuitive error message. I myself was puzzled and struggled quite a few times until I realized that it was not the token that was incorrect, but the project name.
Quoted 5 lines
To help people with a similar mental capacity to my own, I would therefore really want to keep this here patch.
As a compromise, I will switch to using
--failand then looking at the exit code (with 22 indicating authentication issues).
Oh, I do not at all mind de-mystifying the error message for the user. I mostly meant that using --fail would be better than scraping the http code from the header file. Though as you note, I guess we have to interpret the exit code in this case anyway, so it is not that much of a simplification.
-Peff
v2 cover Add a GitHub workflow to submit builds to Coverity Scan
In reply to Johannes Schindelin via GitGitGadgetCoverity [https://scan.coverity.com/] is a powerful static analysis tool that helps prevent vulnerabilities. It is free to use by open source projects, and Git benefits from this, as well as Git for Windows. As is the case with many powerful tools, using Coverity comes with its own set of challenges, one of which being that submitting a build is quite laborious.
The help with this, the Git for Windows project created an Azure Pipeline to automate submitting builds to Coverity Scan
[https://dev.azure.com/git-for-windows/git/_build/index?definitionId=35]
which is ported to a GitHub workflow in this here patch series, keeping an eye specifically on allowing both the Git and the Git for Windows project to use this workflow.
Since Coverity build submissions require authentication, this workflow is skipped by default. To enable it, the repository variable
[https://docs.github.com/en/actions/learn-github-actions/variables]ENABLE_COVERITY_SCAN_FOR_BRANCHES needs to be added. Its value needs to be a JSON string array containing the branch names, e.g. ["master", "next"]. Further, two repository secrets
[https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions]
need to be set: COVERITY_SCAN_EMAIL and COVERITY_SCAN_TOKEN.
An example run in the Git for Windows project can be admired here
[https://github.com/git-for-windows/git/actions/runs/6298399881].
Note: While inspired by vapier/coverity-scan-action
[https://github.com/vapier/coverity-scan-action], I found too many limitations in that Action to be used here. However, I would be willing to fork that Action into the git organization on GitHub, improve the code to accommodate Git's needs, and maintain that Action, if there is enough support for taking that route instead of simply hard-coding the steps in Git's .github/workflows/coverity.yml.
This patch series is based on v2.42.0, but would apply literally everywhere because it adds a new file and modifies no existing one.
Changes since v1:
* After verifying that cURL's --fail option does what we need in Coverity's context, I switched to using that in every curl invocation.
* Adjusted quoting (the ${{ ... }} constructs do not require double quotes because they are interpolated before the script is run).
* Touched up a few commit messages, based on the feedback I received.
* Addressed an actionlint [https://rhysd.github.io/actionlint/] warning.
Johannes Schindelin (6):
ci: add a GitHub workflow to submit Coverity scans
coverity: cache the Coverity Build Tool
coverity: allow overriding the Coverity project
coverity: support building on Windows
coverity: allow running on macOS
coverity: detect and report when the token or project is incorrect
.github/workflows/coverity.yml | 163 +++++++++++++++++++++++++++++++++ 1 file changed, 163 insertions(+) create mode 100644 .github/workflows/coverity.yml
base-commit: 43c8a30d150ecede9709c1f2527c8fba92c65f40
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1588%2Fdscho%2Fcoverity-workflow-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1588/dscho/coverity-workflow-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1588
Range-diff vs v1:
1: 8cb92968c5e ! 1: 46fb6b583d3 ci: add a GitHub workflow to submit Coverity scans
@@ .github/workflows/coverity.yml (new)
+ - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}})
+ run: |
+ curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
-+ --no-progress-meter \
++ --fail --no-progress-meter \
+ --output $RUNNER_TEMP/cov-analysis.tgz \
-+ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT"
++ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
++ --form project="$COVERITY_PROJECT"
+ - name: extract the Coverity Build Tool
+ run: |
+ mkdir $RUNNER_TEMP/cov-analysis &&
@@ .github/workflows/coverity.yml (new)
+ - name: submit the build to Coverity Scan
+ run: |
+ curl \
-+ --form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \
-+ --form email="${{ secrets.COVERITY_SCAN_EMAIL }}" \
++ --fail \
++ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
++ --form email='${{ secrets.COVERITY_SCAN_EMAIL }}' \
+ --form [email protected] \
-+ --form version="${{ github.sha }}" \
++ --form version='${{ github.sha }}' \
+ "https://scan.coverity.com/builds?project=$COVERITY_PROJECT"
2: 8420a76eba3 ! 2: e26545b1ed9 coverity: cache the Coverity Build Tool
@@ .github/workflows/coverity.yml: jobs:
+ id: lookup
+ run: |
+ MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
-+ --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1")
++ --fail \
++ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
++ --form project="$COVERITY_PROJECT" \
++ --form md5=1) &&
+ echo "hash=$MD5" >>$GITHUB_OUTPUT
+
+ # Try to cache the tool to avoid downloading 1GB+ on every run.
@@ .github/workflows/coverity.yml: jobs:
+ if: steps.cache.outputs.cache-hit != 'true'
run: |
curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
- --no-progress-meter \
- --output $RUNNER_TEMP/cov-analysis.tgz \
- --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT"
+ --fail --no-progress-meter \
+@@ .github/workflows/coverity.yml: jobs:
+ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
+ --form project="$COVERITY_PROJECT"
- name: extract the Coverity Build Tool
+ if: steps.cache.outputs.cache-hit != 'true'
run: |
3: 6c1c8286281 = 3: ea85e351233 coverity: allow overriding the Coverity project 4: 14cdefff082 ! 4: 84e1c3eede8 coverity: support building on Windows
@@ Commit message
value, say, ["windows-latest"], this GitHub workflow now runs on Windows, allowing to analyze Windows-specific issues.
+ This allows, say, the Git for Windows fork to submit Windows builds to
+ Coverity Scan instead of Linux builds.
+
## .github/workflows/coverity.yml ##
@@ .github/workflows/coverity.yml: name: Coverity
COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }}
COVERITY_LANGUAGE: cxx
- COVERITY_PLATFORM: linux64
++ COVERITY_PLATFORM: overridden-below
steps:
- uses: actions/checkout@v3
+ - name: install minimal Git for Windows SDK
@@ .github/workflows/coverity.yml: name: Coverity
+ echo "COVERITY_PLATFORM=$COVERITY_PLATFORM" >>$GITHUB_ENV
+ echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV
MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
- --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1")
- echo "hash=$MD5" >>$GITHUB_OUTPUT
+ --fail \
+ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
@@ .github/workflows/coverity.yml: jobs:
run: |
curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
- --no-progress-meter \
+ --fail --no-progress-meter \
- --output $RUNNER_TEMP/cov-analysis.tgz \
+ --output $RUNNER_TEMP/$COVERITY_TOOL_FILENAME \
- --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT"
+ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
+ --form project="$COVERITY_PROJECT"
- name: extract the Coverity Build Tool
if: steps.cache.outputs.cache-hit != 'true'
run: |
5: 782cf2b4403 ! 5: 3d24b6f3b22 coverity: allow running on macOS
@@ .github/workflows/coverity.yml: jobs:
echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV
+ echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV
MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
- --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1")
- echo "hash=$MD5" >>$GITHUB_OUTPUT
+ --fail \
+ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
@@ .github/workflows/coverity.yml: jobs:
mkdir $RUNNER_TEMP/cov-analysis &&
tar -xzf $RUNNER_TEMP/$COVERITY_TOOL_FILENAME --strip 1 -C $RUNNER_TEMP/cov-analysis
6: 458bc2ea91f ! 6: b45cc4b8a25 coverity: detect and report when the token or project is incorrect
@@ Commit message
either an incorrect token, or even more likely due to an incorrect
Coverity project name.
- Let's detect that scenario and provide a helpful error message instead
- of trying to go forward with an empty string instead of the correct MD5.
+ Seeing an authorization failure that is caused by an incorrect project
+ name was somewhat surprising to me when developing the Coverity
+ workflow, as I found such a failure suggestive of an incorrect token
+ instead.
+
+ So let's provide a helpful error message about that specifically when
+ encountering authentication issues.
## .github/workflows/coverity.yml ##
@@ .github/workflows/coverity.yml: jobs:
- echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV
- echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV
- MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \
-+ -D "$RUNNER_TEMP"/headers.txt \
- --data "token=${{ secrets.COVERITY_SCAN_TOKEN }}&project=$COVERITY_PROJECT&md5=1")
-+ http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)"
-+ case "$http_code" in
-+ *200*) ;; # okay
-+ *401*) # access denied
-+ echo "::error::incorrect token or project? ($http_code)" >&2
+ --fail \
+ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
+ --form project="$COVERITY_PROJECT" \
+- --form md5=1) &&
++ --form md5=1)
++ case $? in
++ 0) ;; # okay
++ *22*) # 40x, i.e. access denied
++ echo "::error::incorrect token or project?" >&2
+ exit 1
+ ;;
+ *) # other error
-+ echo "::error::HTTP error $http_code" >&2
++ echo "::error::Failed to retrieve MD5" >&2
+ exit 1
+ ;;
+ esac
Signature · 1 line
gitgitgadget
PATCH v2 1/6 ci: add a GitHub workflow to submit Coverity scans
From: Johannes Schindelin <[email protected]>
Coverity is a static analysis tool that detects and generates reports on various security and code quality issues.
It is particularly useful when diagnosing memory safety issues which may be used as part of exploiting a security vulnerability.
Coverity's website provides a service that accepts "builds" (which contains the object files generated during a standard build as well as a database generated by Coverity's scan tool).
Let's add a GitHub workflow to automate all of this. To avoid running it without appropriate Coverity configuration (e.g. the token required to use Coverity's services), the job only runs when the repository variable "ENABLE_COVERITY_SCAN_FOR_BRANCHES" has been configured accordingly (see
https://docs.github.com/en/actions/learn-github-actions/variables for details how to configure repository variables): It is expected to be a valid JSON array of branch strings, e.g. ["main", "next"].
In addition, this workflow requires two repository secrets:
- COVERITY_SCAN_EMAIL: the email to send the report to, and
- COVERITY_SCAN_TOKEN: the Coverity token (look in the Project Settings tab of your Coverity project).
Note: The initial version of this patch used vapier/coverity-scan-action to benefit from that Action's caching of the Coverity tool, which is rather large. Sadly, that Action only supports Linux, and we want to have the option of building on Windows, too. Besides, in the meantime Coverity requires cov-configure to be runantime, and that Action was not adjusted accordingly, i.e. it seems not to be maintained actively. Therefore it would seem prudent to implement the steps manually instead of using that Action.
--- .github/workflows/coverity.yml | 58 ++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .github/workflows/coverity.yml
.github/workflows/coverity.yml new file +58 -0
@@ -0,0 +1,58 @@ +name: Coverity + +# This GitHub workflow automates submitting builds to Coverity Scan. To enable it, +# set the repository variable `ENABLE_COVERITY_SCAN_FOR_BRANCHES` (for details, see +# https://docs.github.com/en/actions/learn-github-actions/variables) to a JSON +# string array containing the names of the branches for which the workflow should be +# run, e.g. `["main", "next"]`. +# +# In addition, two repository secrets must be set (for details how to add secrets, see +# https://docs.github.com/en/actions/security-guides/using-secrets-in-github-actions): +# `COVERITY_SCAN_EMAIL` and `COVERITY_SCAN_TOKEN`. The former specifies the +# email to which the Coverity reports should be sent and the latter can be +# obtained from the Project Settings tab of the Coverity project). + +on: + push: + +jobs: + coverity: + if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) + runs-on: ubuntu-latest + env: + COVERITY_PROJECT: git + COVERITY_LANGUAGE: cxx + COVERITY_PLATFORM: linux64 + steps: + - uses: actions/checkout@v3 + - run: ci/install-dependencies.sh + env: + runs_on_pool: ubuntu-latest + + - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + run: | + curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --fail --no-progress-meter \ + --output $RUNNER_TEMP/cov-analysis.tgz \ + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ + --form project="$COVERITY_PROJECT" + - name: extract the Coverity Build Tool + run: | + mkdir $RUNNER_TEMP/cov-analysis && + tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis + - name: build with cov-build + run: | + export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" && + cov-configure --gcc && + cov-build --dir cov-int make -j$(nproc) + - name: package the build + run: tar -czvf cov-int.tgz cov-int + - name: submit the build to Coverity Scan + run: | + curl \ + --fail \ + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ + --form email='${{ secrets.COVERITY_SCAN_EMAIL }}' \ + --form [email protected] \ + --form version='${{ github.sha }}' \ + "https://scan.coverity.com/builds?project=$COVERITY_PROJECT"
Signature · 1 line
gitgitgadget
PATCH v2 2/6 coverity: cache the Coverity Build Tool
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
It would add a 1GB+ download for every run, better cache it.
This is inspired by the GitHub Action vapier/coverity-scan-action, however, it uses the finer-grained restore/save method to be able to cache the Coverity Build Tool even if an unrelated step in the GitHub workflow fails later on.
--- .github/workflows/coverity.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
.github/workflows/coverity.yml +28 -0
@@ -29,7 +29,28 @@ jobs: env: runs_on_pool: ubuntu-latest + # The Coverity site says the tool is usually updated twice yearly, so the + # MD5 of download can be used to determine whether there's been an update. + - name: get the Coverity Build Tool hash + id: lookup + run: | + MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ + --fail \ + --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ + --form project="$COVERITY_PROJECT" \ + --form md5=1) && + echo "hash=$MD5" >>$GITHUB_OUTPUT + + # Try to cache the tool to avoid downloading 1GB+ on every run. + # A cache miss will add ~30s to create, but a cache hit will save minutes. + - name: restore the Coverity Build Tool + id: cache + uses: actions/cache/restore@v3 + with: + path: ${{ runner.temp }}/cov-analysis + key: cov-build-${{ env.COVERITY_LANGUAGE }}-${{ env.COVERITY_PLATFORM }}-${{ steps.lookup.outputs.hash }} - name: download the Coverity Build Tool (${{ env.COVERITY_LANGUAGE }} / ${{ env.COVERITY_PLATFORM}}) + if: steps.cache.outputs.cache-hit != 'true' run: | curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --fail --no-progress-meter \
@@ -37,9 +58,16 @@ jobs: --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ --form project="$COVERITY_PROJECT" - name: extract the Coverity Build Tool + if: steps.cache.outputs.cache-hit != 'true' run: | mkdir $RUNNER_TEMP/cov-analysis && tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis + - name: cache the Coverity Build Tool + if: steps.cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v3 + with: + path: ${{ runner.temp }}/cov-analysis + key: cov-build-${{ env.COVERITY_LANGUAGE }}-${{ env.COVERITY_PLATFORM }}-${{ steps.lookup.outputs.hash }} - name: build with cov-build run: | export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" &&
Signature · 1 line
gitgitgadget
PATCH v2 3/6 coverity: allow overriding the Coverity project
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
By default, the builds are submitted to the git project at
https://scan.coverity.com/projects/git.
The Git for Windows project would like to use this workflow, too, though, and needs the builds to be submitted to the git-for-windows Coverity project.
To that end, allow configuring the Coverity project name via the repository variable, you guessed it, COVERITY_PROJECT. The default if that variable is not configured or has an empty value is still git.
--- .github/workflows/coverity.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
.github/workflows/coverity.yml +4 -1
@@ -11,6 +11,9 @@ name: Coverity # `COVERITY_SCAN_EMAIL` and `COVERITY_SCAN_TOKEN`. The former specifies the # email to which the Coverity reports should be sent and the latter can be # obtained from the Project Settings tab of the Coverity project). +# +# By default, the builds are submitted to the Coverity project `git`. To override this, +# set the repository variable `COVERITY_PROJECT`. on: push:
@@ -20,7 +23,7 @@ jobs: if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) runs-on: ubuntu-latest env: - COVERITY_PROJECT: git + COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }} COVERITY_LANGUAGE: cxx COVERITY_PLATFORM: linux64 steps:
Signature · 1 line
gitgitgadget
PATCH v2 4/6 coverity: support building on Windows
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
By adding the repository variable ENABLE_COVERITY_SCAN_ON_OS with a value, say, ["windows-latest"], this GitHub workflow now runs on Windows, allowing to analyze Windows-specific issues.
This allows, say, the Git for Windows fork to submit Windows builds to Coverity Scan instead of Linux builds.
--- .github/workflows/coverity.yml | 57 ++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 6 deletions(-)
.github/workflows/coverity.yml +51 -6
@@ -12,31 +12,62 @@ name: Coverity # email to which the Coverity reports should be sent and the latter can be # obtained from the Project Settings tab of the Coverity project). # +# The workflow runs on `ubuntu-latest` by default. This can be overridden by setting +# the repository variable `ENABLE_COVERITY_SCAN_ON_OS` to a JSON string array specifying +# the operating systems, e.g. `["ubuntu-latest", "windows-latest"]`. +# # By default, the builds are submitted to the Coverity project `git`. To override this, # set the repository variable `COVERITY_PROJECT`. on: push: +defaults: + run: + shell: bash + jobs: coverity: if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name) - runs-on: ubuntu-latest + strategy: + matrix: + os: ${{ fromJSON(vars.ENABLE_COVERITY_SCAN_ON_OS || '["ubuntu-latest"]') }} + runs-on: ${{ matrix.os }} env: COVERITY_PROJECT: ${{ vars.COVERITY_PROJECT || 'git' }} COVERITY_LANGUAGE: cxx - COVERITY_PLATFORM: linux64 + COVERITY_PLATFORM: overridden-below steps: - uses: actions/checkout@v3 + - name: install minimal Git for Windows SDK + if: contains(matrix.os, 'windows') + uses: git-for-windows/setup-git-for-windows-sdk@v1 - run: ci/install-dependencies.sh + if: contains(matrix.os, 'ubuntu') env: - runs_on_pool: ubuntu-latest + runs_on_pool: ${{ matrix.os }} # The Coverity site says the tool is usually updated twice yearly, so the # MD5 of download can be used to determine whether there's been an update. - name: get the Coverity Build Tool hash id: lookup run: | + case "${{ matrix.os }}" in + *windows*) + COVERITY_PLATFORM=win64 + COVERITY_TOOL_FILENAME=cov-analysis.zip + ;; + *ubuntu*) + COVERITY_PLATFORM=linux64 + COVERITY_TOOL_FILENAME=cov-analysis.tgz + ;; + *) + echo '::error::unhandled OS ${{ matrix.os }}' >&2 + exit 1 + ;; + esac + echo "COVERITY_PLATFORM=$COVERITY_PLATFORM" >>$GITHUB_ENV + echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --fail \ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
@@ -57,14 +88,28 @@ jobs: run: | curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --fail --no-progress-meter \ - --output $RUNNER_TEMP/cov-analysis.tgz \ + --output $RUNNER_TEMP/$COVERITY_TOOL_FILENAME \ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ --form project="$COVERITY_PROJECT" - name: extract the Coverity Build Tool if: steps.cache.outputs.cache-hit != 'true' run: | - mkdir $RUNNER_TEMP/cov-analysis && - tar -xzf $RUNNER_TEMP/cov-analysis.tgz --strip 1 -C $RUNNER_TEMP/cov-analysis + case "$COVERITY_TOOL_FILENAME" in + *.tgz) + mkdir $RUNNER_TEMP/cov-analysis && + tar -xzf $RUNNER_TEMP/$COVERITY_TOOL_FILENAME --strip 1 -C $RUNNER_TEMP/cov-analysis + ;; + *.zip) + cd $RUNNER_TEMP && + mkdir cov-analysis-tmp && + unzip -d cov-analysis-tmp $COVERITY_TOOL_FILENAME && + mv cov-analysis-tmp/* cov-analysis + ;; + *) + echo "::error::unhandled archive type: $COVERITY_TOOL_FILENAME" >&2 + exit 1 + ;; + esac - name: cache the Coverity Build Tool if: steps.cache.outputs.cache-hit != 'true' uses: actions/cache/save@v3
Signature · 1 line
gitgitgadget
PATCH v2 5/6 coverity: allow running on macOS
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
For completeness' sake, let's add support for submitting macOS builds to Coverity Scan.
--- .github/workflows/coverity.yml | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-)
.github/workflows/coverity.yml +20 -2
@@ -43,7 +43,7 @@ jobs: if: contains(matrix.os, 'windows') uses: git-for-windows/setup-git-for-windows-sdk@v1 - run: ci/install-dependencies.sh - if: contains(matrix.os, 'ubuntu') + if: contains(matrix.os, 'ubuntu') || contains(matrix.os, 'macos') env: runs_on_pool: ${{ matrix.os }}
@@ -56,10 +56,17 @@ jobs: *windows*) COVERITY_PLATFORM=win64 COVERITY_TOOL_FILENAME=cov-analysis.zip + MAKEFLAGS=-j$(nproc) + ;; + *macos*) + COVERITY_PLATFORM=macOSX + COVERITY_TOOL_FILENAME=cov-analysis.dmg + MAKEFLAGS=-j$(sysctl -n hw.physicalcpu) ;; *ubuntu*) COVERITY_PLATFORM=linux64 COVERITY_TOOL_FILENAME=cov-analysis.tgz + MAKEFLAGS=-j$(nproc) ;; *) echo '::error::unhandled OS ${{ matrix.os }}' >&2
@@ -68,6 +75,7 @@ jobs: esac echo "COVERITY_PLATFORM=$COVERITY_PLATFORM" >>$GITHUB_ENV echo "COVERITY_TOOL_FILENAME=$COVERITY_TOOL_FILENAME" >>$GITHUB_ENV + echo "MAKEFLAGS=$MAKEFLAGS" >>$GITHUB_ENV MD5=$(curl https://scan.coverity.com/download/$COVERITY_LANGUAGE/$COVERITY_PLATFORM \ --fail \ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
@@ -99,6 +107,16 @@ jobs: mkdir $RUNNER_TEMP/cov-analysis && tar -xzf $RUNNER_TEMP/$COVERITY_TOOL_FILENAME --strip 1 -C $RUNNER_TEMP/cov-analysis ;; + *.dmg) + cd $RUNNER_TEMP && + attach="$(hdiutil attach $COVERITY_TOOL_FILENAME)" && + volume="$(echo "$attach" | cut -f 3 | grep /Volumes/)" && + mkdir cov-analysis && + cd cov-analysis && + sh "$volume"/cov-analysis-macosx-*.sh && + ls -l && + hdiutil detach "$volume" + ;; *.zip) cd $RUNNER_TEMP && mkdir cov-analysis-tmp &&
@@ -120,7 +138,7 @@ jobs: run: | export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" && cov-configure --gcc && - cov-build --dir cov-int make -j$(nproc) + cov-build --dir cov-int make - name: package the build run: tar -czvf cov-int.tgz cov-int - name: submit the build to Coverity Scan
Signature · 1 line
gitgitgadget
PATCH v2 6/6 coverity: detect and report when the token or project is incorrect
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When trying to obtain the MD5 of the Coverity Scan Tool (in order to decide whether a cached version can be used or a new version has to be downloaded), it is possible to get a 401 (Authorization required) due to either an incorrect token, or even more likely due to an incorrect Coverity project name.
Seeing an authorization failure that is caused by an incorrect project name was somewhat surprising to me when developing the Coverity workflow, as I found such a failure suggestive of an incorrect token instead.
So let's provide a helpful error message about that specifically when encountering authentication issues.
--- .github/workflows/coverity.yml | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
.github/workflows/coverity.yml +12 -1
@@ -80,7 +80,18 @@ jobs: --fail \ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \ --form project="$COVERITY_PROJECT" \ - --form md5=1) && + --form md5=1) + case $? in + 0) ;; # okay + *22*) # 40x, i.e. access denied + echo "::error::incorrect token or project?" >&2 + exit 1 + ;; + *) # other error + echo "::error::Failed to retrieve MD5" >&2 + exit 1 + ;; + esac echo "hash=$MD5" >>$GITHUB_OUTPUT # Try to cache the tool to avoid downloading 1GB+ on every run.
Signature · 1 line
gitgitgadget
Quoted 8 lines · Johannes Schindelin via GitGitGadget
Changes since v1:
* After verifying that cURL's --fail option does what we need in Coverity's context, I switched to using that in every curl invocation.
* Adjusted quoting (the ${{ ... }} constructs do not require double quotes because they are interpolated before the script is run).
* Touched up a few commit messages, based on the feedback I received.
* Addressed an actionlint [https://rhysd.github.io/actionlint/] warning.
Thanks, this looks fine to me.
The only other comment is the same one I made for Taylor's version: that COVERITY_SCAN_EMAIL could probably be a "var" and not a "secret". Though the main advantage there (besides it being a little easier for the user to edit in the web UI) is that it could be used in the jobs.*.if context (to skip the job in unconfigured repos). But since your "if" uses a default-disallow when ENABLE_COVERITY_SCAN_FOR_BRANCHES is not set, it is not that important to check COVERITY_SCAN_EMAIL.
-Peff
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 6 lines · writes:
Coverity [https://scan.coverity.com/] is a powerful static analysis tool that helps prevent vulnerabilities. It is free to use by open source projects, and Git benefits from this, as well as Git for Windows. As is the case with many powerful tools, using Coverity comes with its own set of challenges, one of which being that submitting a build is quite laborious.
...
One thing that caught my eye was the asterisks around "22" that look as if they were designed to confuse readers and cause them wonder if there are other codes like 122 and 224 that we would also want to catch there. Unless they know what the case statement replaced, that is---the old code to match http_code that was scraped from a text file may not have the code alone and may contain other cruft, so it is entirely understandable, but the new one checks $? and there is no reason other than to catch 122 and 224 to use *22*.
Quoted 14 lines
-+ http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)" -+ case "$http_code" in -+ *200*) ;; # okay -+ *401*) # access denied-+ echo "::error::incorrect token or project? ($http_code)" >&2
+ --fail \
+ --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
+ --form project="$COVERITY_PROJECT" \
+- --form md5=1) &&
++ --form md5=1)
++ case $? in
++ 0) ;; # okay
++ *22*) # 40x, i.e. access denied
++ echo "::error::incorrect token or project?" >&2
Quoted 8 lines
+ exit 1
+ ;;
+ *) # other error
-+ echo "::error::HTTP error $http_code" >&2
++ echo "::error::Failed to retrieve MD5" >&2
+ exit 1
+ ;;
+ esac
Other than that, while I was watching from the sideline, I am very happy to see that you, with Peff's constructive input, came up with a new iteration that looks simpler and more consistent in its use of curl.
Will replace but I may be tempted to edit those asterisks out myself while queueing.
Thanks.
Hi Junio,
Quoted 14 lines · Junio C Hamano
One thing that caught my eye was the asterisks around "22" that look as if they were designed to confuse readers [...]
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 7 lines · writes:
[...]
++ case $? in ++ 0) ;; # okay ++ *22*) # 40x, i.e. access denied ++ echo "::error::incorrect token or project?" >&2 + exit 1 + ;;
Quoted 4 lines
[...]
Will replace but I may be tempted to edit those asterisks out myself while queueing.
D'oh, of course. Thank you,
Johannes