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

[David Marchand <david.marchand@redhat.com>]

On Thu, 25 Sept 2025 at 16:47, Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> > index 49dbc55254..376556f5f4 100644
> > --- a/buildtools/chkincs/meson.build
> > +++ b/buildtools/chkincs/meson.build
> > @@ -12,38 +12,47 @@ gen_c_files = generator(gen_c_file_for_header,
> >          arguments: ['@INPUT@', '@OUTPUT@'])
> >
> >  cflags = machine_args
> > +cflags += '-Wno-missing-field-initializers'
> >  cflags += no_wvla_cflag
> > +cflags += '-I' + dpdk_source_root + '/config'
> > +cflags += '-I' + dpdk_build_root
> > +cflags += '-I' + dpdk_chkinc_staging_dir
> >
>
> Two minor suggestions, 1) should we not use "include_directories()" here to
> pass these properly? and 2) would using the existing "global_inc" value as
> include path not work ok? Is the reason for not using the latter the fact
> it includes too many source dirs?

1) include_directories() requires a source directory to exist, so I
would need to keep almost empty directories with minimal meson.build
in them.

2) Indeed, global_inc is not usable as is.

meson.build:global_inc = [include_directories('.', 'config',
meson.build-    'lib/eal/include',
meson.build-    'lib/eal/@0@/include'.format(host_machine.system()),
meson.build-    'lib/eal/@0@/include'.format(arch_subdir),
meson.build-)]

global_inc refers to the EAL sources, so anything in those directories
is reachable.
There are a number of non exported headers in there, like
lib/eal/include/rte_eal_paging.h or Windows specific/internal headers
in lib/eal/windows/include.

I can split it as two variables.

>
> >  sources = files('main.c')
> >  sources += gen_c_files.process(dpdk_chkinc_headers)
> >
> > -# some driver SDK headers depend on these two buses, which are mandatory in build
> > -# so we always include them in deps list
> > -deps = [get_variable('shared_rte_bus_vdev'), get_variable('shared_rte_bus_pci')]
> > -if dpdk_conf.has('RTE_BUS_VMBUS')
> > -    deps += get_variable('shared_rte_bus_vmbus')
> > -endif

[snip]

> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index f25f425565..dae1e83ca4 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -245,7 +245,31 @@ foreach subpath:subdirs
> >          if get_option('enable_driver_sdk')
> >              install_headers(driver_sdk_headers)
> >          endif
> > -        dpdk_chkinc_headers += driver_sdk_headers
> > +        dpdk_chkinc_headers += headers
> > +        dpdk_chkinc_drivers_headers += driver_sdk_headers
> > +
> > +        if get_option('check_includes')
> > +            if headers.length() > 0
> > +                dpdk_chkinc_staging_deps += declare_dependency(sources:
> > +                        custom_target(lib_name + '_header_staging',
> > +                                output: lib_name + '_header_staging.stamp',
> > +                                command: [stage_headers_cmd, dpdk_chkinc_staging_dir, '@OUTPUT@',
> > +                                    headers],
> > +                                install: false,
> > +                        )
> > +                )
>
> Rather than using an external script and custom target for this, would the
> configure_file() meson function, with "copy" set to true also work for us?
> https://mesonbuild.com/Reference-manual_functions.html#configure_file

I don't like configure_file().
Touching any of those headers will trigger a meson reconfigure, right?
I think I am not the only one who dislike meson reconfigurations that
pollutes your terminal a lot when you do many compilations (Thomas?).

But let's say this is acceptable.

I had tried but configure_file() won't accept outputting to a specific
directory, or I am doing it wrong?
Trying again as an example in lib/meson.build:
        foreach header: headers + indirect_headers
            configure_file(input: header,
                output: 'staging/@BASENAME@.h',
                copy: true,
                install: false,
            )
        endforeach

Gives:
../lib/meson.build:221:12: ERROR: configure_file keyword argument
"output" Output 'staging/@BASENAME@.h' must not contain a path
segment.


Relooking *again* at all this (la nuit porte conseil), if we tracked
headers in global variables, for "normal" headers, arch headers,
generic headers and drivers headers, all of the changes could be
isolated in the buildtools/chkincs directory.
This would also make it possible to factorize install_headers() calls,
and avoid drivers doing it behind drivers/meson.build back (I spotted
one).

I just pushed an experiment:
- single headers install:
https://github.com/david-marchand/dpdk/commit/headers_rework~2
- chkincs rework (still not using configure_file):
https://github.com/david-marchand/dpdk/commit/headers_rework~1

WDYT?

>
> > +            endif
> > +            if driver_sdk_headers.length() > 0
> > +                dpdk_chkinc_drivers_staging_deps += declare_dependency(sources:
> > +                        custom_target(lib_name + '_driver_header_staging',
> > +                                output: lib_name + '_driver_header_staging.stamp',
> > +                                command: [stage_headers_cmd, dpdk_chkinc_drivers_staging_dir,
> > +                                    '@OUTPUT@', driver_sdk_headers],
> > +                                install: false,
> > +                        )
> > +                )
> > +            endif
> > +        endif
> >
> >          if headers.length() > 0
> >              dpdk_includes += include_directories(drv_path)



-- 
David Marchand