Re: [PATCH v5 8/9] buildtools/chkincs: use a staging directory for headers

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Sep 29, 2025 at 11:06:48AM +0200, David Marchand wrote:
> On Mon, 29 Sept 2025 at 10:16, Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Fri, Sep 26, 2025 at 02:41:01PM +0200, David Marchand wrote:
> > > A problem with the current headers check is that it relies on
> > > meson dependencies objects that come with their include_directories
> > > directives, and all of those point at the library / driver sources.
> > >
> > > This means that we won't detect a public header including a private
> > > (as in, not exported) header, or a driver only header.
> > >
> > > To address this issue, a staging directory is added and every header
> > > is copied to it.
> > >
> > > Drivers and library headers are staged to two different directories
> > > and the check is updated accordingly.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > > Changes since v4:
> > > - split global_inc (isolating config include path from EAL headers),
> > > - moved all changes under buildtools/chkincs,
> > >
> > > Changes since v3:
> > > - removed update of global_inc (which was unneeded, and probably was
> > >   what triggered a Windows clang build issue reported by CI),
> > > - moved all staging operations under a check on check_includes= option,
> > > - moved staging directories under buildtools/chkincs/,
> > > - renamed all variables to reflect those concern the headers check,
> > > - made chkincs binaries depend on staging deps instead of having the
> > >   libraries/drivers depend on them,
> > > - added C++ check for driver headers (missed in v3),
> > >
> > > ---
> > >
> > Played around a bit last week with options for doing chkincs, however, I
> > didn't find anything obviously simpler or better than this. Let's go with
> > this solution!
> >
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Thanks for the review.
> 
> I got an interesting comment from a rabbit (in the middle of a lot of
> noise ;-)).
> https://github.com/ovsrobot/dpdk/pull/106#discussion_r2382870892
> 
> That's not a common case, but this could be tricky to someone moving
> around/renaming headers.
> 
> What do you think?
> 
I think we should probably do some cleanup for the headers, yes, because
moving headers is one case where we would really like the CI to confirm
everything is ok (since moving things probably implies other rework as
well).