From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 67635A09E4; Thu, 28 Jan 2021 12:27:27 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E60BD4067B; Thu, 28 Jan 2021 12:27:26 +0100 (CET) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by mails.dpdk.org (Postfix) with ESMTP id 33E114067A for ; Thu, 28 Jan 2021 12:27:25 +0100 (CET) IronPort-SDR: NMrhP6odazUutkqxx/dySOuPkKPntdfXqcq++rn9Fm313As3V5kCCht6VUKgPBZpWz60vqEkg/ rHbo4wpJzb7A== X-IronPort-AV: E=McAfee;i="6000,8403,9877"; a="167314949" X-IronPort-AV: E=Sophos;i="5.79,382,1602572400"; d="scan'208";a="167314949" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jan 2021 03:27:23 -0800 IronPort-SDR: yL1+GjEaY2ji+k9Qtdi9IV/0JuUVvdBhst8CT9twc9bA/eMe7npDyBjt+knBaOBH8aJACjmq6f 1bZvqIG+zO7w== X-IronPort-AV: E=Sophos;i="5.79,382,1602572400"; d="scan'208";a="473509651" Received: from bricha3-mobl.ger.corp.intel.com ([10.252.11.53]) by fmsmga001-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 28 Jan 2021 03:27:22 -0800 Date: Thu, 28 Jan 2021 11:27:19 +0000 From: Bruce Richardson To: David Marchand Cc: dev , Thomas Monjalon Message-ID: <20210128112719.GD899@bricha3-MOBL.ger.corp.intel.com> References: <20210114110606.21142-1-bruce.richardson@intel.com> <20210127173330.1671341-1-bruce.richardson@intel.com> <20210127173330.1671341-7-bruce.richardson@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v6 6/8] buildtools/chkincs: add app to verify header includes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jan 28, 2021 at 12:02:35PM +0100, David Marchand wrote: > On Wed, Jan 27, 2021 at 6:37 PM Bruce Richardson > 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 > > --- > > 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 > > +F: buildtools/chkincs/ > > + > > This can be squashed in the generic "Build System" block. > Ok. > > > Sample packet helper functions for unit test > > M: Reshma Pattan > > 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". > Driver headers are not checked yet by the code, only lib ones, so I will see if this block can be omitted until needed. > > > + > > +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. > Sure. > > > -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. > This was actually a deliberate choice on my part, since I was in two minds as to whether or how this should be called out in the release note. I felt it was not really of interest to user, and only to DPDK developers, so I put it at the end to de-emphasise it.