On Mon, Jul 04, 2022 at 04:46:53PM +0100, Peter Maydell wrote: > On Mon, 4 Jul 2022 at 16:23, Daniel P. Berrangé <[email protected]> wrote: > > > > Historically QEMU has used the 'scripts/checkpatch.pl' script to > > validate various style rules but there are a number of issues: > > > meson.build | 3 + > > tests/Makefile.include | 3 +- > > tests/meson.build | 19 +++ > > tests/style-excludes.mak | 4 + > > tests/style-infra.mak | 265 +++++++++++++++++++++++++++++++++++++++ > > tests/style.mak | 24 ++++ > > From my point of view the main issue with checkpatch.pl is > that nobody in the QEMU developers particularly understands > it or is enthusiastic about trying to add more tests to it > or adjust the existing ones where QEMU style diverges from > the kernel style (but nor are we tracking and upgrading to > newer versions of the kernel's script). > > This seems to be aiming to replace a complex and hard to > understand perl script with a complex and hard to understand > makefile. I can't say I'm terribly enthusiastic :-/
I think the downsides comapred here are rather different orders of magnitude. The checkpatch.pl script is 3000 lines of code where we have years of experiance that no one in QEMU likes touching it. The makefile here is 265 lines of which 50% is comments of license text. In terms of what contributors most care about though, is how you add new rules, and most of the time that's involves just adding a 3 line make rule based off a regex to match the code pattern you want to prohibit. Some of this is a bit crufty to look at, but we've got years of experiance in libvirt with many contributors frequently adding new tests. It only gets hairy if the pattern you're trying to forbid needs to match across multiple lines of text - hence the difference in complexity between matching 'osdep.h' exists in .c, vs 'osdep.h' exists as the very first #include. IME, the single-line matches are most typical need that is addressed. So while I wont claim this proposal is perfect, IMHO this would be a significant step fowards over checkpatch.pl. With 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 :|
