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
