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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 25, 2023, 17:20 UTC
Message-ID: <xmqqy1guuq3w.fsf@gitster.g>
In-Reply-To: <pull.1588.v2.git.1695642662.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines

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

Show 22 quoted lines

>      -+          http_code="$(sed -n 1p <"$RUNNER_TEMP"/headers.txt)"
>      -+          case "$http_code" in
>      -+          *200*) ;; # okay
>      -+          *401*) # access denied
>      -+            echo "::error::incorrect token or project? ($http_code)" >&2
>      +                    --fail \
>      +                    --form token='${{ secrets.COVERITY_SCAN_TOKEN }}' \
>      +                    --form project="$COVERITY_PROJECT" \
>      +-                   --form md5=1) &&
>      ++                   --form md5=1)
>      ++          case $? in
>      ++          0) ;; # okay
>      ++          *22*) # 40x, i.e. access denied
>      ++            echo "::error::incorrect token or project?" >&2
>       +            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.