On Wed, Jan 27, 2021 at 6:37 PM Bruce Richardson
<bruce.richard...@intel.com> 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.
>
> To ensure ongoing compliance, we enable this build test as part of the
> default x86 build in "test-meson-builds.sh".
>
> Signed-off-by: Bruce Richardson <bruce.richard...@intel.com>
> ---
> MAINTAINERS | 4 +++
> buildtools/chkincs/gen_c_file_for_header.py | 12 +++++++
> buildtools/chkincs/main.c | 4 +++
> buildtools/chkincs/meson.build | 40 +++++++++++++++++++++
> devtools/test-meson-builds.sh | 2 +-
> doc/guides/rel_notes/release_21_02.rst | 8 +++++
> meson.build | 5 +++
> 7 files changed, 74 insertions(+), 1 deletion(-)
> create mode 100755 buildtools/chkincs/gen_c_file_for_header.py
> create mode 100644 buildtools/chkincs/main.c
> create mode 100644 buildtools/chkincs/meson.build
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1a12916f56..d4f9ebe46d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1562,6 +1562,10 @@ F: app/test/test_resource.c
> F: app/test/virtual_pmd.c
> F: app/test/virtual_pmd.h
>
> +Header build sanity checking
> +M: Bruce Richardson <bruce.richard...@intel.com>
> +F: buildtools/chkincs/
> +
This can be squashed in the generic "Build System" block.
> Sample packet helper functions for unit test
> M: Reshma Pattan <reshma.pat...@intel.com>
> F: app/test/sample_packet_forward.c
> diff --git a/buildtools/chkincs/gen_c_file_for_header.py
> b/buildtools/chkincs/gen_c_file_for_header.py
> new file mode 100755
> index 0000000000..ed46948aea
> --- /dev/null
> +++ b/buildtools/chkincs/gen_c_file_for_header.py
> @@ -0,0 +1,12 @@
> +#! /usr/bin/env python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2021 Intel Corporation
> +
> +from sys import argv
> +from os.path import abspath
> +
> +(h_file, c_file) = argv[1:]
> +
> +contents = '#include "' + abspath(h_file) + '"'
> +with open(c_file, 'w') as cf:
> + cf.write(contents)
> diff --git a/buildtools/chkincs/main.c b/buildtools/chkincs/main.c
> new file mode 100644
> index 0000000000..d25bb8852a
> --- /dev/null
> +++ b/buildtools/chkincs/main.c
> @@ -0,0 +1,4 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Intel Corporation
> + */
> +int main(void) { return 0; }
> diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> new file mode 100644
> index 0000000000..5dc43283e0
> --- /dev/null
> +++ b/buildtools/chkincs/meson.build
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2021 Intel Corporation
> +
> +if not get_option('check_includes')
> + build = false
> + subdir_done()
> +endif
> +
> +if is_windows
> + # for windows, the shebang line in the script won't work.
> + error('option "check_includes" is not supported on windows')
> +endif
> +
> +gen_c_file_for_header = find_program('gen_c_file_for_header.py')
> +gen_c_files = generator(gen_c_file_for_header,
> + output: '@BASENAME@.c',
> + arguments: ['@INPUT@', '@OUTPUT@'])
> +
> +cflags = machine_args
> +cflags += '-Wno-unused-function' # needed if we include generic headers
> +cflags += '-DALLOW_EXPERIMENTAL_API'
> +
> +# some ethdev headers depend on bus headers
> +includes = include_directories('../../drivers/bus/pci',
> + '../../drivers/bus/vdev')
ethdev headers are fine now, afaics.
So this comment can be changed to a more vague "some driver headers
depend on bus headers".
> +
> +sources = files('main.c')
> +sources += gen_c_files.process(dpdk_chkinc_headers)
> +
> +deps = []
> +foreach l:enabled_libs
> + deps += get_variable('static_rte_' + l)
> +endforeach
> +
> +executable('chkincs', sources,
> + c_args: cflags,
> + include_directories: includes,
> + dependencies: deps,
> + link_whole: dpdk_static_libraries + dpdk_drivers,
> + install: false)
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index efa91e0e40..07b5e6aeca 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -227,7 +227,7 @@ default_machine='nehalem'
> if ! check_cc_flags "-march=$default_machine" ; then
> default_machine='corei7'
> fi
> -build build-x86-default cc skipABI \
> +build build-x86-default cc skipABI -Dcheck_includes=true\
Space before \ to be consistent with the rest of the file.
> -Dlibdir=lib -Dmachine=$default_machine $use_shared
>
> # 32-bit with default compiler
> diff --git a/doc/guides/rel_notes/release_21_02.rst
> b/doc/guides/rel_notes/release_21_02.rst
> index 33bac4fff8..245d1a6473 100644
> --- a/doc/guides/rel_notes/release_21_02.rst
> +++ b/doc/guides/rel_notes/release_21_02.rst
> @@ -122,6 +122,14 @@ New Features
> * Added support for aes-cbc sha256-128-hmac cipher combination in OCTEON
> TX2
> crypto PMD lookaside protocol offload for IPsec.
>
> +* **Added support for build-time checking of header includes**
> +
> + A new build option ``check_includes`` has been added, which, when enabled,
> + will perform build-time checking on DPDK public header files, to ensure
> none
> + are missing dependent header includes. This feature, disabled by default,
> is
> + intended for use by developers contributing to the DPDK SDK itself, and is
> + integrated into the build scripts and automated CI for patch contributions.
> +
>
Should be earlier in the new features list.
> Removed Items
> -------------
> diff --git a/meson.build b/meson.build
> index e6e34d0a98..fcc4d4c900 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -68,6 +68,11 @@ if get_option('enable_kmods')
> subdir('kernel')
> endif
>
> +# check header includes if requested
> +if get_option('check_includes')
> + subdir('buildtools/chkincs')
> +endif
> +
> # write the build config
> build_cfg = 'rte_build_config.h'
> configure_file(output: build_cfg,
> --
> 2.27.0
>
--
David Marchand
