On Thu, Sep 25, 2025 at 12:29:08PM +0200, David Marchand wrote: > On Thu, 25 Sept 2025 at 11:22, Bruce Richardson > <[email protected]> wrote: > > > > 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 > > > <[email protected]> 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 <[email protected]> > > > > > > > > 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. > > I am sticking to the headers check update atm. > The slow down should only concern developers using check_includes, > once I properly gate those deps to the meson option. > > Is it still a concern for you? > No, no concerns about copying the headers over for check_includes checks, that makes sense. I'm just flagging a potential issue if you want to switch back to the old model of copying the headers as part of a normal build.
/Bruce
