On Fri, Jan 15, 2021 at 03:09:25PM +0100, Thomas Monjalon wrote: > 15/01/2021 12:59, Bruce Richardson: > > On Fri, Jan 15, 2021 at 11:51:49AM +0000, Ferruh Yigit wrote: > > > On 1/15/2021 11:10 AM, Bruce Richardson wrote: > > > > To verify that all DPDK headers are ok for inclusion directly in a C > > > > file, and are not missing any other pre-requisite headers, we can > > > > auto-generate for each header an empty C file that includes that header. > > > > Compiling these files will throw errors if any header has unmet > > > > dependencies. > > > > > > > > The list of headers to check is based of the "headers" value returned > > > > from > > > > each library's meson.build file. However, since not all headers are for > > > > direct inclusion, add a build variable "headers_no_chkincs" to list > > > > those > > > > headers and skip checking them. > > > > > > > > Signed-off-by: Bruce Richardson <bruce.richard...@intel.com> > > > > --- > > > > > > > > v2: > > > > * add maintainers entry > > > > * distribute exception list among meson.build files. > > > > > > > > MAINTAINERS | 4 ++++ > > > > app/chkincs/gen_c_file_for_header.py | 12 ++++++++++ > > > > app/chkincs/main.c | 4 ++++ > > > > app/chkincs/meson.build | 28 ++++++++++++++++++++++++ > > > > > > +1 to have this kind of tool to check, but it is not an application like > > > others in the 'app' folder, what do you think placing it under 'devtools' > > > or > > > 'buildtools'? > > > > Couple of reasons why it's placed in app. > > > > 1. We previously had a "chkincs" app in DPDK which was kept in the app > > folder > > 2. It allows us to reuse the build infrastructure for building apps, rather > > than reduplicating it. > > 3. We don't have any compilable code currently in the devtools folder, and > > even in buildtools the pmdinfogen app is going to go away. > > > > That being said, none of those reasons are major issues that can't be > > worked around if the consensus is to move it. > > It could be easily in devtools if it was a script. > By the way, we already have devtools/check-includes.sh > If your solution is better, please remove this script. > I only discovered the script existed when doing the v2 of this patchset, since it showed up in some grep calls I did for exception cases. I'm not sure that either approach is necessarily better, it's just right now that the script is unused (and also unknown) which is why I did this cleanup work.
Here is how I see the current comparison between two approaches: * Script as advantage in that it performs C++ checks as well as C * Script also allows passing arbitrary additional C flags into checks for higher levels of compliance, but I'm not sure this is something I like as I'd rather have standardisation here across all headers than have some headers more pedantic-friendly than others. * Main downside of the script is that is works off directories rather than a list of files, which means it requires maintenance of the exception list in the script, rather than in the build definition files where we call out the headers to be installed I'm honestly fine either way on this (as with directory where implementation lives) - main thing is to have the checking done, rather than ignored. /Bruce
