[dpdk-dev] [PATCH v6 6/8] buildtools/chkincs: add app to verify header includes

[David Marchand]

On Wed, Jan 27, 2021 at 6:37 PM Bruce Richardson
<bruce.richardson at 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.richardson at 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.richardson at 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.pattan at 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 at .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