Flag existing and new compiler warnings with reviewdog

[NickSzapiro-NOAA]

Commit Queue Requirements:

• This PR addresses a relevant WM issue (if not, create an issue).
• All subcomponent pull requests (if any) have been reviewed by their code managers.
• Run the full Intel+GNU RT suite (compared to current baselines), preferably on Ursa (Derecho or Hercules are acceptable alternatives). Exceptions: documentation-only PRs, CI-only PRs, etc.
  • Commit log file w/full results from RT suite run (if applicable).
  • Verify that test_changes.list indicates which tests, if any, are changed by this PR. Commit test_changes.list, even if it is empty.
• Fill out all sections of this template.

---

Description:

We're looking for a friendlier way of iterating with developers to reduce compiler warnings, towards better code quality and EE2 requirements. Here is a go at this via reviewdog

https://github.com/reviewdog/reviewdog "provides a way to post review comments to code hosting services, such as GitHub, automatically by integrating with any linter tools with ease. It uses an output of lint tools and posts them as a comment if findings are in the diff of patches to review."

Here we add GitHub Actions to flag existing and new compiler warnings with reviewdog for s2swa application in GNU debug mode. These complement the tracking of the number of warnings on supported platforms via the regression test platform logs.

NickSzapiro-NOAA#24 is an example of what a developer may see

Commit Message:

* UFSWM - Flag existing and new compiler warnings with reviewdog GHActions

Priority:

• Normal

Git Tracking

UFSWM:

• Closes Compile warnings in UFS and subcomponents #2703

Sub component Pull Requests:

• None

UFSWM Blocking Dependencies:

• None

Documentation:

• Documentation update required.
  • Relevant updates are included with this PR.
  • A WM issue has been opened to track the need for a documentation update; a person responsible for submitting the update has been assigned to the issue (link issue).
• Documentation update NOT required.
  • Explanation:

---

Changes

Regression Test Changes (Please commit test_changes.list):

• No Baseline Changes.

Input data Changes:

• None.

Library Changes/Upgrades:

• No Updates as runs in CI

---

Testing Log:

• RDHPCS
  • Orion
  • Hercules
  • GaeaC6
  • Derecho
  • Ursa
• WCOSS2
  • Dogwood/Cactus
  • Acorn
• CI
• opnReqTest (complete task if unnecessary)

[BrianCurtis-NOAA]

There won't be any extra warning messages in this PR, so I assume this new check won't produce anything and pass. Is there any way to test this PR's work? Create a new warning manually and see what it does?

[dpsarmie]

This is neat. Looking at your example PR, it looks like the previous logs are already stored somewhere. Is this maintained by the EPIC Spackstack team? Also, we should think about how often this should be run if it takes ~20 minutes to complete. (I'm thinking not on every pull request, maybe a less frequent trigger, but 20 minutes also isn't too bad of a runtime.)

[NickSzapiro-NOAA]

Here is an example from adding an unused variable @BrianCurtis-NOAA : NickSzapiro-NOAA#24 (comment)

I'm not sure that previous logs are involved. Where do you see that @dpsarmie ?
The filter-mode=added filters results by added/modified lines ... reviewdog lints (here for compiler warnings) and overlaps with git patch line diffs to check a PR's changes.

That's not 100%, as code changes could trigger warnings elsewhere but we separately track the total number of warnings

[dpsarmie]

I was looking at the https://github.com/NickSzapiro-NOAA/ufs-weather-model/actions/runs/24421069485/job/71343114530?pr=24 logs for the Github action, but you're right there's no download of the previous logs. I was confused but now I see that the logs it downloads are from the S2SWA and ATM builds from the previous steps (looking at the SHA256 hashes).

So now my question is what is being compared to generate the report? I'm assuming there's a diff being run somewhere between a baseline and the new logs being generated or does reviewdog work some other way?

[NickSzapiro-NOAA]

Another way without a diff to a previous version. Currently this

• gets the build log from s2swa gnu debug Spack job
• cleans it into a temporary cleaned_warnings.txt, since reviewdog likes single lines
• pipes cleaned_warnings into 2 reviewdog runs:
  • -reporter=github-check to annotate all warnings
  • -reporter=github-pr-review -filter-mode=added to comment on PR for new added warnings in lines changed in PR

The Spack job is the time consuming part and the warnings wait on that (for the build logs)

[DeniseWorthen]

Are we sure this is a well-maintained GHA? It has ~130 open issues, the top one of which is "looking for maintainers" posted in 2019.

There are two new issues, one posted by @DavidHuber-NOAA last week and then closed.

[AlexanderRichert-NOAA]

As far as custom GitHub actions are concerned, I suggest pinning specific commit hashes and updating carefully. Here's an instance of exploits apparently being introduced into reviewdog: GHSA-qmg3-hpqr-gqvc
Denise's point about maintenance is important, not least because it's the older, less actively maintained stuff that's more vulnerable to malicious updates (modifications that could leak secrets like API keys, automatically create/modify/approve PRs to try to modify our code, etc).

[AlexanderRichert-NOAA]

@DeniseWorthen @dpsarmie @NickSzapiro-NOAA I've been looking at GitHub Actions-related repository security settings. I don't know how locked down they are currently but it might be worth reviewing them and minimizing permissions/features to whatever is strictly needed.

[NickSzapiro-NOAA]

I'm not an expert and welcome more reviews/feedback

reviewdog seems to be a widely used, popular open-source tool for automated code reviews, particularly within the GitHub ecosystem. But, I'm not sure this is the best tool , especially with a major security vulnerability in March 2025 (see https://www.wiz.io/blog/new-github-action-supply-chain-attack-reviewdog-action-setup) as Alex mentioned

Happy to try other approaches, if folks have suggestions

[NickSzapiro-NOAA]

ok ... I'm starting a new branch that will work similarly but using our own script with read permissions instead
https://github.com/NickSzapiro-NOAA/ufs-weather-model/tree/helper_warnings_script

@AlexanderRichert-NOAA Do you want to make a separate issue on reviewing the GitHub Actions-related repository security settings?

[AlexanderRichert-NOAA]

Sounds good, will do

[NickSzapiro-NOAA]

Closing in favor of #3200