From 05050ac4ce99d199ef416bd3c1e08443ddb433eb Mon Sep 17 00:00:00 2001
From: Bruce Richardson <bruce.richardson@intel.com>
Date: Fri, 29 Jan 2021 16:48:21 +0000
Subject: [PATCH] build: add header includes check

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.

For some libraries, there may be some header files which are not for direct
inclusion, but rather are to be included via other header files. To allow
later checking of these files for missing includes, we separate out the
indirect include files from the direct ones.

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/contributing/coding_style.rst    | 12 +++++++
 doc/guides/rel_notes/release_21_02.rst      |  8 +++++
 lib/librte_eal/include/meson.build          |  2 +-
 lib/librte_eal/x86/include/meson.build      | 14 +++++---
 lib/librte_ethdev/meson.build               |  5 +--
 lib/librte_hash/meson.build                 |  4 +--
 lib/librte_ipsec/meson.build                |  3 +-
 lib/librte_lpm/meson.build                  |  2 +-
 lib/librte_regexdev/meson.build             |  2 +-
 lib/librte_ring/meson.build                 |  4 ++-
 lib/librte_stack/meson.build                |  4 ++-
 lib/librte_table/meson.build                |  7 ++--
 lib/meson.build                             |  3 ++
 meson.build                                 |  6 ++++
 meson_options.txt                           |  2 ++
 20 files changed, 113 insertions(+), 20 deletions(-)
 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 6af59eaff6..ec203143c3 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/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index bb3f3efcbc..734d1901b2 100644
--- a/doc/guides/contributing/coding_style.rst
+++ b/doc/guides/contributing/coding_style.rst
@@ -891,6 +891,18 @@ headers
 	installed to $PREFIX/include when ``ninja install`` is run. As with
 	source files, these should be specified using the meson ``files()``
 	function.
+	When ``check_includes`` build option is set to ``true``, each header file
+	has additional checks performed on it, for example to ensure that it is
+	not missing any include statements for dependent headers.
+	For header files which are public, but only included indirectly in
+	applications, these checks can be skipped by using the ``indirect_headers``
+	variable rather than ``headers``.
+
+indirect_headers
+	**Default Value = []**.
+	As with ``headers`` option above, except that the files are not checked
+	for all needed include files as part of a DPDK build when
+	``check_includes`` is set to ``true``.
 
 includes:
 	**Default Value = []**.
diff --git a/doc/guides/rel_notes/release_21_02.rst b/doc/guides/rel_notes/release_21_02.rst
index d26e581563..29046a6f7d 100644
--- a/doc/guides/rel_notes/release_21_02.rst
+++ b/doc/guides/rel_notes/release_21_02.rst
@@ -138,6 +138,14 @@ New Features
 
   See the :doc:`../compressdevs/mlx5` for more details.
 
+* **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/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index 0dea342e1d..3969cf4ac7 100644
--- a/lib/librte_eal/include/meson.build
+++ b/lib/librte_eal/include/meson.build
@@ -16,7 +16,6 @@ headers += files(
 	'rte_dev.h',
 	'rte_devargs.h',
 	'rte_eal.h',
-	'rte_eal_interrupts.h',
 	'rte_eal_memconfig.h',
 	'rte_eal_trace.h',
 	'rte_errno.h',
@@ -49,6 +48,7 @@ headers += files(
 	'rte_version.h',
 	'rte_vfio.h',
 )
+indirect_headers += files('rte_eal_interrupts.h')
 
 # special case install the generic headers, since they go in a subdir
 generic_headers = files(
diff --git a/lib/librte_eal/x86/include/meson.build b/lib/librte_eal/x86/include/meson.build
index 549cc21a42..1a6ad0b173 100644
--- a/lib/librte_eal/x86/include/meson.build
+++ b/lib/librte_eal/x86/include/meson.build
@@ -2,11 +2,7 @@
 # Copyright(c) 2017 Intel Corporation
 
 arch_headers = files(
-	'rte_atomic_32.h',
-	'rte_atomic_64.h',
 	'rte_atomic.h',
-	'rte_byteorder_32.h',
-	'rte_byteorder_64.h',
 	'rte_byteorder.h',
 	'rte_cpuflags.h',
 	'rte_cycles.h',
@@ -22,4 +18,12 @@ arch_headers = files(
 	'rte_ticketlock.h',
 	'rte_vect.h',
 )
-install_headers(arch_headers, subdir: get_option('include_subdir_arch'))
+arch_indirect_headers = files(
+	'rte_atomic_32.h',
+	'rte_atomic_64.h',
+	'rte_byteorder_32.h',
+	'rte_byteorder_64.h',
+)
+install_headers(arch_headers + arch_indirect_headers,
+		subdir: get_option('include_subdir_arch'))
+dpdk_chkinc_headers += arch_headers
diff --git a/lib/librte_ethdev/meson.build b/lib/librte_ethdev/meson.build
index 829abd456b..c37b2e3772 100644
--- a/lib/librte_ethdev/meson.build
+++ b/lib/librte_ethdev/meson.build
@@ -11,10 +11,8 @@ sources = files('ethdev_private.c',
 	'rte_tm.c')
 
 headers = files('rte_ethdev.h',
-	'rte_ethdev_core.h',
 	'rte_ethdev_trace.h',
 	'rte_ethdev_trace_fp.h',
-	'rte_eth_ctrl.h',
 	'rte_dev_info.h',
 	'rte_flow.h',
 	'rte_flow_driver.h',
@@ -22,5 +20,8 @@ headers = files('rte_ethdev.h',
 	'rte_mtr_driver.h',
 	'rte_tm.h',
 	'rte_tm_driver.h')
+indirect_headers += files(
+	'rte_ethdev_core.h',
+	'rte_eth_ctrl.h')
 
 deps += ['net', 'kvargs', 'meter', 'telemetry']
diff --git a/lib/librte_hash/meson.build b/lib/librte_hash/meson.build
index 0977a63fd2..242859f5ad 100644
--- a/lib/librte_hash/meson.build
+++ b/lib/librte_hash/meson.build
@@ -1,12 +1,12 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-headers = files('rte_crc_arm64.h',
-	'rte_fbk_hash.h',
+headers = files('rte_fbk_hash.h',
 	'rte_hash_crc.h',
 	'rte_hash.h',
 	'rte_jhash.h',
 	'rte_thash.h')
+indirect_headers += files('rte_crc_arm64.h')
 
 sources = files('rte_cuckoo_hash.c', 'rte_fbk_hash.c')
 deps += ['ring']
diff --git a/lib/librte_ipsec/meson.build b/lib/librte_ipsec/meson.build
index fc69970ec5..1497f573bb 100644
--- a/lib/librte_ipsec/meson.build
+++ b/lib/librte_ipsec/meson.build
@@ -3,6 +3,7 @@
 
 sources = files('esp_inb.c', 'esp_outb.c', 'sa.c', 'ses.c', 'ipsec_sad.c')
 
-headers = files('rte_ipsec.h', 'rte_ipsec_group.h', 'rte_ipsec_sa.h', 'rte_ipsec_sad.h')
+headers = files('rte_ipsec.h', 'rte_ipsec_sa.h', 'rte_ipsec_sad.h')
+indirect_headers += files('rte_ipsec_group.h')
 
 deps += ['mbuf', 'net', 'cryptodev', 'security', 'hash']
diff --git a/lib/librte_lpm/meson.build b/lib/librte_lpm/meson.build
index f93c866409..90cbf8646a 100644
--- a/lib/librte_lpm/meson.build
+++ b/lib/librte_lpm/meson.build
@@ -5,6 +5,6 @@ sources = files('rte_lpm.c', 'rte_lpm6.c')
 headers = files('rte_lpm.h', 'rte_lpm6.h')
 # since header files have different names, we can install all vector headers
 # without worrying about which architecture we actually need
-headers += files('rte_lpm_altivec.h', 'rte_lpm_neon.h', 'rte_lpm_sse.h', 'rte_lpm_sve.h')
+indirect_headers += files('rte_lpm_altivec.h', 'rte_lpm_neon.h', 'rte_lpm_sse.h', 'rte_lpm_sve.h')
 deps += ['hash']
 deps += ['rcu']
diff --git a/lib/librte_regexdev/meson.build b/lib/librte_regexdev/meson.build
index c417b9caf0..1b3b6db22a 100644
--- a/lib/librte_regexdev/meson.build
+++ b/lib/librte_regexdev/meson.build
@@ -3,6 +3,6 @@
 
 sources = files('rte_regexdev.c')
 headers = files('rte_regexdev.h',
-	'rte_regexdev_core.h',
 	'rte_regexdev_driver.h')
+indirect_headers += files('rte_regexdev_core.h')
 deps += ['mbuf']
diff --git a/lib/librte_ring/meson.build b/lib/librte_ring/meson.build
index f007893b5f..ea050d564a 100644
--- a/lib/librte_ring/meson.build
+++ b/lib/librte_ring/meson.build
@@ -2,7 +2,9 @@
 # Copyright(c) 2017 Intel Corporation
 
 sources = files('rte_ring.c')
-headers = files('rte_ring.h',
+headers = files('rte_ring.h')
+# most sub-headers are not for direct inclusion
+indirect_headers += files (
 		'rte_ring_core.h',
 		'rte_ring_elem.h',
 		'rte_ring_elem_pvt.h',
diff --git a/lib/librte_stack/meson.build b/lib/librte_stack/meson.build
index 8f82a40ec2..9ff33722b2 100644
--- a/lib/librte_stack/meson.build
+++ b/lib/librte_stack/meson.build
@@ -2,7 +2,9 @@
 # Copyright(c) 2019 Intel Corporation
 
 sources = files('rte_stack.c', 'rte_stack_std.c', 'rte_stack_lf.c')
-headers = files('rte_stack.h',
+headers = files('rte_stack.h')
+# subheaders, not for direct inclusion by apps
+indirect_headers += files(
 		'rte_stack_std.h',
 		'rte_stack_lf.h',
 		'rte_stack_lf_generic.h',
diff --git a/lib/librte_table/meson.build b/lib/librte_table/meson.build
index d69678386e..aa1e1d0385 100644
--- a/lib/librte_table/meson.build
+++ b/lib/librte_table/meson.build
@@ -20,7 +20,6 @@ headers = files('rte_table.h',
 		'rte_table_hash.h',
 		'rte_table_hash_cuckoo.h',
 		'rte_table_hash_func.h',
-		'rte_table_hash_func_arm64.h',
 		'rte_lru.h',
 		'rte_table_array.h',
 		'rte_table_stub.h',
@@ -28,6 +27,6 @@ headers = files('rte_table.h',
 		'rte_swx_table_em.h',)
 deps += ['mbuf', 'port', 'lpm', 'hash', 'acl']
 
-if arch_subdir == 'x86'
-	headers += files('rte_lru_x86.h')
-endif
+indirect_headers += files('rte_lru_x86.h',
+		'rte_lru_arm64.h',
+		'rte_table_hash_func_arm64.h')
diff --git a/lib/meson.build b/lib/meson.build
index 44f0a62142..7712aa4977 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -66,6 +66,7 @@ foreach l:libraries
 	use_function_versioning = false
 	sources = []
 	headers = []
+	indirect_headers = [] # public headers not directly included by apps
 	includes = []
 	cflags = default_cflags
 	objs = [] # other object files to link against, used e.g. for
@@ -103,6 +104,8 @@ foreach l:libraries
 		enabled_libs += name
 		dpdk_conf.set('RTE_LIB_' + name.to_upper(), 1)
 		install_headers(headers)
+		install_headers(indirect_headers)
+		dpdk_chkinc_headers += headers
 
 		libname = 'rte_' + name
 		includes += include_directories(dir_name)
diff --git a/meson.build b/meson.build
index 2b9c37eb42..fcc4d4c900 100644
--- a/meson.build
+++ b/meson.build
@@ -16,6 +16,7 @@ cc = meson.get_compiler('c')
 dpdk_conf = configuration_data()
 dpdk_libraries = []
 dpdk_static_libraries = []
+dpdk_chkinc_headers = []
 dpdk_driver_classes = []
 dpdk_drivers = []
 dpdk_extra_ldflags = []
@@ -67,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,
diff --git a/meson_options.txt b/meson_options.txt
index 4604328224..5c382487da 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('check_includes', type: 'boolean', value: false,
+	description: 'build "chkincs" to verify each header file can compile alone')
 option('disable_drivers', type: 'string', value: '',
 	description: 'Comma-separated list of drivers to explicitly disable.')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',