[PATCH 0/6] Add a GitHub workflow to submit builds to Coverity Scan

[PATCH 0/6] Add a GitHub workflow to submit builds to Coverity Scan

[Johannes Schindelin via GitGitGadget]

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
-- 
gitgitgadget

[PATCH 1/6] ci: add a GitHub workflow to submit Coverity scans

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Initial-patch-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 56 ++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 .github/workflows/coverity.yml

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
new file mode 100644
index 00000000000..24408f6282c
--- /dev/null
+++ b/.github/workflows/coverity.yml
@@ -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 file=@cov-int.tgz \
+            --form version="${{ github.sha }}" \
+            "https://scan.coverity.com/builds?project=$COVERITY_PROJECT"
-- 
gitgitgadget

[PATCH 2/6] coverity: cache the Coverity Build Tool

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 24408f6282c..e8d0be52702 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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" &&
-- 
gitgitgadget

[PATCH 3/6] coverity: allow overriding the Coverity project

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index e8d0be52702..8aac00bb20f 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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:
-- 
gitgitgadget

[PATCH 4/6] coverity: support building on Windows

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 56 ++++++++++++++++++++++++++++++----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 8aac00bb20f..70ba3f97c18 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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
-- 
gitgitgadget

[PATCH 5/6] coverity: allow running on macOS

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

For completeness' sake, let's add support for submitting macOS builds to
Coverity Scan.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 70ba3f97c18..ca51048ed9d 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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
-- 
gitgitgadget

[PATCH 6/6] coverity: detect and report when the token or project is incorrect

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index ca51048ed9d..12cdbaf7ffd 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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.
-- 
gitgitgadget

Re: [PATCH 1/6] ci: add a GitHub workflow to submit Coverity scans

[Jeff King]

On Fri, Sep 22, 2023 at 10:41:58AM +0000, Johannes Schindelin via GitGitGadget wrote:

> 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.

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.

> +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).

> +    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.

> +      - 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. :)

> +      - 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).

> +      - name: submit the build to Coverity Scan
> +        run: |
> +          curl \
> +            --form token="${{ secrets.COVERITY_SCAN_TOKEN }}" \
> +            --form email="${{ secrets.COVERITY_SCAN_EMAIL }}" \
> +            --form file=@cov-int.tgz \
> +            --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 2/6] coverity: cache the Coverity Build Tool

[Jeff King]

On Fri, Sep 22, 2023 at 10:41:59AM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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.

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:

> +      # 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 3/6] coverity: allow overriding the Coverity project

[Jeff King]

On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote:

> +# 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 4/6] coverity: support building on Windows

[Jeff King]

On Fri, Sep 22, 2023 at 10:42:01AM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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.

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.

> +# 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

Re: [PATCH 5/6] coverity: allow running on macOS

[Jeff King]

On Fri, Sep 22, 2023 at 10:42:02AM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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 6/6] coverity: detect and report when the token or project is incorrect

[Jeff King]

On Fri, Sep 22, 2023 at 10:42:03AM +0000, Johannes Schindelin via GitGitGadget wrote:

> From: Johannes Schindelin <johannes.schindelin@gmx.de>
> 
> 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

[PATCH v2 0/6] Add a GitHub workflow to submit builds to Coverity Scan

[Johannes Schindelin via GitGitGadget]

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 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 file=@cov-int.tgz \
     -+            --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.
     +
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       ## .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.
      
          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
      
       ## .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

-- 
gitgitgadget

[PATCH v2 1/6] ci: add a GitHub workflow to submit Coverity scans

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Initial-patch-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 58 ++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 .github/workflows/coverity.yml

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
new file mode 100644
index 00000000000..d8d1e328578
--- /dev/null
+++ b/.github/workflows/coverity.yml
@@ -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 file=@cov-int.tgz \
+            --form version='${{ github.sha }}' \
+            "https://scan.coverity.com/builds?project=$COVERITY_PROJECT"
-- 
gitgitgadget

[PATCH v2 2/6] coverity: cache the Coverity Build Tool

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index d8d1e328578..4bc1572f040 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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" &&
-- 
gitgitgadget

[PATCH v2 3/6] coverity: allow overriding the Coverity project

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 4bc1572f040..55a3a8f5acf 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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:
-- 
gitgitgadget

[PATCH v2 4/6] coverity: support building on Windows

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 57 ++++++++++++++++++++++++++++++----
 1 file changed, 51 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 55a3a8f5acf..ca364c3d692 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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
-- 
gitgitgadget

[PATCH v2 5/6] coverity: allow running on macOS

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

For completeness' sake, let's add support for submitting macOS builds to
Coverity Scan.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index ca364c3d692..53f9ee6a418 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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
-- 
gitgitgadget

[PATCH v2 6/6] coverity: detect and report when the token or project is incorrect

[Johannes Schindelin via GitGitGadget]

