diff mbox series

[v6,5/8] build: separate out headers for include checking

Message ID 20210127173330.1671341-6-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers show
Series add checking of header includes | expand

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Jan. 27, 2021, 5:33 p.m. UTC
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.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 doc/guides/contributing/coding_style.rst | 12 ++++++++++++
 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                              |  1 +
 meson_options.txt                        |  2 ++
 14 files changed, 46 insertions(+), 19 deletions(-)

Comments

Thomas Monjalon Jan. 28, 2021, 11:07 a.m. UTC | #1
27/01/2021 18:33, Bruce Richardson:
> 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.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

[...]
> +	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 ``headers_no_chkincs``
> +	variable rather than ``headers``.
> +
> +headers_no_chkincs
> +	**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``.

If all such headers are included directly, I would prefer naming this group
"indirect_headers" because maybe we will want to do other kind of processing
on indirect headers.

[...]
> --- 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')

This should in the patch introducing the check?
Bruce Richardson Jan. 28, 2021, 11:23 a.m. UTC | #2
On Thu, Jan 28, 2021 at 12:07:11PM +0100, Thomas Monjalon wrote:
> 27/01/2021 18:33, Bruce Richardson:
> > 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.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> [...]
> > +	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 ``headers_no_chkincs``
> > +	variable rather than ``headers``.
> > +
> > +headers_no_chkincs
> > +	**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``.
> 
> If all such headers are included directly, I would prefer naming this group
> "indirect_headers" because maybe we will want to do other kind of processing
> on indirect headers.
> 
Sure. The current naming was chosen so that a grep of the code for on
receiving errors about "chkincs" would show this up, but I think your
suggestion is better.

> [...]
> > --- 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')
> 
> This should in the patch introducing the check?
> 
It was originally, but then we hit an issue with wanting/needing to refer
to this option in the documentation about splitting the header file lists.
Therefore, I decided to add the option in this patch. Alternatively, this
patch and the next can be merged into one.
diff mbox series

Patch

diff --git a/doc/guides/contributing/coding_style.rst b/doc/guides/contributing/coding_style.rst
index bb3f3efcbc..dba4145228 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 ``headers_no_chkincs``
+	variable rather than ``headers``.
+
+headers_no_chkincs
+	**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/lib/librte_eal/include/meson.build b/lib/librte_eal/include/meson.build
index 0dea342e1d..449740e510 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',
 )
+headers_no_chkincs += 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..835ea22947 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_headers_no_chkincs = files(
+	'rte_atomic_32.h',
+	'rte_atomic_64.h',
+	'rte_byteorder_32.h',
+	'rte_byteorder_64.h',
+)
+install_headers(arch_headers + arch_headers_no_chkincs,
+		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..b7da959dd1 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')
+headers_no_chkincs += 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..b3ebc8b078 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')
+headers_no_chkincs += 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..e24e6ed22b 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')
+headers_no_chkincs += 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..3d3d515a4d 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')
+headers_no_chkincs += 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..1ab21bd4d8 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')
+headers_no_chkincs += files('rte_regexdev_core.h')
 deps += ['mbuf']
diff --git a/lib/librte_ring/meson.build b/lib/librte_ring/meson.build
index 36fdcb6a57..1a95dae9e5 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
+headers_no_chkincs += files (
 		'rte_ring_core.h',
 		'rte_ring_elem.h',
 		'rte_ring_c11_mem.h',
diff --git a/lib/librte_stack/meson.build b/lib/librte_stack/meson.build
index 8f82a40ec2..5d9b3601b3 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
+headers_no_chkincs += 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..a8b1c9a254 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
+headers_no_chkincs += 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..aa0a5ac8fc 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -66,6 +66,7 @@  foreach l:libraries
 	use_function_versioning = false
 	sources = []
 	headers = []
+	headers_no_chkincs = [] # 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(headers_no_chkincs)
+		dpdk_chkinc_headers += headers
 
 		libname = 'rte_' + name
 		includes += include_directories(dir_name)
diff --git a/meson.build b/meson.build
index 2b9c37eb42..e6e34d0a98 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 = []
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>',