Skip to content
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.

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

Jenkins is not reporting cppcheck failures #446

Open
scpeters opened this issue Apr 15, 2021 · 5 comments
Open

Jenkins is not reporting cppcheck failures #446

scpeters opened this issue Apr 15, 2021 · 5 comments

Comments

@scpeters
Copy link
Contributor

The Ubuntu jenkins builds are configured in dsl to report cppcheck errors by default, and I can see the xml files generated in the workspace of several jobs that includes some errors, but these are not reported by jenkins in the summary of that job. For example, the following job has errors in the cpplint.xml but does not report these error in the job summary, and is marked as a clean build:

@chapulina
Copy link
Contributor

One thing to keep in mind is that we're also running cppcheck on GitHub actions, and if the cppcheck version is different between actions and Jenkins, it may be impossible to satisfy both at the same time.

So I think we should consider only running cppcheck in one place. The downside of actions is that the reports are hard to read. The downsides of Jenkins are this issue and also #148.

@scpeters
Copy link
Contributor Author

I used ign-physics as an example because it has a relatively small number of errors, but this affects osrf/gazebo, which doesn't use GitHub actions. If we don't want to run the code check in jenkins, then we should explicitly disable it in dsl for ignition and sdformat using the enable_cppcheck parameter to OSRFLinuxCompilationAnyGitHub. I'd appreciate it if we could fix this bug for gazebo though.

@chapulina
Copy link
Contributor

I just noticed this again. It's confusing to have code_check.sh while it doesn't match make codecheck. We can only enforce one of them at a time, so I think having both does more harm than good.

How about we only run it if the file exists? This would allow us to remove it from Ignition while maintaining it on classic.

The downside of actions is that the reports are hard to read.

Come to think of it, I don't think this is a big deal, because it's not like we have flaky warnings whose history we want to check. We've been keeping the builds free of static warnings since that turns Actions red, so I think that just reading the logs has been working out.

@scpeters
Copy link
Contributor Author

scpeters commented Apr 29, 2021

How about we only run it if the file exists? This would allow us to remove it from Ignition while maintaining it on classic.

sure, let's update the logic and delete code_check.sh for packages that use ign-cmake for configuration; that works for me

@chapulina
Copy link
Contributor

chapulina commented Apr 29, 2021

I opened gazebosim/gz-math#211 to test removing it and #455 to update the logic.

I'll wait for the Ubuntu build on the ign-math PR to fail first (as I'd expect), and then trigger a build using the release-tools PR. Oh I see @scpeters already triggered one: Build Status https://build.osrfoundation.org/job/ignition_math-ci-main-bionic-amd64/4/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants