[dpdk-dev,01/17] build: add initial infrastructure for meson & ninja builds
Checks
Commit Message
To build with meson and ninja, we need some initial infrastructure in
place. The build files for meson always need to be called "meson.build",
and options get placed in meson_options.txt
This commit adds a top-level meson.build file, which sets up the global
variables for tracking drivers, libraries, etc., and then includes other
build files, before finishing by writing the global build configuration
header file and a DPDK pkgconfig file at the end, using some of those same
globals.
From the top level build file, the only include file thus far is for the
config folder, which does some other setup of global configuration
parameters, including pulling in architecture specific parameters from an
architectural subdirectory. A number of configuration build options are
provided for the project to tune a number of global variables which will be
used later e.g. max numa nodes, max cores, etc. These settings all make
their way to the global build config header "rte_build_config.h". There is
also a file "rte_config.h", which includes "rte_build_config.h", and this
file is meant to hold other build-time values which are present in our
current static build configuration but are not normally meant for
user-configuration. Ideally, over time, the values placed here should be
moved to the individual libraries or drivers which want those values.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
config/meson.build | 69 +++++++++++++++++++++++++++++++++++++++++
config/rte_config.h | 50 ++++++++++++++++++++++++++++++
config/x86/meson.build | 70 ++++++++++++++++++++++++++++++++++++++++++
meson.build | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
meson_options.txt | 6 ++++
5 files changed, 278 insertions(+)
create mode 100644 config/meson.build
create mode 100644 config/rte_config.h
create mode 100644 config/x86/meson.build
create mode 100644 meson.build
create mode 100644 meson_options.txt
Comments
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Friday, September 1, 2017 11:04 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure for meson & ninja builds
>
> To build with meson and ninja, we need some initial infrastructure in
> place. The build files for meson always need to be called "meson.build",
> and options get placed in meson_options.txt
>
> This commit adds a top-level meson.build file, which sets up the global
> variables for tracking drivers, libraries, etc., and then includes other
> build files, before finishing by writing the global build configuration
> header file and a DPDK pkgconfig file at the end, using some of those same
> globals.
>
> From the top level build file, the only include file thus far is for the
> config folder, which does some other setup of global configuration
> parameters, including pulling in architecture specific parameters from an
> architectural subdirectory. A number of configuration build options are
> provided for the project to tune a number of global variables which will be
> used later e.g. max numa nodes, max cores, etc. These settings all make
> their way to the global build config header "rte_build_config.h". There is
> also a file "rte_config.h", which includes "rte_build_config.h", and this
> file is meant to hold other build-time values which are present in our
> current static build configuration but are not normally meant for
> user-configuration. Ideally, over time, the values placed here should be
> moved to the individual libraries or drivers which want those values.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Comments inline below;
<snip>
> diff --git a/config/meson.build b/config/meson.build
<snip>
> +# disable any unwanted warnings
> +unwanted_warnings = [
> + '-Wno-address-of-packed-member',
> + '-Wno-format-truncation'
> +]
> +foreach arg: unwanted_warnings
> + if cc.has_argument(arg)
> + add_project_arguments(arg, language: 'c')
> + endif
> +endforeach
I can't fault the code here, or Meson, just noting that the disabling of these warnings causes prints on GCC 4.8.5 and GCC 5.4.0, GCC 7 only prints on -Wno-address-of-packed-member. Clang handles it correctly - and no out-of-the-ordinary prints occur. No code changes required here.
> diff --git a/config/rte_config.h b/config/rte_config.h
> new file mode 100644
> index 000000000..79b0db90f
> --- /dev/null
> +++ b/config/rte_config.h
> @@ -0,0 +1,50 @@
> +/*
> + * BSD LICENSE
> + *
> + * Copyright(c) 2016 Intel Corporation. All rights reserved.
> + * All rights reserved.
2017 for license year
<snip>
Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
On Mon, Sep 04, 2017 at 02:36:42PM +0100, Van Haaren, Harry wrote:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Friday, September 1, 2017 11:04 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure for meson & ninja builds
> >
> > To build with meson and ninja, we need some initial infrastructure in
> > place. The build files for meson always need to be called "meson.build",
> > and options get placed in meson_options.txt
> >
> > This commit adds a top-level meson.build file, which sets up the global
> > variables for tracking drivers, libraries, etc., and then includes other
> > build files, before finishing by writing the global build configuration
> > header file and a DPDK pkgconfig file at the end, using some of those same
> > globals.
> >
> > From the top level build file, the only include file thus far is for the
> > config folder, which does some other setup of global configuration
> > parameters, including pulling in architecture specific parameters from an
> > architectural subdirectory. A number of configuration build options are
> > provided for the project to tune a number of global variables which will be
> > used later e.g. max numa nodes, max cores, etc. These settings all make
> > their way to the global build config header "rte_build_config.h". There is
> > also a file "rte_config.h", which includes "rte_build_config.h", and this
> > file is meant to hold other build-time values which are present in our
> > current static build configuration but are not normally meant for
> > user-configuration. Ideally, over time, the values placed here should be
> > moved to the individual libraries or drivers which want those values.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>
> Comments inline below;
>
> <snip>
>
> > diff --git a/config/meson.build b/config/meson.build
> <snip>
> > +# disable any unwanted warnings
> > +unwanted_warnings = [
> > + '-Wno-address-of-packed-member',
> > + '-Wno-format-truncation'
> > +]
> > +foreach arg: unwanted_warnings
> > + if cc.has_argument(arg)
> > + add_project_arguments(arg, language: 'c')
> > + endif
> > +endforeach
>
> I can't fault the code here, or Meson, just noting that the disabling of these warnings causes prints on GCC 4.8.5 and GCC 5.4.0, GCC 7 only prints on -Wno-address-of-packed-member. Clang handles it correctly - and no out-of-the-ordinary prints occur. No code changes required here.
>
Strange. Just tested with gcc 4.8.5 and gcc 5.4, and everything worked as
expected, no errors or warnings.
>
> > diff --git a/config/rte_config.h b/config/rte_config.h
> > new file mode 100644
> > index 000000000..79b0db90f
> > --- /dev/null
> > +++ b/config/rte_config.h
> > @@ -0,0 +1,50 @@
> > +/*
> > + * BSD LICENSE
> > + *
> > + * Copyright(c) 2016 Intel Corporation. All rights reserved.
> > + * All rights reserved.
>
> 2017 for license year
Good catch, will fix in V2.
>
> <snip>
>
> Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
On Mon, Sep 04, 2017 at 02:51:01PM +0100, Bruce Richardson wrote:
> On Mon, Sep 04, 2017 at 02:36:42PM +0100, Van Haaren, Harry wrote:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> > > Richardson Sent: Friday, September 1, 2017 11:04 AM To:
> > > dev@dpdk.org Cc: Richardson, Bruce <bruce.richardson@intel.com>
> > > Subject: [dpdk-dev] [PATCH 01/17] build: add initial
> > > infrastructure for meson & ninja builds
> > >
> > > To build with meson and ninja, we need some initial infrastructure
> > > in place. The build files for meson always need to be called
> > > "meson.build", and options get placed in meson_options.txt
> > >
> > > This commit adds a top-level meson.build file, which sets up the
> > > global variables for tracking drivers, libraries, etc., and then
> > > includes other build files, before finishing by writing the global
> > > build configuration header file and a DPDK pkgconfig file at the
> > > end, using some of those same globals.
> > >
> > > From the top level build file, the only include file thus far is
> > > for the config folder, which does some other setup of global
> > > configuration parameters, including pulling in architecture
> > > specific parameters from an architectural subdirectory. A number
> > > of configuration build options are provided for the project to
> > > tune a number of global variables which will be used later e.g.
> > > max numa nodes, max cores, etc. These settings all make their way
> > > to the global build config header "rte_build_config.h". There is
> > > also a file "rte_config.h", which includes "rte_build_config.h",
> > > and this file is meant to hold other build-time values which are
> > > present in our current static build configuration but are not
> > > normally meant for user-configuration. Ideally, over time, the
> > > values placed here should be moved to the individual libraries or
> > > drivers which want those values.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > Comments inline below;
> >
> > <snip>
> >
> > > diff --git a/config/meson.build b/config/meson.build
> > <snip>
> > > +# disable any unwanted warnings +unwanted_warnings = [ +
> > > '-Wno-address-of-packed-member', + '-Wno-format-truncation'
> > > +] +foreach arg: unwanted_warnings + if cc.has_argument(arg)
> > > + add_project_arguments(arg, language: 'c') + endif
> > > +endforeach
> >
> > I can't fault the code here, or Meson, just noting that the
> > disabling of these warnings causes prints on GCC 4.8.5 and GCC
> > 5.4.0, GCC 7 only prints on -Wno-address-of-packed-member. Clang
> > handles it correctly - and no out-of-the-ordinary prints occur. No
> > code changes required here.
> >
>
> Strange. Just tested with gcc 4.8.5 and gcc 5.4, and everything worked
> as expected, no errors or warnings.
>
Dug into the invalid flag warnings a bit more and found a number of
items in the root cause.
First is that gcc only complains about invalid -Wno-* parameters in the
case where there is another error or warning being printed. This
explains why the warnings are only seen for some files, rather than
others.
Second is that, because of this meson, is not correctly identifying the
relevant flags as unsupported by the compiler. I'm looking to upstream
to meson a fix for this.
Third is that there are more warnings that need to be disabled for some
files when compiled with gcc 5.4. So long as there are no other problems
with the code, gcc will never print the errors about the invalid warning
disable flags.
So, for V2 of this, I hope to fix point 3.
/Bruce
On Wed, Sep 06, 2017 at 05:18:46PM +0100, Bruce Richardson wrote:
> On Mon, Sep 04, 2017 at 02:51:01PM +0100, Bruce Richardson wrote:
> > On Mon, Sep 04, 2017 at 02:36:42PM +0100, Van Haaren, Harry wrote:
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce
> > > > Richardson Sent: Friday, September 1, 2017 11:04 AM To:
> > > > dev@dpdk.org Cc: Richardson, Bruce <bruce.richardson@intel.com>
> > > > Subject: [dpdk-dev] [PATCH 01/17] build: add initial
> > > > infrastructure for meson & ninja builds
> > > >
> > > > To build with meson and ninja, we need some initial
> > > > infrastructure in place. The build files for meson always need
> > > > to be called "meson.build", and options get placed in
> > > > meson_options.txt
> > > >
> > > > This commit adds a top-level meson.build file, which sets up the
> > > > global variables for tracking drivers, libraries, etc., and then
> > > > includes other build files, before finishing by writing the
> > > > global build configuration header file and a DPDK pkgconfig file
> > > > at the end, using some of those same globals.
> > > >
> > > > From the top level build file, the only include file thus far is
> > > > for the config folder, which does some other setup of global
> > > > configuration parameters, including pulling in architecture
> > > > specific parameters from an architectural subdirectory. A number
> > > > of configuration build options are provided for the project to
> > > > tune a number of global variables which will be used later e.g.
> > > > max numa nodes, max cores, etc. These settings all make their
> > > > way to the global build config header "rte_build_config.h".
> > > > There is also a file "rte_config.h", which includes
> > > > "rte_build_config.h", and this file is meant to hold other
> > > > build-time values which are present in our current static build
> > > > configuration but are not normally meant for user-configuration.
> > > > Ideally, over time, the values placed here should be moved to
> > > > the individual libraries or drivers which want those values.
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > Comments inline below;
> > >
> > > <snip>
> > >
> > > > diff --git a/config/meson.build b/config/meson.build
> > > <snip>
> > > > +# disable any unwanted warnings +unwanted_warnings = [ +
> > > > '-Wno-address-of-packed-member', + '-Wno-format-truncation'
> > > > +] +foreach arg: unwanted_warnings + if cc.has_argument(arg)
> > > > + add_project_arguments(arg, language: 'c') +
> > > > endif +endforeach
> > >
> > > I can't fault the code here, or Meson, just noting that the
> > > disabling of these warnings causes prints on GCC 4.8.5 and GCC
> > > 5.4.0, GCC 7 only prints on -Wno-address-of-packed-member. Clang
> > > handles it correctly - and no out-of-the-ordinary prints occur. No
> > > code changes required here.
> > >
> >
> > Strange. Just tested with gcc 4.8.5 and gcc 5.4, and everything
> > worked as expected, no errors or warnings.
> >
> Dug into the invalid flag warnings a bit more and found a number of
> items in the root cause.
>
> First is that gcc only complains about invalid -Wno-* parameters in
> the case where there is another error or warning being printed. This
> explains why the warnings are only seen for some files, rather than
> others. Second is that, because of this meson, is not correctly
> identifying the relevant flags as unsupported by the compiler. I'm
> looking to upstream to meson a fix for this. Third is that there are
> more warnings that need to be disabled for some files when compiled
> with gcc 5.4. So long as there are no other problems with the code,
> gcc will never print the errors about the invalid warning disable
> flags.
>
> So, for V2 of this, I hope to fix point 3.
>
[Hopefully] Final update here. It appears that this is an issue that
only appears on Ubuntu, as I can't reproduce it on Fedora with any of a
number of gcc version when I turn on -Wunused-result flag. I believe
that this may be due to extra hardening in the standard headers that is
done on Ubuntu to mark the functions as requiring the extra checking. To
fix these, rather than disable the warnings - something that may hide
other problems, I've sent two small patches [1] [2] to add return value
checks to the code as applicable.
/Bruce
[1] http://dpdk.org/dev/patchwork/patch/28457/
[2] http://dpdk.org/dev/patchwork/patch/28458/
On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
> To build with meson and ninja, we need some initial infrastructure in
> place. The build files for meson always need to be called "meson.build",
> and options get placed in meson_options.txt
>
> This commit adds a top-level meson.build file, which sets up the global
> variables for tracking drivers, libraries, etc., and then includes other
> build files, before finishing by writing the global build configuration
> header file and a DPDK pkgconfig file at the end, using some of those same
> globals.
>
> >From the top level build file, the only include file thus far is for the
> config folder, which does some other setup of global configuration
> parameters, including pulling in architecture specific parameters from an
> architectural subdirectory. A number of configuration build options are
> provided for the project to tune a number of global variables which will be
> used later e.g. max numa nodes, max cores, etc. These settings all make
> their way to the global build config header "rte_build_config.h". There is
> also a file "rte_config.h", which includes "rte_build_config.h", and this
> file is meant to hold other build-time values which are present in our
> current static build configuration but are not normally meant for
> user-configuration. Ideally, over time, the values placed here should be
> moved to the individual libraries or drivers which want those values.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
I feel like I need to underscore my previous concern here. While I'm not
opposed per-se to a new build system, I am very concerned about the burden that
switching places on downstream consumers, in particular distributions (since I
represent one of them). Moving to a new build system with new tools means those
tools need to be packaged, tested and shipped, which is a significant work
effort. While it might be a net gain long term, its something you need to keep
in mind when making these changes.
I know you've said that we will be keepting the existing build system, I just
need to be sure everyone understands just how important that is.
Though perhaps the time frame for keeping the current build system as priarmy is
less concerning, as feature parity is even more critical. That is to say, the
new build system must be able to produce the same configurations that the
current build system does. Without it I don't think anyone will be able to use
it consistently, and that will leave a great number of users in a very poor
position. I think getting a little closer to parity with the current system is
warranted. I'd suggest as a gating factor:
1) Building on all supported arches
2) Cross building on all supported arches
3) Proper identification of targeted machine (i.e. equivalent of the machine
component of the current build system)
Specific notes inline
> ---
> config/meson.build | 69 +++++++++++++++++++++++++++++++++++++++++
> config/rte_config.h | 50 ++++++++++++++++++++++++++++++
> config/x86/meson.build | 70 ++++++++++++++++++++++++++++++++++++++++++
> meson.build | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
> meson_options.txt | 6 ++++
> 5 files changed, 278 insertions(+)
> create mode 100644 config/meson.build
> create mode 100644 config/rte_config.h
> create mode 100644 config/x86/meson.build
> create mode 100644 meson.build
> create mode 100644 meson_options.txt
>
> diff --git a/config/meson.build b/config/meson.build
> new file mode 100644
> index 000000000..3a6bcc58d
> --- /dev/null
> +++ b/config/meson.build
> @@ -0,0 +1,69 @@
> +# BSD LICENSE
> +#
> +# Copyright(c) 2017 Intel Corporation. All rights reserved.
> +# All rights reserved.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions
> +# are met:
> +#
> +# * Redistributions of source code must retain the above copyright
> +# notice, this list of conditions and the following disclaimer.
> +# * Redistributions in binary form must reproduce the above copyright
> +# notice, this list of conditions and the following disclaimer in
> +# the documentation and/or other materials provided with the
> +# distribution.
> +# * Neither the name of Intel Corporation nor the names of its
> +# contributors may be used to endorse or promote products derived
> +# from this software without specific prior written permission.
> +#
> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +
> +# set the machine type and cflags for it
> +machine = get_option('machine')
> +dpdk_conf.set('RTE_MACHINE', machine)
> +add_project_arguments('-march=@0@'.format(machine), language: 'c')
So, in the current build system, arch defined the process architecture, while
'machine' defined the specific processor family (nhm, ivb, etc). This seems
like you are merging those two concepts together. While that seems reasonable,
is that going to be workable with non-x86 architectures?
Have you considered using the cross-script option in meson to define a per arch
build file? That I think would eliminate some of this top level parsing of arch
options
> +# some libs depend on maths lib
> +add_project_link_arguments('-lm', language: 'c')
> +
> +# add -include rte_config to cflags
> +add_project_arguments('-include', 'rte_config.h', language: 'c')
> +
> +# disable any unwanted warnings
> +unwanted_warnings = [
> + '-Wno-address-of-packed-member',
> + '-Wno-format-truncation'
> +]
> +foreach arg: unwanted_warnings
> + if cc.has_argument(arg)
> + add_project_arguments(arg, language: 'c')
> + endif
> +endforeach
> +
> +compile_time_cpuflags = []
> +if host_machine.cpu_family().startswith('x86')
> + arch_subdir = 'x86'
> + subdir(arch_subdir)
> +endif
> +dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
> +
Likewise, I think if you use the --cross-script approach, this logic gets
eliminated in favor of a file pointer from the command line
<snip>
> +
> +# set up some global vars for compiler, platform, configuration, etc.
> +cc = meson.get_compiler('c')
> +dpdk_conf = configuration_data()
> +dpdk_libraries = []
> +dpdk_drivers = []
> +dpdk_extra_ldflags = []
> +
> +# for static libs, treat the drivers as regular libraries, otherwise
> +# for shared libs, put them in a driver folder
> +if get_option('default_library') == 'static'
> + driver_install_path = get_option('libdir')
> +else
> + driver_install_path = '@0@/dpdk/drivers'.format(get_option('prefix'))
> +endif
> +
So, I like this, as it appears to default to shared library builds, which is
great. Unfortunately, it doesn't seem to work for me when using this command:
meson -Ddefault_library=static -Dlibdir=./build/lib . build
If I do that and then run ninja in my build directory, I still get DSO's not
static libraries. I am assuming that I'm doing something subtly wrong in my
build, but I can't seem to see what it is.
On the other hand, if static builds don't work yet, thats going to be an issue.
<snip>
> +# configure the build, and make sure configs here and in config folder are
> +# able to be included in any file. We also store a global array of include dirs
> +# for passing to pmdinfogen scripts
> +global_inc = include_directories('.', 'config')
> +subdir('config')
> +
> +# TODO build libs and drivers
> +
> +# TODO build binaries and installable tools
> +
This seems outdated, but I think you remove it in a later patch
Neil
> On Sep 7, 2017, at 9:21 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
>> To build with meson and ninja, we need some initial infrastructure in
>> place. The build files for meson always need to be called "meson.build",
>> and options get placed in meson_options.txt
>>
>> This commit adds a top-level meson.build file, which sets up the global
>> variables for tracking drivers, libraries, etc., and then includes other
>> build files, before finishing by writing the global build configuration
>> header file and a DPDK pkgconfig file at the end, using some of those same
>> globals.
>>
>>> From the top level build file, the only include file thus far is for the
>> config folder, which does some other setup of global configuration
>> parameters, including pulling in architecture specific parameters from an
>> architectural subdirectory. A number of configuration build options are
>> provided for the project to tune a number of global variables which will be
>> used later e.g. max numa nodes, max cores, etc. These settings all make
>> their way to the global build config header "rte_build_config.h". There is
>> also a file "rte_config.h", which includes "rte_build_config.h", and this
>> file is meant to hold other build-time values which are present in our
>> current static build configuration but are not normally meant for
>> user-configuration. Ideally, over time, the values placed here should be
>> moved to the individual libraries or drivers which want those values.
>>
>> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
>> Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> I feel like I need to underscore my previous concern here. While I'm not
> opposed per-se to a new build system, I am very concerned about the burden that
> switching places on downstream consumers, in particular distributions (since I
> represent one of them). Moving to a new build system with new tools means those
> tools need to be packaged, tested and shipped, which is a significant work
> effort. While it might be a net gain long term, its something you need to keep
> in mind when making these changes.
>
> I know you've said that we will be keepting the existing build system, I just
> need to be sure everyone understands just how important that is.
>
> Though perhaps the time frame for keeping the current build system as priarmy is
> less concerning, as feature parity is even more critical. That is to say, the
> new build system must be able to produce the same configurations that the
> current build system does. Without it I don't think anyone will be able to use
> it consistently, and that will leave a great number of users in a very poor
> position. I think getting a little closer to parity with the current system is
> warranted. I'd suggest as a gating factor:
>
> 1) Building on all supported arches
> 2) Cross building on all supported arches
> 3) Proper identification of targeted machine (i.e. equivalent of the machine
> component of the current build system)
I think your concerns are important and we have to keep the current build system even after the new build system is at parity with the current one. We most likely will have to keep the current build system around for a while like year or more as it will be hard for all distros to convert and add the needed tools to build with DPDK. The problem will be making sure changes in one are reflected in the other build system.
The new build system has a lot of advantages and Bruce is doing a good job, but we need to open it up into a project for more to contribute, which I assume is the goal here.
>
> Specific notes inline
>
>> ---
>> config/meson.build | 69 +++++++++++++++++++++++++++++++++++++++++
>> config/rte_config.h | 50 ++++++++++++++++++++++++++++++
>> config/x86/meson.build | 70 ++++++++++++++++++++++++++++++++++++++++++
>> meson.build | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> meson_options.txt | 6 ++++
>> 5 files changed, 278 insertions(+)
>> create mode 100644 config/meson.build
>> create mode 100644 config/rte_config.h
>> create mode 100644 config/x86/meson.build
>> create mode 100644 meson.build
>> create mode 100644 meson_options.txt
>>
>> diff --git a/config/meson.build b/config/meson.build
>> new file mode 100644
>> index 000000000..3a6bcc58d
>> --- /dev/null
>> +++ b/config/meson.build
>> @@ -0,0 +1,69 @@
>> +# BSD LICENSE
>> +#
>> +# Copyright(c) 2017 Intel Corporation. All rights reserved.
>> +# All rights reserved.
>> +#
>> +# Redistribution and use in source and binary forms, with or without
>> +# modification, are permitted provided that the following conditions
>> +# are met:
>> +#
>> +# * Redistributions of source code must retain the above copyright
>> +# notice, this list of conditions and the following disclaimer.
>> +# * Redistributions in binary form must reproduce the above copyright
>> +# notice, this list of conditions and the following disclaimer in
>> +# the documentation and/or other materials provided with the
>> +# distribution.
>> +# * Neither the name of Intel Corporation nor the names of its
>> +# contributors may be used to endorse or promote products derived
>> +# from this software without specific prior written permission.
>> +#
>> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>> +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>> +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>> +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> +
>> +# set the machine type and cflags for it
>> +machine = get_option('machine')
>> +dpdk_conf.set('RTE_MACHINE', machine)
>> +add_project_arguments('-march=@0@'.format(machine), language: 'c')
> So, in the current build system, arch defined the process architecture, while
> 'machine' defined the specific processor family (nhm, ivb, etc). This seems
> like you are merging those two concepts together. While that seems reasonable,
> is that going to be workable with non-x86 architectures?
>
> Have you considered using the cross-script option in meson to define a per arch
> build file? That I think would eliminate some of this top level parsing of arch
> options
>
>> +# some libs depend on maths lib
>> +add_project_link_arguments('-lm', language: 'c')
>> +
>> +# add -include rte_config to cflags
>> +add_project_arguments('-include', 'rte_config.h', language: 'c')
>> +
>> +# disable any unwanted warnings
>> +unwanted_warnings = [
>> + '-Wno-address-of-packed-member',
>> + '-Wno-format-truncation'
>> +]
>> +foreach arg: unwanted_warnings
>> + if cc.has_argument(arg)
>> + add_project_arguments(arg, language: 'c')
>> + endif
>> +endforeach
>> +
>> +compile_time_cpuflags = []
>> +if host_machine.cpu_family().startswith('x86')
>> + arch_subdir = 'x86'
>> + subdir(arch_subdir)
>> +endif
>> +dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
>> +
> Likewise, I think if you use the --cross-script approach, this logic gets
> eliminated in favor of a file pointer from the command line
>
> <snip>
>
>
>> +
>> +# set up some global vars for compiler, platform, configuration, etc.
>> +cc = meson.get_compiler('c')
>> +dpdk_conf = configuration_data()
>> +dpdk_libraries = []
>> +dpdk_drivers = []
>> +dpdk_extra_ldflags = []
>> +
>> +# for static libs, treat the drivers as regular libraries, otherwise
>> +# for shared libs, put them in a driver folder
>> +if get_option('default_library') == 'static'
>> + driver_install_path = get_option('libdir')
>> +else
>> + driver_install_path = '@0@/dpdk/drivers'.format(get_option('prefix'))
>> +endif
>> +
> So, I like this, as it appears to default to shared library builds, which is
> great. Unfortunately, it doesn't seem to work for me when using this command:
>
> meson -Ddefault_library=static -Dlibdir=./build/lib . build
In a previous version of the build system I just edited the meson_option.txt file can changed it to static and that worked. The command line should work, but have not tried that option.
>
> If I do that and then run ninja in my build directory, I still get DSO's not
> static libraries. I am assuming that I'm doing something subtly wrong in my
> build, but I can't seem to see what it is.
>
> On the other hand, if static builds don't work yet, thats going to be an issue.
>
> <snip>
>
>> +# configure the build, and make sure configs here and in config folder are
>> +# able to be included in any file. We also store a global array of include dirs
>> +# for passing to pmdinfogen scripts
>> +global_inc = include_directories('.', 'config')
>> +subdir('config')
>> +
>> +# TODO build libs and drivers
>> +
>> +# TODO build binaries and installable tools
>> +
> This seems outdated, but I think you remove it in a later patch
>
> Neil
Regards,
Keith
On Thu, Sep 07, 2017 at 04:47:00PM +0000, Wiles, Keith wrote:
>
> > On Sep 7, 2017, at 9:21 AM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
> >> To build with meson and ninja, we need some initial infrastructure in
> >> place. The build files for meson always need to be called "meson.build",
> >> and options get placed in meson_options.txt
> >>
> >> This commit adds a top-level meson.build file, which sets up the global
> >> variables for tracking drivers, libraries, etc., and then includes other
> >> build files, before finishing by writing the global build configuration
> >> header file and a DPDK pkgconfig file at the end, using some of those same
> >> globals.
> >>
> >>> From the top level build file, the only include file thus far is for the
> >> config folder, which does some other setup of global configuration
> >> parameters, including pulling in architecture specific parameters from an
> >> architectural subdirectory. A number of configuration build options are
> >> provided for the project to tune a number of global variables which will be
> >> used later e.g. max numa nodes, max cores, etc. These settings all make
> >> their way to the global build config header "rte_build_config.h". There is
> >> also a file "rte_config.h", which includes "rte_build_config.h", and this
> >> file is meant to hold other build-time values which are present in our
> >> current static build configuration but are not normally meant for
> >> user-configuration. Ideally, over time, the values placed here should be
> >> moved to the individual libraries or drivers which want those values.
> >>
> >> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> >> Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > I feel like I need to underscore my previous concern here. While I'm not
> > opposed per-se to a new build system, I am very concerned about the burden that
> > switching places on downstream consumers, in particular distributions (since I
> > represent one of them). Moving to a new build system with new tools means those
> > tools need to be packaged, tested and shipped, which is a significant work
> > effort. While it might be a net gain long term, its something you need to keep
> > in mind when making these changes.
> >
> > I know you've said that we will be keepting the existing build system, I just
> > need to be sure everyone understands just how important that is.
> >
> > Though perhaps the time frame for keeping the current build system as priarmy is
> > less concerning, as feature parity is even more critical. That is to say, the
> > new build system must be able to produce the same configurations that the
> > current build system does. Without it I don't think anyone will be able to use
> > it consistently, and that will leave a great number of users in a very poor
> > position. I think getting a little closer to parity with the current system is
> > warranted. I'd suggest as a gating factor:
> >
> > 1) Building on all supported arches
> > 2) Cross building on all supported arches
> > 3) Proper identification of targeted machine (i.e. equivalent of the machine
> > component of the current build system)
>
> I think your concerns are important and we have to keep the current build system even after the new build system is at parity with the current one. We most likely will have to keep the current build system around for a while like year or more as it will be hard for all distros to convert and add the needed tools to build with DPDK. The problem will be making sure changes in one are reflected in the other build system.
>
> The new build system has a lot of advantages and Bruce is doing a good job, but we need to open it up into a project for more to contribute, which I assume is the goal here.
>
No argument, Bruce is doing a fine job here, and while I don't really agree with
the choice of tools, I have to admit its fast and somewhat slick. But I do feel
strongly about being very careful with how we procede in its introduction.
Neil
On Thu, Sep 07, 2017 at 12:21:57PM -0400, Neil Horman wrote:
> On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
> > To build with meson and ninja, we need some initial infrastructure in
> > place. The build files for meson always need to be called "meson.build",
> > and options get placed in meson_options.txt
> >
> > This commit adds a top-level meson.build file, which sets up the global
> > variables for tracking drivers, libraries, etc., and then includes other
> > build files, before finishing by writing the global build configuration
> > header file and a DPDK pkgconfig file at the end, using some of those same
> > globals.
> >
> > >From the top level build file, the only include file thus far is for the
> > config folder, which does some other setup of global configuration
> > parameters, including pulling in architecture specific parameters from an
> > architectural subdirectory. A number of configuration build options are
> > provided for the project to tune a number of global variables which will be
> > used later e.g. max numa nodes, max cores, etc. These settings all make
> > their way to the global build config header "rte_build_config.h". There is
> > also a file "rte_config.h", which includes "rte_build_config.h", and this
> > file is meant to hold other build-time values which are present in our
> > current static build configuration but are not normally meant for
> > user-configuration. Ideally, over time, the values placed here should be
> > moved to the individual libraries or drivers which want those values.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
>
> I feel like I need to underscore my previous concern here. While I'm not
> opposed per-se to a new build system, I am very concerned about the burden that
> switching places on downstream consumers, in particular distributions (since I
> represent one of them). Moving to a new build system with new tools means those
> tools need to be packaged, tested and shipped, which is a significant work
> effort. While it might be a net gain long term, its something you need to keep
> in mind when making these changes.
>
Understood. If there is anything we/I can do to make this transition
easier, please flag it for consideration.
> I know you've said that we will be keepting the existing build system,
> I just need to be sure everyone understands just how important that
> is.
>
What is your feeling here, in terms of timescale. After any new system
reaches feature parity, how long would you estimate that we would need
to support the existing makefile system before it would be safe to
deprecate it? Should we start a deprecation plan, or is it best just to
commit to support both until we get all - or almost all - downstream
consumers switched over? While I wouldn't push for deprecating the old
system any time soon, and I wouldn't consider maintaining the two
unduly burdensome, it's not something we want to do in the long term.
> Though perhaps the time frame for keeping the current build system as priarmy is
> less concerning, as feature parity is even more critical. That is to say, the
> new build system must be able to produce the same configurations that the
> current build system does. Without it I don't think anyone will be able to use
> it consistently, and that will leave a great number of users in a very poor
> position. I think getting a little closer to parity with the current system is
> warranted. I'd suggest as a gating factor:
>
> 1) Building on all supported arches
> 2) Cross building on all supported arches
> 3) Proper identification of targeted machine (i.e. equivalent of the machine
> component of the current build system)
>
The question there is gating factor for what? Presumably not for merging
into the staging tree. But for merging into the main tree for releases?
I'd push back a little on that, as the new system does not interfere in
any way with the old, and by keeping it in a staging tree until it
reaches full feature parity will make the job considerably harder. For
example, it means that anyone submitting a new driver or library has to
submit the code and makefiles in one set and the meson patches in a
separate one for a separate build tree. It also makes it less likely
that people will try out the new system and find the issues with it, and
help fill in the gaps. While I can understand us not recommending the
new build system until it reaches feature parity, I think there are a
lot of benefits to be got by making it widely available, even if it's
incomplete.
On a semi-related note, some folks here are chomping at the bit for it
to get mainlined, as they want the improved build time for recompiles
for speed up their development process. They can work off the posted
patches, but it's more painful than having it locked in.
> Specific notes inline
>
> > ---
> > config/meson.build | 69 +++++++++++++++++++++++++++++++++++++++++
> > config/rte_config.h | 50 ++++++++++++++++++++++++++++++
> > config/x86/meson.build | 70 ++++++++++++++++++++++++++++++++++++++++++
> > meson.build | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > meson_options.txt | 6 ++++
> > 5 files changed, 278 insertions(+)
> > create mode 100644 config/meson.build
> > create mode 100644 config/rte_config.h
> > create mode 100644 config/x86/meson.build
> > create mode 100644 meson.build
> > create mode 100644 meson_options.txt
> >
> > diff --git a/config/meson.build b/config/meson.build
> > new file mode 100644
> > index 000000000..3a6bcc58d
> > --- /dev/null
> > +++ b/config/meson.build
> > @@ -0,0 +1,69 @@
> > +# BSD LICENSE
> > +#
> > +# Copyright(c) 2017 Intel Corporation. All rights reserved.
> > +# All rights reserved.
> > +#
> > +# Redistribution and use in source and binary forms, with or without
> > +# modification, are permitted provided that the following conditions
> > +# are met:
> > +#
> > +# * Redistributions of source code must retain the above copyright
> > +# notice, this list of conditions and the following disclaimer.
> > +# * Redistributions in binary form must reproduce the above copyright
> > +# notice, this list of conditions and the following disclaimer in
> > +# the documentation and/or other materials provided with the
> > +# distribution.
> > +# * Neither the name of Intel Corporation nor the names of its
> > +# contributors may be used to endorse or promote products derived
> > +# from this software without specific prior written permission.
> > +#
> > +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > +
> > +# set the machine type and cflags for it
> > +machine = get_option('machine')
> > +dpdk_conf.set('RTE_MACHINE', machine)
> > +add_project_arguments('-march=@0@'.format(machine), language: 'c')
> So, in the current build system, arch defined the process architecture, while
> 'machine' defined the specific processor family (nhm, ivb, etc). This seems
> like you are merging those two concepts together. While that seems reasonable,
> is that going to be workable with non-x86 architectures?
>
I'm not sure I am, but I'm not familiar enough with other architectures
to be sure. I'd appreciate some feedback from those more familiar with
ARM/PPC to help support the effort to be sure.
For now, in this set, the machine value is being used as now on IA, a
tuning flag to be passed to the compiler. The actual architecture is
pulled from host_machine.cpu_family() - which is the final target
machine the case of a cross build.
> Have you considered using the cross-script option in meson to define a per arch
> build file? That I think would eliminate some of this top level parsing of arch
> options
>
No, I haven't looked into that yet, but I will do so shortly. I'd still
look to get this in as a starting baseline and then modify it as
necessary.
> > +# some libs depend on maths lib
> > +add_project_link_arguments('-lm', language: 'c')
> > +
> > +# add -include rte_config to cflags
> > +add_project_arguments('-include', 'rte_config.h', language: 'c')
> > +
> > +# disable any unwanted warnings
> > +unwanted_warnings = [
> > + '-Wno-address-of-packed-member',
> > + '-Wno-format-truncation'
> > +]
> > +foreach arg: unwanted_warnings
> > + if cc.has_argument(arg)
> > + add_project_arguments(arg, language: 'c')
> > + endif
> > +endforeach
> > +
> > +compile_time_cpuflags = []
> > +if host_machine.cpu_family().startswith('x86')
> > + arch_subdir = 'x86'
> > + subdir(arch_subdir)
> > +endif
> > +dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
> > +
> Likewise, I think if you use the --cross-script approach, this logic gets
> eliminated in favor of a file pointer from the command line
>
> <snip>
>
I'd rather not force the use of a cross-script if not cross-compiling.
>
> > +
> > +# set up some global vars for compiler, platform, configuration, etc.
> > +cc = meson.get_compiler('c')
> > +dpdk_conf = configuration_data()
> > +dpdk_libraries = []
> > +dpdk_drivers = []
> > +dpdk_extra_ldflags = []
> > +
> > +# for static libs, treat the drivers as regular libraries, otherwise
> > +# for shared libs, put them in a driver folder
> > +if get_option('default_library') == 'static'
> > + driver_install_path = get_option('libdir')
> > +else
> > + driver_install_path = '@0@/dpdk/drivers'.format(get_option('prefix'))
> > +endif
> > +
> So, I like this, as it appears to default to shared library builds, which is
> great. Unfortunately, it doesn't seem to work for me when using this command:
>
> meson -Ddefault_library=static -Dlibdir=./build/lib . build
>
meson --default-library=static ...
> If I do that and then run ninja in my build directory, I still get DSO's not
> static libraries. I am assuming that I'm doing something subtly wrong in my
> build, but I can't seem to see what it is.
>
> On the other hand, if static builds don't work yet, thats going to be an issue.
>
> <snip>
>
> > +# configure the build, and make sure configs here and in config folder are
> > +# able to be included in any file. We also store a global array of include dirs
> > +# for passing to pmdinfogen scripts
> > +global_inc = include_directories('.', 'config')
> > +subdir('config')
> > +
> > +# TODO build libs and drivers
> > +
> > +# TODO build binaries and installable tools
> > +
> This seems outdated, but I think you remove it in a later patch
>
Yep. I put them in as comment placeholders in the early patches to show
where things would go in later ones to try and make the flow clearer
i.e. they were deliberately put in, even though removed later.
Thanks for the feedback.
/Bruce
On Fri, Sep 08, 2017 at 09:50:26AM +0100, Bruce Richardson wrote:
> On Thu, Sep 07, 2017 at 12:21:57PM -0400, Neil Horman wrote:
> > On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
> > > To build with meson and ninja, we need some initial infrastructure in
> > > place. The build files for meson always need to be called "meson.build",
> > > and options get placed in meson_options.txt
> > >
> > > This commit adds a top-level meson.build file, which sets up the global
> > > variables for tracking drivers, libraries, etc., and then includes other
> > > build files, before finishing by writing the global build configuration
> > > header file and a DPDK pkgconfig file at the end, using some of those same
> > > globals.
> > >
> > > >From the top level build file, the only include file thus far is for the
> > > config folder, which does some other setup of global configuration
> > > parameters, including pulling in architecture specific parameters from an
> > > architectural subdirectory. A number of configuration build options are
> > > provided for the project to tune a number of global variables which will be
> > > used later e.g. max numa nodes, max cores, etc. These settings all make
> > > their way to the global build config header "rte_build_config.h". There is
> > > also a file "rte_config.h", which includes "rte_build_config.h", and this
> > > file is meant to hold other build-time values which are present in our
> > > current static build configuration but are not normally meant for
> > > user-configuration. Ideally, over time, the values placed here should be
> > > moved to the individual libraries or drivers which want those values.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> >
> > I feel like I need to underscore my previous concern here. While I'm not
> > opposed per-se to a new build system, I am very concerned about the burden that
> > switching places on downstream consumers, in particular distributions (since I
> > represent one of them). Moving to a new build system with new tools means those
> > tools need to be packaged, tested and shipped, which is a significant work
> > effort. While it might be a net gain long term, its something you need to keep
> > in mind when making these changes.
> >
> Understood. If there is anything we/I can do to make this transition
> easier, please flag it for consideration.
>
Thank you, I appreciate that.
> > I know you've said that we will be keepting the existing build system,
> > I just need to be sure everyone understands just how important that
> > is.
> >
> What is your feeling here, in terms of timescale. After any new system
> reaches feature parity, how long would you estimate that we would need
> to support the existing makefile system before it would be safe to
> deprecate it? Should we start a deprecation plan, or is it best just to
> commit to support both until we get all - or almost all - downstream
> consumers switched over? While I wouldn't push for deprecating the old
> system any time soon, and I wouldn't consider maintaining the two
> unduly burdensome, it's not something we want to do in the long term.
>
I was hoping to avoid putting a specific time frame on it, but its a fair
question to ask. I feel like any particular timetable is somewhat arbitrary.
Keith suggested a year, which is likely as good as any in my mind. To put a bit
more detail behind it, a RHEL release cycle is anywhere from 6 to 18 months, so
a year fits well. If we assume starting a few weeks back when you first
proposed this change, that its going to be merged, that gives us time to package
the build components, build the new package using them, get it through a qa
cycle and fix anything that pops up as a result. That way, when the switch is
made, it can be done with an immediate deprecation of the old build system with
a level of confidence that some of the more esoteric build targets/configs will
likely work.
> > Though perhaps the time frame for keeping the current build system as priarmy is
> > less concerning, as feature parity is even more critical. That is to say, the
> > new build system must be able to produce the same configurations that the
> > current build system does. Without it I don't think anyone will be able to use
> > it consistently, and that will leave a great number of users in a very poor
> > position. I think getting a little closer to parity with the current system is
> > warranted. I'd suggest as a gating factor:
> >
> > 1) Building on all supported arches
> > 2) Cross building on all supported arches
> > 3) Proper identification of targeted machine (i.e. equivalent of the machine
> > component of the current build system)
> >
> The question there is gating factor for what? Presumably not for merging
> into the staging tree. But for merging into the main tree for releases?
> I'd push back a little on that, as the new system does not interfere in
> any way with the old, and by keeping it in a staging tree until it
> reaches full feature parity will make the job considerably harder. For
> example, it means that anyone submitting a new driver or library has to
> submit the code and makefiles in one set and the meson patches in a
> separate one for a separate build tree. It also makes it less likely
> that people will try out the new system and find the issues with it, and
> help fill in the gaps. While I can understand us not recommending the
> new build system until it reaches feature parity, I think there are a
> lot of benefits to be got by making it widely available, even if it's
> incomplete.
Yes, sorry, the implied "what" here is gating its introduction to mainline. I
have no problem with this going into a development or staging branch/tree, only
with it getting merged to mainline and becoming the primary build system today.
I get that it makes reaching feature parity harder, but to not do so relegates
anyone that hasn't had a chance to test the new build system to second class
citizen status (or at least potentially does so). To be a bit more specific, I
can see how energized you might be to get this in place now because you've
tested it on a wide array on intel hardware, but I'm guessing if it went in
today, people at IBM and Linaro would have to drop active developement to start
switching their build environments over to the new system lest they get left out
in the cold. I think its more about balacing where the hardship lies here.
As I'm writing this, I wonder if a reasonable compromise couldn't involve the
use of CI? That is to say, what if we integrated the build system now-ish, and
stood up an official CI instance, that both:
1) built the dpdk in all style configuration, which mandated the use of the old
build system (i.e. we implicitly mandate that the current build system stay
working, and is not forgotten), and gate patch merges on that result.
2) add a test that any change to a meson file in mainline also include a change
to the a Makefile
I'm just spitballing here, but I'm looking for ways to enforce the continued use
of the current build system above and beyond a verbal promise to do so. The
idea is to ensure that it stays operational and primary to the development of
dpdk until build system parity is reached.
>
> On a semi-related note, some folks here are chomping at the bit for it
> to get mainlined, as they want the improved build time for recompiles
> for speed up their development process. They can work off the posted
> patches, but it's more painful than having it locked in.
>
I'm sure they are, but they are just one segment of the community. While its
faster for them, it also potentially causes anyone not on that platform a
headache, because they have to figure out how to integrate the new system to
their environment.
> > Specific notes inline
> >
> > > ---
> > > config/meson.build | 69 +++++++++++++++++++++++++++++++++++++++++
> > > config/rte_config.h | 50 ++++++++++++++++++++++++++++++
> > > config/x86/meson.build | 70 ++++++++++++++++++++++++++++++++++++++++++
> > > meson.build | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++
> > > meson_options.txt | 6 ++++
> > > 5 files changed, 278 insertions(+)
> > > create mode 100644 config/meson.build
> > > create mode 100644 config/rte_config.h
> > > create mode 100644 config/x86/meson.build
> > > create mode 100644 meson.build
> > > create mode 100644 meson_options.txt
> > >
> > > diff --git a/config/meson.build b/config/meson.build
> > > new file mode 100644
> > > index 000000000..3a6bcc58d
> > > --- /dev/null
> > > +++ b/config/meson.build
> > > @@ -0,0 +1,69 @@
> > > +# BSD LICENSE
> > > +#
> > > +# Copyright(c) 2017 Intel Corporation. All rights reserved.
> > > +# All rights reserved.
> > > +#
> > > +# Redistribution and use in source and binary forms, with or without
> > > +# modification, are permitted provided that the following conditions
> > > +# are met:
> > > +#
> > > +# * Redistributions of source code must retain the above copyright
> > > +# notice, this list of conditions and the following disclaimer.
> > > +# * Redistributions in binary form must reproduce the above copyright
> > > +# notice, this list of conditions and the following disclaimer in
> > > +# the documentation and/or other materials provided with the
> > > +# distribution.
> > > +# * Neither the name of Intel Corporation nor the names of its
> > > +# contributors may be used to endorse or promote products derived
> > > +# from this software without specific prior written permission.
> > > +#
> > > +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > > +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > > +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > > +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > > +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > > +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > > +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > > +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > > +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > > +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > > +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > > +
> > > +# set the machine type and cflags for it
> > > +machine = get_option('machine')
> > > +dpdk_conf.set('RTE_MACHINE', machine)
> > > +add_project_arguments('-march=@0@'.format(machine), language: 'c')
> > So, in the current build system, arch defined the process architecture, while
> > 'machine' defined the specific processor family (nhm, ivb, etc). This seems
> > like you are merging those two concepts together. While that seems reasonable,
> > is that going to be workable with non-x86 architectures?
> >
> I'm not sure I am, but I'm not familiar enough with other architectures
> to be sure. I'd appreciate some feedback from those more familiar with
> ARM/PPC to help support the effort to be sure.
>
> For now, in this set, the machine value is being used as now on IA, a
> tuning flag to be passed to the compiler. The actual architecture is
> pulled from host_machine.cpu_family() - which is the final target
> machine the case of a cross build.
>
> > Have you considered using the cross-script option in meson to define a per arch
> > build file? That I think would eliminate some of this top level parsing of arch
> > options
> >
>
> No, I haven't looked into that yet, but I will do so shortly. I'd still
> look to get this in as a starting baseline and then modify it as
> necessary.
>
> > > +# some libs depend on maths lib
> > > +add_project_link_arguments('-lm', language: 'c')
> > > +
> > > +# add -include rte_config to cflags
> > > +add_project_arguments('-include', 'rte_config.h', language: 'c')
> > > +
> > > +# disable any unwanted warnings
> > > +unwanted_warnings = [
> > > + '-Wno-address-of-packed-member',
> > > + '-Wno-format-truncation'
> > > +]
> > > +foreach arg: unwanted_warnings
> > > + if cc.has_argument(arg)
> > > + add_project_arguments(arg, language: 'c')
> > > + endif
> > > +endforeach
> > > +
> > > +compile_time_cpuflags = []
> > > +if host_machine.cpu_family().startswith('x86')
> > > + arch_subdir = 'x86'
> > > + subdir(arch_subdir)
> > > +endif
> > > +dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
> > > +
> > Likewise, I think if you use the --cross-script approach, this logic gets
> > eliminated in favor of a file pointer from the command line
> >
> > <snip>
> >
> I'd rather not force the use of a cross-script if not cross-compiling.
>
> >
> > > +
> > > +# set up some global vars for compiler, platform, configuration, etc.
> > > +cc = meson.get_compiler('c')
> > > +dpdk_conf = configuration_data()
> > > +dpdk_libraries = []
> > > +dpdk_drivers = []
> > > +dpdk_extra_ldflags = []
> > > +
> > > +# for static libs, treat the drivers as regular libraries, otherwise
> > > +# for shared libs, put them in a driver folder
> > > +if get_option('default_library') == 'static'
> > > + driver_install_path = get_option('libdir')
> > > +else
> > > + driver_install_path = '@0@/dpdk/drivers'.format(get_option('prefix'))
> > > +endif
> > > +
> > So, I like this, as it appears to default to shared library builds, which is
> > great. Unfortunately, it doesn't seem to work for me when using this command:
> >
> > meson -Ddefault_library=static -Dlibdir=./build/lib . build
> >
>
> meson --default-library=static ...
>
> > If I do that and then run ninja in my build directory, I still get DSO's not
> > static libraries. I am assuming that I'm doing something subtly wrong in my
> > build, but I can't seem to see what it is.
> >
> > On the other hand, if static builds don't work yet, thats going to be an issue.
> >
> > <snip>
> >
> > > +# configure the build, and make sure configs here and in config folder are
> > > +# able to be included in any file. We also store a global array of include dirs
> > > +# for passing to pmdinfogen scripts
> > > +global_inc = include_directories('.', 'config')
> > > +subdir('config')
> > > +
> > > +# TODO build libs and drivers
> > > +
> > > +# TODO build binaries and installable tools
> > > +
> > This seems outdated, but I think you remove it in a later patch
> >
> Yep. I put them in as comment placeholders in the early patches to show
> where things would go in later ones to try and make the flow clearer
> i.e. they were deliberately put in, even though removed later.
>
> Thanks for the feedback.
>
> /Bruce
>
On Fri, Sep 08, 2017 at 07:57:06AM -0400, Neil Horman wrote:
> On Fri, Sep 08, 2017 at 09:50:26AM +0100, Bruce Richardson wrote:
> > On Thu, Sep 07, 2017 at 12:21:57PM -0400, Neil Horman wrote:
> > > On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
> > > > To build with meson and ninja, we need some initial infrastructure in
> > > > place. The build files for meson always need to be called "meson.build",
> > > > and options get placed in meson_options.txt
> > > >
> > > > This commit adds a top-level meson.build file, which sets up the global
> > > > variables for tracking drivers, libraries, etc., and then includes other
> > > > build files, before finishing by writing the global build configuration
> > > > header file and a DPDK pkgconfig file at the end, using some of those same
> > > > globals.
> > > >
> > > > >From the top level build file, the only include file thus far is for the
> > > > config folder, which does some other setup of global configuration
> > > > parameters, including pulling in architecture specific parameters from an
> > > > architectural subdirectory. A number of configuration build options are
> > > > provided for the project to tune a number of global variables which will be
> > > > used later e.g. max numa nodes, max cores, etc. These settings all make
> > > > their way to the global build config header "rte_build_config.h". There is
> > > > also a file "rte_config.h", which includes "rte_build_config.h", and this
> > > > file is meant to hold other build-time values which are present in our
> > > > current static build configuration but are not normally meant for
> > > > user-configuration. Ideally, over time, the values placed here should be
> > > > moved to the individual libraries or drivers which want those values.
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> > >
> > > I feel like I need to underscore my previous concern here. While I'm not
> > > opposed per-se to a new build system, I am very concerned about the burden that
> > > switching places on downstream consumers, in particular distributions (since I
> > > represent one of them). Moving to a new build system with new tools means those
> > > tools need to be packaged, tested and shipped, which is a significant work
> > > effort. While it might be a net gain long term, its something you need to keep
> > > in mind when making these changes.
> > >
> > Understood. If there is anything we/I can do to make this transition
> > easier, please flag it for consideration.
> >
> Thank you, I appreciate that.
>
> > > I know you've said that we will be keepting the existing build system,
> > > I just need to be sure everyone understands just how important that
> > > is.
> > >
> > What is your feeling here, in terms of timescale. After any new system
> > reaches feature parity, how long would you estimate that we would need
> > to support the existing makefile system before it would be safe to
> > deprecate it? Should we start a deprecation plan, or is it best just to
> > commit to support both until we get all - or almost all - downstream
> > consumers switched over? While I wouldn't push for deprecating the old
> > system any time soon, and I wouldn't consider maintaining the two
> > unduly burdensome, it's not something we want to do in the long term.
> >
> I was hoping to avoid putting a specific time frame on it, but its a fair
> question to ask. I feel like any particular timetable is somewhat arbitrary.
> Keith suggested a year, which is likely as good as any in my mind. To put a bit
> more detail behind it, a RHEL release cycle is anywhere from 6 to 18 months, so
> a year fits well. If we assume starting a few weeks back when you first
> proposed this change, that its going to be merged, that gives us time to package
> the build components, build the new package using them, get it through a qa
> cycle and fix anything that pops up as a result. That way, when the switch is
> made, it can be done with an immediate deprecation of the old build system with
> a level of confidence that some of the more esoteric build targets/configs will
> likely work.
>
> > > Though perhaps the time frame for keeping the current build system as priarmy is
> > > less concerning, as feature parity is even more critical. That is to say, the
> > > new build system must be able to produce the same configurations that the
> > > current build system does. Without it I don't think anyone will be able to use
> > > it consistently, and that will leave a great number of users in a very poor
> > > position. I think getting a little closer to parity with the current system is
> > > warranted. I'd suggest as a gating factor:
> > >
> > > 1) Building on all supported arches
> > > 2) Cross building on all supported arches
> > > 3) Proper identification of targeted machine (i.e. equivalent of the machine
> > > component of the current build system)
> > >
> > The question there is gating factor for what? Presumably not for merging
> > into the staging tree. But for merging into the main tree for releases?
> > I'd push back a little on that, as the new system does not interfere in
> > any way with the old, and by keeping it in a staging tree until it
> > reaches full feature parity will make the job considerably harder. For
> > example, it means that anyone submitting a new driver or library has to
> > submit the code and makefiles in one set and the meson patches in a
> > separate one for a separate build tree. It also makes it less likely
> > that people will try out the new system and find the issues with it, and
> > help fill in the gaps. While I can understand us not recommending the
> > new build system until it reaches feature parity, I think there are a
> > lot of benefits to be got by making it widely available, even if it's
> > incomplete.
>
> Yes, sorry, the implied "what" here is gating its introduction to mainline. I
> have no problem with this going into a development or staging branch/tree, only
> with it getting merged to mainline and becoming the primary build system today.
> I get that it makes reaching feature parity harder, but to not do so relegates
> anyone that hasn't had a chance to test the new build system to second class
> citizen status (or at least potentially does so). To be a bit more specific, I
> can see how energized you might be to get this in place now because you've
> tested it on a wide array on intel hardware, but I'm guessing if it went in
> today, people at IBM and Linaro would have to drop active developement to start
> switching their build environments over to the new system lest they get left out
> in the cold. I think its more about balacing where the hardship lies here.
>
> As I'm writing this, I wonder if a reasonable compromise couldn't involve the
> use of CI? That is to say, what if we integrated the build system now-ish, and
> stood up an official CI instance, that both:
> 1) built the dpdk in all style configuration, which mandated the use of the old
> build system (i.e. we implicitly mandate that the current build system stay
> working, and is not forgotten), and gate patch merges on that result.
>
> 2) add a test that any change to a meson file in mainline also include a change
> to the a Makefile
>
> I'm just spitballing here, but I'm looking for ways to enforce the continued use
> of the current build system above and beyond a verbal promise to do so. The
> idea is to ensure that it stays operational and primary to the development of
> dpdk until build system parity is reached.
>
Yes, that all makes sense, and I agree that the main issue is how to
ensure/enforce the fact that the old build system stays as the main way
to build DPDK, until such time as the new system reaches parity. A CI
system like you describe would be the ideal case, and we have some parts
of that, just not enough I think to satify our needs. On the other hand,
while holding the new system in a staging tree till it reaches parity
would definitely achieve that objective I think the cost of that is too
high.
Right now patchwork reports results of "Intel-compilation". I'm not
aware of any immediate plans to expand that - somebody please should if
there are. There is also the planned DPDK lab, but I'm not sure of the
schedule for that or if it will help in this instance.
For now, I'll prepare a slightly-updated V2 patchset, with a view to
getting work started on the build-next tree. While that is going on, we
can maybe continue to think about how to put in suitable guard-rails for
ensuring old system compatibility for when it's pulled to mainline.
Thanks again for raising your concerns about this. It's easy for me to
sometimes push too hard for something I'm enthusiastic about! :-) I do
want this migration done properly, so if you have the bandwidth I'd
appreciate any reviews or feedback you can give, as the work goes on.
Regards,
/Bruce
+Thomas and Qian from our validation team, in case they have any insight
on build CI.
On Fri, Sep 08, 2017 at 02:55:08PM +0100, Bruce Richardson wrote:
> On Fri, Sep 08, 2017 at 07:57:06AM -0400, Neil Horman wrote:
> > On Fri, Sep 08, 2017 at 09:50:26AM +0100, Bruce Richardson wrote:
> > > On Thu, Sep 07, 2017 at 12:21:57PM -0400, Neil Horman wrote:
> > > > On Fri, Sep 01, 2017 at 11:04:00AM +0100, Bruce Richardson wrote:
> > > > > To build with meson and ninja, we need some initial infrastructure in
> > > > > place. The build files for meson always need to be called "meson.build",
> > > > > and options get placed in meson_options.txt
> > > > >
> > > > > This commit adds a top-level meson.build file, which sets up the global
> > > > > variables for tracking drivers, libraries, etc., and then includes other
> > > > > build files, before finishing by writing the global build configuration
> > > > > header file and a DPDK pkgconfig file at the end, using some of those same
> > > > > globals.
> > > > >
> > > > > >From the top level build file, the only include file thus far is for the
> > > > > config folder, which does some other setup of global configuration
> > > > > parameters, including pulling in architecture specific parameters from an
> > > > > architectural subdirectory. A number of configuration build options are
> > > > > provided for the project to tune a number of global variables which will be
> > > > > used later e.g. max numa nodes, max cores, etc. These settings all make
> > > > > their way to the global build config header "rte_build_config.h". There is
> > > > > also a file "rte_config.h", which includes "rte_build_config.h", and this
> > > > > file is meant to hold other build-time values which are present in our
> > > > > current static build configuration but are not normally meant for
> > > > > user-configuration. Ideally, over time, the values placed here should be
> > > > > moved to the individual libraries or drivers which want those values.
> > > > >
> > > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > > > Reviewed-by: Harry van Haaren <harry.van.haaren@intel.com>
> > > >
> > > > I feel like I need to underscore my previous concern here. While I'm not
> > > > opposed per-se to a new build system, I am very concerned about the burden that
> > > > switching places on downstream consumers, in particular distributions (since I
> > > > represent one of them). Moving to a new build system with new tools means those
> > > > tools need to be packaged, tested and shipped, which is a significant work
> > > > effort. While it might be a net gain long term, its something you need to keep
> > > > in mind when making these changes.
> > > >
> > > Understood. If there is anything we/I can do to make this transition
> > > easier, please flag it for consideration.
> > >
> > Thank you, I appreciate that.
> >
> > > > I know you've said that we will be keepting the existing build system,
> > > > I just need to be sure everyone understands just how important that
> > > > is.
> > > >
> > > What is your feeling here, in terms of timescale. After any new system
> > > reaches feature parity, how long would you estimate that we would need
> > > to support the existing makefile system before it would be safe to
> > > deprecate it? Should we start a deprecation plan, or is it best just to
> > > commit to support both until we get all - or almost all - downstream
> > > consumers switched over? While I wouldn't push for deprecating the old
> > > system any time soon, and I wouldn't consider maintaining the two
> > > unduly burdensome, it's not something we want to do in the long term.
> > >
> > I was hoping to avoid putting a specific time frame on it, but its a fair
> > question to ask. I feel like any particular timetable is somewhat arbitrary.
> > Keith suggested a year, which is likely as good as any in my mind. To put a bit
> > more detail behind it, a RHEL release cycle is anywhere from 6 to 18 months, so
> > a year fits well. If we assume starting a few weeks back when you first
> > proposed this change, that its going to be merged, that gives us time to package
> > the build components, build the new package using them, get it through a qa
> > cycle and fix anything that pops up as a result. That way, when the switch is
> > made, it can be done with an immediate deprecation of the old build system with
> > a level of confidence that some of the more esoteric build targets/configs will
> > likely work.
> >
> > > > Though perhaps the time frame for keeping the current build system as priarmy is
> > > > less concerning, as feature parity is even more critical. That is to say, the
> > > > new build system must be able to produce the same configurations that the
> > > > current build system does. Without it I don't think anyone will be able to use
> > > > it consistently, and that will leave a great number of users in a very poor
> > > > position. I think getting a little closer to parity with the current system is
> > > > warranted. I'd suggest as a gating factor:
> > > >
> > > > 1) Building on all supported arches
> > > > 2) Cross building on all supported arches
> > > > 3) Proper identification of targeted machine (i.e. equivalent of the machine
> > > > component of the current build system)
> > > >
> > > The question there is gating factor for what? Presumably not for merging
> > > into the staging tree. But for merging into the main tree for releases?
> > > I'd push back a little on that, as the new system does not interfere in
> > > any way with the old, and by keeping it in a staging tree until it
> > > reaches full feature parity will make the job considerably harder. For
> > > example, it means that anyone submitting a new driver or library has to
> > > submit the code and makefiles in one set and the meson patches in a
> > > separate one for a separate build tree. It also makes it less likely
> > > that people will try out the new system and find the issues with it, and
> > > help fill in the gaps. While I can understand us not recommending the
> > > new build system until it reaches feature parity, I think there are a
> > > lot of benefits to be got by making it widely available, even if it's
> > > incomplete.
> >
> > Yes, sorry, the implied "what" here is gating its introduction to mainline. I
> > have no problem with this going into a development or staging branch/tree, only
> > with it getting merged to mainline and becoming the primary build system today.
> > I get that it makes reaching feature parity harder, but to not do so relegates
> > anyone that hasn't had a chance to test the new build system to second class
> > citizen status (or at least potentially does so). To be a bit more specific, I
> > can see how energized you might be to get this in place now because you've
> > tested it on a wide array on intel hardware, but I'm guessing if it went in
> > today, people at IBM and Linaro would have to drop active developement to start
> > switching their build environments over to the new system lest they get left out
> > in the cold. I think its more about balacing where the hardship lies here.
> >
> > As I'm writing this, I wonder if a reasonable compromise couldn't involve the
> > use of CI? That is to say, what if we integrated the build system now-ish, and
> > stood up an official CI instance, that both:
> > 1) built the dpdk in all style configuration, which mandated the use of the old
> > build system (i.e. we implicitly mandate that the current build system stay
> > working, and is not forgotten), and gate patch merges on that result.
> >
> > 2) add a test that any change to a meson file in mainline also include a change
> > to the a Makefile
> >
> > I'm just spitballing here, but I'm looking for ways to enforce the continued use
> > of the current build system above and beyond a verbal promise to do so. The
> > idea is to ensure that it stays operational and primary to the development of
> > dpdk until build system parity is reached.
> >
>
> Yes, that all makes sense, and I agree that the main issue is how to
> ensure/enforce the fact that the old build system stays as the main way
> to build DPDK, until such time as the new system reaches parity. A CI
> system like you describe would be the ideal case, and we have some parts
> of that, just not enough I think to satify our needs. On the other hand,
> while holding the new system in a staging tree till it reaches parity
> would definitely achieve that objective I think the cost of that is too
> high.
>
> Right now patchwork reports results of "Intel-compilation". I'm not
> aware of any immediate plans to expand that - somebody please should if
> there are. There is also the planned DPDK lab, but I'm not sure of the
> schedule for that or if it will help in this instance.
>
> For now, I'll prepare a slightly-updated V2 patchset, with a view to
> getting work started on the build-next tree. While that is going on, we
> can maybe continue to think about how to put in suitable guard-rails for
> ensuring old system compatibility for when it's pulled to mainline.
>
> Thanks again for raising your concerns about this. It's easy for me to
> sometimes push too hard for something I'm enthusiastic about! :-) I do
> want this migration done properly, so if you have the bandwidth I'd
> appreciate any reviews or feedback you can give, as the work goes on.
>
> Regards,
> /Bruce
>
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> Sent: Friday, September 1, 2017 11:04 AM
> To: dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure for meson &
> ninja builds
<snip commit message etc>
> diff --git a/config/meson.build b/config/meson.build
<snip>
> +# set the machine type and cflags for it
> +machine = get_option('machine')
> +dpdk_conf.set('RTE_MACHINE', machine)
> +add_project_arguments('-march=@0@'.format(machine), language: 'c')
> +# some libs depend on maths lib
> +add_project_link_arguments('-lm', language: 'c')
> +
> +# add -include rte_config to cflags
> +add_project_arguments('-include', 'rte_config.h', language: 'c')
> +
> +# disable any unwanted warnings
> +unwanted_warnings = [
> + '-Wno-address-of-packed-member',
> + '-Wno-format-truncation'
> +]
Feedback from usage while developing new features;
- Mis-matched integer sign comparison doesn't cause a warning
- And -Werror isn't set by default
Adding these as per below fixes things... but for cleanliness "unwanted warnings" should probably be renamed, we want more warnings! :D
# disable any unwanted warnings
unwanted_warnings = [
'-Werror',
'-Wno-address-of-packed-member',
'-Wno-format-truncation',
'-Wsign-compare',
]
The performance of Ninja is amazing for testing shared-object builds and version.map changes - feedback is instant. Makes light work of re-checking ABI/API stuff.
> +foreach arg: unwanted_warnings
> + if cc.has_argument(arg)
> + add_project_arguments(arg, language: 'c')
> + endif
> +endforeach
> +
> -----Original Message-----
> From: Van Haaren, Harry
> Sent: Friday, September 8, 2017 5:03 PM
> To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org
> Cc: Richardson, Bruce <bruce.richardson@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure
> for meson & ninja builds
>
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > Sent: Friday, September 1, 2017 11:04 AM
> > To: dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure
> > for meson & ninja builds
>
> <snip commit message etc>
>
> > diff --git a/config/meson.build b/config/meson.build
> <snip>
> > +# set the machine type and cflags for it machine =
> > +get_option('machine') dpdk_conf.set('RTE_MACHINE', machine)
> > +add_project_arguments('-march=@0@'.format(machine), language: 'c') #
> > +some libs depend on maths lib add_project_link_arguments('-lm',
> > +language: 'c')
> > +
> > +# add -include rte_config to cflags
> > +add_project_arguments('-include', 'rte_config.h', language: 'c')
> > +
> > +# disable any unwanted warnings
> > +unwanted_warnings = [
> > + '-Wno-address-of-packed-member',
> > + '-Wno-format-truncation'
> > +]
>
>
> Feedback from usage while developing new features;
> - Mis-matched integer sign comparison doesn't cause a warning
> - And -Werror isn't set by default
>
Will fix the former in V2.
For the latter, I already have a patch for it that approximates the behavior of the existing system - i.e. a "developer build" setting, which is "off" by default, except when you are building from a git checkout in which case it defaults to "on". However, I don't think I'll include that in V2, as having Werror when we are not reasonably sure we have fixed all warnings in the code for all supported compilers and distros is a bad idea IMHO.
> Adding these as per below fixes things... but for cleanliness "unwanted
> warnings" should probably be renamed, we want more warnings! :D
>
> # disable any unwanted warnings
> unwanted_warnings = [
> '-Werror',
> '-Wno-address-of-packed-member',
> '-Wno-format-truncation',
> '-Wsign-compare',
> ]
>
Actually, I'd rather keep it as a list of warnings to disable for now. Enabling warnings should be handled differently, as we may only enable specific warnings depending on the build type being done.
/Bruce
On Fri, Sep 08, 2017 at 04:13:58PM +0000, Richardson, Bruce wrote:
>
>
> > -----Original Message-----
> > From: Van Haaren, Harry
> > Sent: Friday, September 8, 2017 5:03 PM
> > To: Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure
> > for meson & ninja builds
> >
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson
> > > Sent: Friday, September 1, 2017 11:04 AM
> > > To: dev@dpdk.org
> > > Cc: Richardson, Bruce <bruce.richardson@intel.com>
> > > Subject: [dpdk-dev] [PATCH 01/17] build: add initial infrastructure
> > > for meson & ninja builds
> >
> > <snip commit message etc>
> >
> > > diff --git a/config/meson.build b/config/meson.build
> > <snip>
> > > +# set the machine type and cflags for it machine =
> > > +get_option('machine') dpdk_conf.set('RTE_MACHINE', machine)
> > > +add_project_arguments('-march=@0@'.format(machine), language: 'c') #
> > > +some libs depend on maths lib add_project_link_arguments('-lm',
> > > +language: 'c')
> > > +
> > > +# add -include rte_config to cflags
> > > +add_project_arguments('-include', 'rte_config.h', language: 'c')
> > > +
> > > +# disable any unwanted warnings
> > > +unwanted_warnings = [
> > > + '-Wno-address-of-packed-member',
> > > + '-Wno-format-truncation'
> > > +]
> >
> >
> > Feedback from usage while developing new features;
> > - Mis-matched integer sign comparison doesn't cause a warning
> > - And -Werror isn't set by default
> >
> Will fix the former in V2.
> For the latter, I already have a patch for it that approximates the behavior of the existing system - i.e. a "developer build" setting, which is "off" by default, except when you are building from a git checkout in which case it defaults to "on". However, I don't think I'll include that in V2, as having Werror when we are not reasonably sure we have fixed all warnings in the code for all supported compilers and distros is a bad idea IMHO.
>
> > Adding these as per below fixes things... but for cleanliness "unwanted
> > warnings" should probably be renamed, we want more warnings! :D
> >
> > # disable any unwanted warnings
> > unwanted_warnings = [
> > '-Werror',
> > '-Wno-address-of-packed-member',
> > '-Wno-format-truncation',
> > '-Wsign-compare',
> > ]
> >
> Actually, I'd rather keep it as a list of warnings to disable for now. Enabling warnings should be handled differently, as we may only enable specific warnings depending on the build type being done.
>
On second-thoughts, for V2 I will do as you suggest and add in the extra
warnings '-Wsign-compare' and '-Wcast-qual', to the existing warnings
list. It simplifies the code and we can just split them out later if we
want them only for certain build targets. For now, I don't think that is
needed, so I'll keep it simple.
/Bruce
new file mode 100644
@@ -0,0 +1,69 @@
+# BSD LICENSE
+#
+# Copyright(c) 2017 Intel Corporation. All rights reserved.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in
+# the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Intel Corporation nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# set the machine type and cflags for it
+machine = get_option('machine')
+dpdk_conf.set('RTE_MACHINE', machine)
+add_project_arguments('-march=@0@'.format(machine), language: 'c')
+# some libs depend on maths lib
+add_project_link_arguments('-lm', language: 'c')
+
+# add -include rte_config to cflags
+add_project_arguments('-include', 'rte_config.h', language: 'c')
+
+# disable any unwanted warnings
+unwanted_warnings = [
+ '-Wno-address-of-packed-member',
+ '-Wno-format-truncation'
+]
+foreach arg: unwanted_warnings
+ if cc.has_argument(arg)
+ add_project_arguments(arg, language: 'c')
+ endif
+endforeach
+
+compile_time_cpuflags = []
+if host_machine.cpu_family().startswith('x86')
+ arch_subdir = 'x86'
+ subdir(arch_subdir)
+endif
+dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
+
+# set the install path for the drivers
+dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', driver_install_path)
+
+# set other values pulled from the build options
+dpdk_conf.set('RTE_MAX_LCORE', get_option('max_lcores'))
+dpdk_conf.set('RTE_MAX_NUMA_NODES', get_option('max_numa_nodes'))
+dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
+dpdk_conf.set('RTE_EAL_ALLOW_INV_SOCKET_ID', get_option('allow_invalid_socket_id'))
+
+install_headers('rte_config.h')
new file mode 100644
@@ -0,0 +1,50 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016 Intel Corporation. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/**
+ * @file Header file containing DPDK compilation parameters
+ *
+ * Header file containing DPDK compilation parameters. Also include the
+ * meson-generated header file containing the detected parameters that
+ * are variable across builds or build environments.
+ *
+ * NOTE: This file is only used for meson+ninja builds. For builds done
+ * using make/gmake, the rte_config.h file is autogenerated from the
+ * defconfig_* files in the config directory.
+ */
+#ifndef _RTE_CONFIG_H_
+#define _RTE_CONFIG_H_
+
+#include <rte_build_config.h>
+
+#endif /* _RTE_CONFIG_H_ */
new file mode 100644
@@ -0,0 +1,70 @@
+# BSD LICENSE
+#
+# Copyright(c) 2017 Intel Corporation. All rights reserved.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in
+# the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Intel Corporation nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+# for checking defines we need to use the correct compiler flags
+march_opt = '-march=@0@'.format(machine)
+
+# we require SSE4.2 for DPDK
+sse_errormsg = '''SSE4.2 instruction set is required for DPDK.
+Please set the machine type to "nehalem" or "corei7" or higher value'''
+
+if cc.get_define('__SSE4_2__', args: march_opt) == ''
+ error(sse_errormsg)
+endif
+
+dpdk_conf.set('RTE_ARCH_X86', 1)
+if (host_machine.cpu_family() == 'x86_64')
+ dpdk_conf.set('RTE_ARCH_X86_64', 1)
+ dpdk_conf.set('RTE_ARCH', 'x86_64')
+ dpdk_conf.set('RTE_ARCH_64', 1)
+else
+ dpdk_conf.set('RTE_ARCH_I686', 1)
+ dpdk_conf.set('RTE_ARCH', 'i686')
+endif
+
+if cc.get_define('__AES__', args: march_opt) != ''
+ dpdk_conf.set('RTE_MACHINE_CPUFLAG_AES', 1)
+ compile_time_cpuflags += ['RTE_CPUFLAG_AES']
+endif
+if cc.get_define('__PCLMUL__', args: march_opt) != ''
+ dpdk_conf.set('RTE_MACHINE_CPUFLAG_PCLMULQDQ', 1)
+ compile_time_cpuflags += ['RTE_CPUFLAG_PCLMULQDQ']
+endif
+if cc.get_define('__AVX__', args: march_opt) != ''
+ dpdk_conf.set('RTE_MACHINE_CPUFLAG_AVX', 1)
+ compile_time_cpuflags += ['RTE_CPUFLAG_AVX']
+endif
+if cc.get_define('__AVX2__', args: march_opt) != ''
+ dpdk_conf.set('RTE_MACHINE_CPUFLAG_AVX2', 1)
+ compile_time_cpuflags += ['RTE_CPUFLAG_AVX2']
+endif
+
+dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
new file mode 100644
@@ -0,0 +1,83 @@
+# BSD LICENSE
+#
+# Copyright(c) 2017 Intel Corporation. All rights reserved.
+# All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions
+# are met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in
+# the documentation and/or other materials provided with the
+# distribution.
+# * Neither the name of Intel Corporation nor the names of its
+# contributors may be used to endorse or promote products derived
+# from this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+project('DPDK', 'C',
+ version: '17.08.0',
+ license: 'BSD',
+ default_options: ['buildtype=release'],
+ meson_version: '>= 0.41'
+)
+
+# set up some global vars for compiler, platform, configuration, etc.
+cc = meson.get_compiler('c')
+dpdk_conf = configuration_data()
+dpdk_libraries = []
+dpdk_drivers = []
+dpdk_extra_ldflags = []
+
+# for static libs, treat the drivers as regular libraries, otherwise
+# for shared libs, put them in a driver folder
+if get_option('default_library') == 'static'
+ driver_install_path = get_option('libdir')
+else
+ driver_install_path = '@0@/dpdk/drivers'.format(get_option('prefix'))
+endif
+
+# configure the build, and make sure configs here and in config folder are
+# able to be included in any file. We also store a global array of include dirs
+# for passing to pmdinfogen scripts
+global_inc = include_directories('.', 'config')
+subdir('config')
+
+# TODO build libs and drivers
+
+# TODO build binaries and installable tools
+
+# write the build config
+build_cfg = 'rte_build_config.h'
+configure_file(output: build_cfg,
+ configuration: dpdk_conf,
+ install_dir: get_option('includedir'))
+
+# for static builds, include the drivers as libs, and also any
+# other dependent libs that DPDK needs to link against
+if get_option('default_library') == 'static'
+ dpdk_drivers = ['-Wl,--whole-archive'] + dpdk_drivers + ['-Wl,--no-whole-archive']
+ dpdk_libraries = dpdk_drivers + dpdk_libraries + dpdk_extra_ldflags
+endif
+
+pkg = import('pkgconfig')
+pkg.generate(name: meson.project_name(),
+ version: meson.project_version(),
+ libraries: dpdk_libraries,
+ description: 'The Data Plane Development Kit (DPDK)',
+ extra_cflags: '-include "rte_config.h"'
+)
new file mode 100644
@@ -0,0 +1,6 @@
+option('machine', type: 'string', value: 'native', description: 'set the target machine type')
+option('max_lcores', type: 'string', value: '128', description: 'maximum number of cores/threads supported by EAL')
+option('max_numa_nodes', type: 'string', value: '4', description: 'maximum number of NUMA nodes supported by EAL')
+option('use_hpet', type: 'boolean', value: false, description: 'use HPET timer in EAL')
+option('allow_invalid_socket_id', type: 'boolean', value: false,
+ description: 'allow out-of-range NUMA socket id\'s for platforms that don\'t report the value correctly')