From: Johannes Schindelin <johannes.schindelin@gmx.de>

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.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 .github/workflows/coverity.yml | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 53f9ee6a418..ae76c06e7ce 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -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.
-- 
gitgitgadget

Re: [PATCH 1/6] ci: add a GitHub workflow to submit Coverity scans

[Johannes Schindelin]

Hi Peff,

On Sat, 23 Sep 2023, Jeff King wrote:

> On Fri, Sep 22, 2023 at 10:41:58AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > 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.
>
> 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.

> > +      - 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?

> 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

> 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 2/6] coverity: cache the Coverity Build Tool

[Johannes Schindelin]

Hi Peff,

On Sat, 23 Sep 2023, Jeff King wrote:

> On Fri, Sep 22, 2023 at 10:41:59AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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.
>
> 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.

>
> One note:
>
> > +      # 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.

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

Re: [PATCH 3/6] coverity: allow overriding the Coverity project

[Johannes Schindelin]

Hi Peff,

On Sat, 23 Sep 2023, Jeff King wrote:

> On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > +# 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 5/6] coverity: allow running on macOS

[Johannes Schindelin]

Hi Peff,

On Sat, 23 Sep 2023, Jeff King wrote:

> On Fri, Sep 22, 2023 at 10:42:02AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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.

> 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 6/6] coverity: detect and report when the token or project is incorrect

[Johannes Schindelin]

Hi Jeff,

On Sat, 23 Sep 2023, Jeff King wrote:

> On Fri, Sep 22, 2023 at 10:42:03AM +0000, Johannes Schindelin via GitGitGadget wrote:
>
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > 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.

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 1/6] ci: add a GitHub workflow to submit Coverity scans

[Jeff King]

On Mon, Sep 25, 2023 at 01:52:34PM +0200, Johannes Schindelin wrote:

> > 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.

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.

> 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?

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.

> > 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.

That seems like good reasoning to me.

-Peff

Re: [PATCH 3/6] coverity: allow overriding the Coverity project

[Jeff King]

On Mon, Sep 25, 2023 at 01:52:47PM +0200, Johannes Schindelin wrote:

> Hi Peff,
> 
> On Sat, 23 Sep 2023, Jeff King wrote:
> 
> > On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote:
> >
> > > +# 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.

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").

> 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?

I understood your intention. I think you misunderstood mine. :) The
commit message is fine as-is, I think.

-Peff

Re: [PATCH 5/6] coverity: allow running on macOS

[Jeff King]

On Mon, Sep 25, 2023 at 01:52:52PM +0200, Johannes Schindelin wrote:

> > > 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.

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

Re: [PATCH 6/6] coverity: detect and report when the token or project is incorrect

[Jeff King]

On Mon, Sep 25, 2023 at 01:52:56PM +0200, Johannes Schindelin wrote:

> > > 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).

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

Re: [PATCH v2 0/6] Add a GitHub workflow to submit builds to Coverity Scan

[Jeff King]

On Mon, Sep 25, 2023 at 11:50:56AM +0000, Johannes Schindelin via GitGitGadget wrote:

> 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

Re: [PATCH v2 0/6] Add a GitHub workflow to submit builds to Coverity Scan

[Junio C Hamano]

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
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*.



>      -+          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

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.

Re: [PATCH v2 0/6] Add a GitHub workflow to submit builds to Coverity Scan

[Johannes Schindelin]

Hi Junio,

On Mon, 25 Sep 2023, Junio C Hamano wrote:

> 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" <gitgitgadget@gmail.com>
> writes:
>
> > [...]
> >      ++          case $? in
> >      ++          0) ;; # okay
> >      ++          *22*) # 40x, i.e. access denied
> >      ++            echo "::error::incorrect token or project?" >&2
> >       +            exit 1
> >       +            ;;
>
> [...]
>
> Will replace but I may be tempted to edit those asterisks out myself
> while queueing.

D'oh, of course. Thank you,
Johannes

Re: [PATCH 3/6] coverity: allow overriding the Coverity project

[Johannes Schindelin]

Hi Jeff,

On Mon, 25 Sep 2023, Jeff King wrote:

> On Mon, Sep 25, 2023 at 01:52:47PM +0200, Johannes Schindelin wrote:
>
> > On Sat, 23 Sep 2023, Jeff King wrote:
> >
> > > On Fri, Sep 22, 2023 at 10:42:00AM +0000, Johannes Schindelin via GitGitGadget wrote:
> > >
> > > > +# 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.
>
> 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.

> 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

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> 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

[Jeff King]

On Tue, Sep 26, 2023 at 07:19:46AM -0700, Junio C Hamano wrote:

> 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

[Jeff King]

On Tue, Sep 26, 2023 at 04:02:27PM +0200, Johannes Schindelin wrote:

> > > > 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.
> >
> > 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.

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

Re: [PATCH 3/6] coverity: allow overriding the Coverity project

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Sep 26, 2023 at 07:19:46AM -0700, Junio C Hamano wrote:
>
>> 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).

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.