diff mbox series

[v7,08/10] buildtools/chkincs: add app to verify header includes

Message ID 20210129164823.3205861-9-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series add checking of header includes |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Bruce Richardson Jan. 29, 2021, 4:48 p.m. UTC 
  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@intel.com>
---
 MAINTAINERS                                 |  1 +
 buildtools/chkincs/gen_c_file_for_header.py | 12 +++++++
 buildtools/chkincs/main.c                   |  4 +++
 buildtools/chkincs/meson.build              | 36 +++++++++++++++++++++
 devtools/test-meson-builds.sh               |  2 +-
 doc/guides/rel_notes/release_21_02.rst      |  8 +++++
 meson.build                                 |  5 +++
 7 files changed, 67 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

Comments

David Marchand Feb. 2, 2021, 11:36 a.m. UTC | #1 
On Fri, Jan 29, 2021 at 5:49 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> new file mode 100644
> index 0000000000..f345e87551
> --- /dev/null
> +++ b/buildtools/chkincs/meson.build
> @@ -0,0 +1,36 @@
> +# 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

With http://patchwork.dpdk.org/patch/87567/, we have no direct
inclusion of the generic headers remaining.
This flag could be removed.
WDYT?


> +cflags += '-DALLOW_EXPERIMENTAL_API'
> +
> +sources = files('main.c')
> +sources += gen_c_files.process(dpdk_chkinc_headers)
> +


--
David Marchand
Bruce Richardson Feb. 2, 2021, 12:23 p.m. UTC | #2 
On Tue, Feb 02, 2021 at 12:36:34PM +0100, David Marchand wrote:
> On Fri, Jan 29, 2021 at 5:49 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> > diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> > new file mode 100644
> > index 0000000000..f345e87551
> > --- /dev/null
> > +++ b/buildtools/chkincs/meson.build
> > @@ -0,0 +1,36 @@
> > +# 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
> 
> With http://patchwork.dpdk.org/patch/87567/, we have no direct
> inclusion of the generic headers remaining.
> This flag could be removed.
> WDYT?
> 

If it can be removed, it should be, IMHO.

/Bruce
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 1a12916f56..aeefe3b1be 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -98,6 +98,7 @@  F: Makefile
 F: meson.build
 F: meson_options.txt
 F: config/
+F: buildtools/chkincs/
 F: buildtools/call-sphinx-build.py
 F: buildtools/list-dir-globs.py
 F: buildtools/pkg-config/
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..f345e87551
--- /dev/null
+++ b/buildtools/chkincs/meson.build
@@ -0,0 +1,36 @@ 
+# 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'
+
+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..a99a177e0c 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 \
 	-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 17fcfd661c..fc3bf51cb6 100644
--- a/doc/guides/rel_notes/release_21_02.rst
+++ b/doc/guides/rel_notes/release_21_02.rst
@@ -132,6 +132,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.
+
 
 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,