Re: [PATCH 1/2] ci(coverity): fix building on Windows

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jun 11, 2025, 15:53 UTC
Message-ID: <8d71bc71-3e5b-c59c-8948-c07d7b4f62ac@gmx.de>
In-Reply-To: <xmqqtt4mjqz4.fsf@gitster.g>

Hi Junio,

On Wed, 11 Jun 2025, Junio C Hamano wrote:

Show 23 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> > index 124301dbbe2f..a5d99e59d4eb 100644
> > --- a/.github/workflows/coverity.yml
> > +++ b/.github/workflows/coverity.yml
> > @@ -147,7 +147,7 @@ jobs:
> >            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" &&
> > +          export PATH="$PATH:$(cygpath -au "$RUNNER_TEMP")/cov-analysis/bin" &&
> 
> Additionally two things are lacking explanation in the proposed log
> message, though, or an uninitiated will still be left scratching his
> head:
> 
>  - Why didn't the original need "cygpath -au"?
> 
>  - Even though many steps in this job deals with different
>    env.COVERITY_PLATFORM, this part does not seem to be conditional.
>    Why is $(cygpath -au ...) safe outside Windows environment?

I am delighted by your feedback which points out a functional problem. The `cygpath -au` is a left-over from some interactive debugging session where I _thought_ that `RUNNER_TEMP` contains a Windows path, and I wanted to make sure that it is a Unix-like path.

But yes, this is totally in a cross-platform part of the workflow (which is itself already guarded by that `if: contains(fromJSON(vars.ENABLE_COVERITY_SCAN_FOR_BRANCHES || '[""]'), github.ref_name)` condition so that it is skipped in forks that did not enable this workflow explicitly.

As such, that `cygpath` call is incorrect, as it would fail on anything by Windows. It is also unnecessary, as I just verified in a manual run. Therefore I drop it from v2.

Thank you, Johannes