Skip to content

Conversation

@j-rivero
Copy link
Contributor

🦟 Bug fix

Fixes #446

Summary

Cppcheck was returning exit code 0 even when errors were detected, causing CI to pass when it should fail. Adding --error-exitcode=1 ensures cppcheck returns non-zero when any errors are found.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Opus 4.5

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

Cppcheck was returning exit code 0 even when errors were detected,
causing CI to pass when it should fail. Adding --error-exitcode=1
ensures cppcheck returns non-zero when any errors are found.

Fixes #446

Signed-off-by: Jose Luis Rivero <[email protected]>
Generated-by: claude-opus-4-5-20251101
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Thanks for looking into this. I was really hopeing someone would find the time to fix this. However, before we merge/release, we should check that it doesn't break CI in downstream libraries.

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Jan 21, 2026
@j-rivero
Copy link
Contributor Author

j-rivero commented Feb 2, 2026

Running a long one here Build Status

@j-rivero
Copy link
Contributor Author

j-rivero commented Feb 3, 2026

Thanks for looking into this. I was really hopeing someone would find the time to fix this. However, before we merge/release, we should check that it doesn't break CI in downstream libraries.

Summary of downstream:

Looking into stable branches, the problems are being silently ignore https://github.com/gazebosim/sdformat/actions/runs/21492493963/job/61918436059

@j-rivero j-rivero marked this pull request as draft February 4, 2026 18:11
@j-rivero
Copy link
Contributor Author

j-rivero commented Feb 4, 2026

I have changed gears, we can not make all cppcheck in the first pass as mandatory, too noisy with fool warnings on standard headers. The second pass without any custom configuration seems fine to enable, making sdformat to fail with some macros that needs configuration:

nofile:0:0: information: Active checkers: There was critical errors (use --checkers-report=<filename> to see details) [checkersReport]

/home/jrivero/code/gz/codecheck_ws/src/sdformat/include/sdf/Element.hh:385:5: error: There is an unknown macro here somewhere. Configuration is required. If GZ_DEPRECATED is a macro then please configure it. [unknownMacro]
    GZ_DEPRECATED(16)
    ^
/home/jrivero/code/gz/codecheck_ws/src/sdformat/src/cmd/sdf_main.cc:137:5: error: There is an unknown macro here somewhere. Configuration is required. If SDF_PROTOCOL_VERSION is a macro then please configure it. [unknownMacro]
    SDF_PROTOCOL_VERSION ")")

I want to run this on all the libraries, still on it. In draft by now.

In my mind: we should use a modern tool that can replace the cppcheck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Cppcheck exits with code 0 (success) despite errors

2 participants