[RFC,1/1] build: increase minimum C standard for DPDK builds

Message ID 20230112113556.47485-2-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Specify C-standard requirement for DPDK builds |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing fail Unit Testing FAIL
ci/Intel-compilation fail Compilation issues

Commit Message

Bruce Richardson Jan. 12, 2023, 11:35 a.m. UTC
  Set the default C language standard to be used for DPDK builds to C99.
This requires no actual code changes to build successfully.

To ensure compatibility is kept for external apps using DPDK headers, we
explicitly set the build parameters for the chkincs binary to the old
minimum standard of "gnu89". [NOTE: DPDK code does not compile and has
previously not compiled for pure c89 standard, so that stricter
requirement need not be checked.] By adding this additional check, we
can separately manage C standards used internally in DPDK builds and
that required in the build flags for external apps using DPDK.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 buildtools/chkincs/meson.build | 1 +
 meson.build                    | 1 +
 2 files changed, 2 insertions(+)
  

Comments

Morten Brørup Jan. 12, 2023, 12:42 p.m. UTC | #1
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Thursday, 12 January 2023 12.36
> 
> Set the default C language standard to be used for DPDK builds to C99.
> This requires no actual code changes to build successfully.

Great!

> 
> To ensure compatibility is kept for external apps using DPDK headers,
> we
> explicitly set the build parameters for the chkincs binary to the old
> minimum standard of "gnu89". [NOTE: DPDK code does not compile and has
> previously not compiled for pure c89 standard, so that stricter
> requirement need not be checked.] By adding this additional check, we
> can separately manage C standards used internally in DPDK builds and
> that required in the build flags for external apps using DPDK.

It seems I have to accept this techboard decision for now. ;-)

> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---

Acked-by: Morten Brørup <mb@smartsharesystems.com>
  
Bruce Richardson Jan. 12, 2023, 12:47 p.m. UTC | #2
On Thu, Jan 12, 2023 at 01:42:48PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Thursday, 12 January 2023 12.36
> > 
> > Set the default C language standard to be used for DPDK builds to C99.
> > This requires no actual code changes to build successfully.
> 
> Great!
> 
> > 
> > To ensure compatibility is kept for external apps using DPDK headers,
> > we
> > explicitly set the build parameters for the chkincs binary to the old
> > minimum standard of "gnu89". [NOTE: DPDK code does not compile and has
> > previously not compiled for pure c89 standard, so that stricter
> > requirement need not be checked.] By adding this additional check, we
> > can separately manage C standards used internally in DPDK builds and
> > that required in the build flags for external apps using DPDK.
> 
> It seems I have to accept this techboard decision for now. ;-)
> 
There is no techboard decision here - certainly not yet. There is plenty of
discussion still to be had. However, for now this RFC is being very
conservative - perhaps overly so.

/Bruce
  
Morten Brørup Jan. 12, 2023, 3:06 p.m. UTC | #3
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Thursday, 12 January 2023 13.47
> 
> On Thu, Jan 12, 2023 at 01:42:48PM +0100, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Thursday, 12 January 2023 12.36
> > >
> > > Set the default C language standard to be used for DPDK builds to
> C99.
> > > This requires no actual code changes to build successfully.
> >
> > Great!
> >
> > >
> > > To ensure compatibility is kept for external apps using DPDK
> headers,
> > > we
> > > explicitly set the build parameters for the chkincs binary to the
> old
> > > minimum standard of "gnu89". [NOTE: DPDK code does not compile and
> has
> > > previously not compiled for pure c89 standard, so that stricter
> > > requirement need not be checked.] By adding this additional check,
> we
> > > can separately manage C standards used internally in DPDK builds
> and
> > > that required in the build flags for external apps using DPDK.
> >
> > It seems I have to accept this techboard decision for now. ;-)
> >
> There is no techboard decision here - certainly not yet. There is
> plenty of
> discussion still to be had. However, for now this RFC is being very
> conservative - perhaps overly so.

Yeah, perhaps I was phrasing that somewhat emotionally loaded. :-)

I didn't mean to cause confusion on the mailing list. Thanks for clarifying, Bruce.
  
Tyler Retzlaff Jan. 12, 2023, 5:04 p.m. UTC | #4
On Thu, Jan 12, 2023 at 11:35:56AM +0000, Bruce Richardson wrote:
> Set the default C language standard to be used for DPDK builds to C99.
> This requires no actual code changes to build successfully.
> 
> To ensure compatibility is kept for external apps using DPDK headers, we
> explicitly set the build parameters for the chkincs binary to the old
> minimum standard of "gnu89". [NOTE: DPDK code does not compile and has
> previously not compiled for pure c89 standard, so that stricter
> requirement need not be checked.] By adding this additional check, we
> can separately manage C standards used internally in DPDK builds and
> that required in the build flags for external apps using DPDK.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  buildtools/chkincs/meson.build | 1 +
>  meson.build                    | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
> index 378c2f19ef..322ac775ce 100644
> --- a/buildtools/chkincs/meson.build
> +++ b/buildtools/chkincs/meson.build
> @@ -30,6 +30,7 @@ executable('chkincs', sources,
>          c_args: cflags,
>          include_directories: includes,
>          dependencies: deps,
> +        override_options: 'c_std=gnu89',
>          install: false)
>  
>  # run tests for c++ builds also
> diff --git a/meson.build b/meson.build
> index f91d652bc5..9a2963cc16 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -9,6 +9,7 @@ project('DPDK', 'C',
>          license: 'BSD',
>          default_options: [
>              'buildtype=release',
> +            'c_std=c99',
>              'default_library=static',
>              'warning_level=2',
>          ],

subject to the atomics abstraction proposal where a meson option is
provided enable_stdatomics=true we'll want to be able to upgrade to
-std=c11 / c_std=c11 (as a minimum).

is there a meson mechanism that will let us evaluate that condition and
specify the higher required standard version in default_options  or is it
a matter of some post project() or is this just done by adding
-std=c11 using add_project_arguments() after project()?

second, i think you will run into a build break with this change on some
platform / compiler combinations. somewhere we are using strerror_r where
the required posix versions are not specified in the translation unit
worked around by using c_std=gnu99 or by adding appropriate undef GNUC
etc.. in the place where strerror_r is used. (just something i noticed
when testing a similar change during prototyping i think ubuntu
22.04?)

going one step further i'd just ask that the default options be
"portable" i.e. they work with ! gcc and ! clang to help me with future
work. though i know that contradicts with my advice in the previous
paragraph, maybe we can set c_std=xxx subject to toolchain/platform?

Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
  

Patch

diff --git a/buildtools/chkincs/meson.build b/buildtools/chkincs/meson.build
index 378c2f19ef..322ac775ce 100644
--- a/buildtools/chkincs/meson.build
+++ b/buildtools/chkincs/meson.build
@@ -30,6 +30,7 @@  executable('chkincs', sources,
         c_args: cflags,
         include_directories: includes,
         dependencies: deps,
+        override_options: 'c_std=gnu89',
         install: false)
 
 # run tests for c++ builds also
diff --git a/meson.build b/meson.build
index f91d652bc5..9a2963cc16 100644
--- a/meson.build
+++ b/meson.build
@@ -9,6 +9,7 @@  project('DPDK', 'C',
         license: 'BSD',
         default_options: [
             'buildtype=release',
+            'c_std=c99',
             'default_library=static',
             'warning_level=2',
         ],