Re: [libvirt PATCH 00/10] [RFC] clang-tidy CI integration
On Fri, Feb 12, 2021 at 02:25:24PM +0100, Tim Wiederhake wrote:
"clang-tidy" is a static code analysis tool for c and c++
code. See
https://listman.redhat.com/archives/libvir-list/2021-January/msg01152.html
for some issues found by clang-tidy and more background information.
Meson has support for clang-tidy and generates target named "clang-tidy" if
it finds a ".clang-tidy" configuration file in the project's root
directory.
There are some problems with this approach though, with regards to inclusion
in GitLab's CI:
* Single-threaded runtime of a complete scan takes between 95 and 110 minutes,
depending on the enabled checks, which is significantly longer than GitLab's
maximum run time of 60 minutes, after which jobs are aborted.
IIUC, you can override the timeout setting per job...
* Even without this limit on runtime, this new check would double to triple
the run time of the libVirt pipeline in GitLab.
Yeah that's a problem, but bear in mind there are two separate scenarios
Running post merge we need to check all files, but the length of time
is a non-issue since no one is waiting for a post-merge check to
complete.
Running pre- merge we only need to check files which are modified by
the patches on the current branch. That ought to be orders of magnitude
faster.
* clang-tidy finds a lot of false positives (see link above for explanation)
and has checks that contradict the libVirt code style (e.g. braces around
blocks). This makes a quite complex configuration in ".clang-tidy" neccessary.
IMHO for code checkers to add real value to people you need the existing
codebase in git to be 100% clean.
If you're not going to make the existing code compliant, then users will
never be sure of what they should do for new code. We see this all the
time in QEMU where its patch checker complains about problems that were
pre-existing.
If we could get clang-tidy to do a subset of checks where we can malke
libvirt 100% clean
* I was unable to make clang-tidy recognize the settings from the
configuration file for generated files, leading clang-tidy to always add some
checks. These checks were among those that produced false positives.
* The list of enabled / disabled checks in the yaml configuration file is a
quite long string, making it hard to weave in some comments / documentation
on which checks are enabled / disabled for what reason.
This series introduces a new script, "run-clang-tidy.py". This is a
replacement for the script of the same name from clang-tools-extra. It offers
parallel execution, caching of results and a configurable soft-timeout.
Please see the individual commits for more details. Comments welcome.
https://gitlab.com/twiederh/libvirt/-/pipelines/255321968 → "clang-tidy".
What am I missing here - that job just seems to show a list of
filenames, but isn't reporting any issues for them ?
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|