Re: [PATCH v3 6/7] buildtools/chkincs: use a staging directory for headers

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

On Thu, Sep 25, 2025 at 10:42:47AM +0200, David Marchand wrote:
> Hello Bruce,
> 
> On Thu, 25 Sept 2025 at 10:00, Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, Sep 24, 2025 at 07:25:34PM +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>
> >
> > In general looks ok to me.  One small comment though - can we not have
> > "staging" as a top-level directory, but instead hide it inside the
> > buildtools directory, or even the chkincs directory? I dislike having
> > too many subdirectories directly off the root of the project,
> > especially ones purely for internal tooling.
> 
> Well, at first I was trying to change the whole build process iow rely
> only on the staging directory and remove all the include_directories:
> directives from the declare_dependency() objects.  Libraries and apps
> were ok, but there were a *lot* of complications with drivers (what a
> *huge mess*, especially for NXP drivers with "compat.h" includes, and
> Marvell drivers to a smaller extent).  I may retry in the future with
> some AI tool that will brute force this :-).
> 
One note of caution here: if doing this, you may want to consider using
run_command rather than a custom_target to copy the headers in order to
avoid potentially slowing down the build.

We used to do this copying of header files in the old make build system,
and one downside of it is that it means that the build of ".c" files in one
directory cannot be started until the copying of headers from the dependent
components are completed. The decision to use the include paths in the
dependency objects rather than copying the headers to a central location
was a deliberate decision when moving build system because it means that
when you run "ninja", every single .c file can be compiled to a .o file
in parallel, because all dependent headers are available at their original
locations. For most components, it's only the final link stage that has any
dependencies, the compile commands are all independent.

On the other hand, using run_command is not necessarily a good solution
either, because it means that all changes to the headers require a re-run
of meson, which will also be slower because of all the copying. :-(

Therefore, I'd tend towards keeping things as they are here, in order to
minimise reconfiguration and build times.

/Bruce