Suggestion: remove cppcheck related stuff from build

Bug #988498 reported by Jens Beyer on 2012-04-25
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Undecided
Unassigned

Bug Description

Currently, cmake -DWL_EXTRAWARNINGS=true enables (among more compiler warnings) cppcheck in a manner like CodeCheck.py script - it calls cppcheck and caches the result if it is ok, so it doesn't run it again.

Unfortunately, the cppcheck people often change their command line parameters, so currently the built-in cppcheck functionality doesn't work anymore (again). Only option would be enabling all checks which would then generate lots of noise.

Now we still have utils/create_cppcheck_report tool... which creates a nice report and, even if run with --enable=all, doesn't look so noisy with current versions of cppcheck (see bug #986611 for details, thanks hjd). This more beautiful result can never be reached with the way the build utilizes cppcheck.

I suggest therefor to remove all cppcheck related stuff from the buildsystem, in favor of the utils/create_cppcheck_report which might instead invest some time in utils/create_cppcheck_report if that's necessary or helpful.

Changed in widelands:
status: Opinion → Incomplete

I agree. The utils script seems enough.

Hans Joachim Desserud (hjd) wrote :

Ok, I've looked back into this.

It is fairly easy to get the cppcheck working again in WL_EXTRAWARNINGS by changing to --enable=all rather than the currently assigned values.

That said, it is very noisy since it prints each issue each time it finds it for all files and it is slightly hard to separate compiler warnings from cppcheck issues in the output. (My approach for creating the warning logs has been to print everything written to stderr to a file). I think only a tiny minority ever looks at this, and given the noise level it is hardly of any use, so I doubt anyone would miss it.

utils/create_cppcheck_report on the other hand provides a rather nice report with each issue on less than 300 lines at the moment, which is a lot easier to relate to and look into. I therefore agree we should drop this and focus on the report script. Note that I have made some improvements to it, and I'll submit a patch shortly.

Oh, and Jens; regarding the false positive which we have been suppressing; it shows up with cppcheck 1.49, but I no longer get it when checking the file with version 1.54 so I assume they have fixed it.

Jens Beyer (qcumber-some) wrote :

Removed as of bzr6379

Changed in widelands:
status: Incomplete → Fix Committed
milestone: none → build18-rc1
SirVer (sirver) wrote :

Released in build-18 rc1.

Changed in widelands:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers