Skip to content

Fix Coverity builds on Windows #1934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix Coverity builds on Windows #1934

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
712602c
ci(coverity): fix building on Windows
dscho Jun 11, 2025
52c3497
ci(coverity): output the build log upon error
dscho Jun 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .github/workflows/coverity.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,13 @@ jobs:
key: cov-build-${{ env.COVERITY_LANGUAGE }}-${{ env.COVERITY_PLATFORM }}-${{ steps.lookup.outputs.hash }}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> ...
> In the meantime, the current Coverity documentation describes a very
> different way to install the analysis tool, recommending to add the
> `bin/` directory to the _end_ of `PATH` (when originally, IIRC, it was
> recommended to add it to the _beginning_ of the `PATH`).
> ..., and finding the "wrong" ones first on the
> `PATH` misleads that logic.
>
> Let's fix this problem by following Coverity's current recommendation
> and append the `bin/` directory in which `cov-int` can be found to the
> _end_ of `PATH`.

Wow, that is a very well described change.

> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>  .github/workflows/coverity.yml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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?

Other than that, nicely done and very nicely explained.



>            cov-configure --gcc &&
>            cov-build --dir cov-int make
>        - name: package the build

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Johannes Schindelin wrote (reply to this):

Hi Junio,

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

> "Johannes Schindelin via GitGitGadget" <[email protected]>
> 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

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Johannes Schindelin <[email protected]> writes:

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

Great.  Thanks.  Will replace with v2.

- name: build with cov-build
run: |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Johannes Schindelin via GitGitGadget" <[email protected]>
writes:

> From: Johannes Schindelin <[email protected]>
>
> It is quite helpful to know what Coverity said, exactly, in case it
> fails to analyze the code.
>
> Signed-off-by: Johannes Schindelin <[email protected]>
> ---
>  .github/workflows/coverity.yml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Wonderful.  Will queue, together with 1/2.  Thanks.

>
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> index a5d99e59d4eb..1e8bd85ecd4e 100644
> --- a/.github/workflows/coverity.yml
> +++ b/.github/workflows/coverity.yml
> @@ -149,7 +149,11 @@ jobs:
>          run: |
>            export PATH="$PATH:$(cygpath -au "$RUNNER_TEMP")/cov-analysis/bin" &&
>            cov-configure --gcc &&
> -          cov-build --dir cov-int make
> +          if ! cov-build --dir cov-int make
> +          then
> +            cat cov-int/build-log.txt
> +            exit 1
> +          fi
>        - name: package the build
>          run: tar -czvf cov-int.tgz cov-int
>        - name: submit the build to Coverity Scan

export PATH="$RUNNER_TEMP/cov-analysis/bin:$PATH" &&
export PATH="$PATH:$RUNNER_TEMP/cov-analysis/bin" &&
cov-configure --gcc &&
cov-build --dir cov-int make
if ! cov-build --dir cov-int make
then
cat cov-int/build-log.txt
exit 1
fi
- name: package the build
run: tar -czvf cov-int.tgz cov-int
- name: submit the build to Coverity Scan
Expand Down
Loading