Re: [dpdk-dev] [PATCH 2/2] net/sfc: add support for meson build

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Feb 02, 2018 at 12:43:58PM +0000, Andrew Rybchenko wrote:
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> 
> Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
> Signed-off-by: Andrew Rybchenko <arybchenko@solarflare.com>
> ---
>  drivers/net/meson.build          |  2 +-
>  drivers/net/sfc/base/meson.build | 69 ++++++++++++++++++++++++++++++++++++++++
>  drivers/net/sfc/meson.build      | 61 +++++++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/sfc/base/meson.build
>  create mode 100644 drivers/net/sfc/meson.build

Hi,

looks pretty good in general. I have a few comments below to improve
things.

Unfortunately, this will break the ARM builds in it's current form too
(or at least it broke the builds using the cross-files that I tested).
I think you need to add a check at the top of the driver meson.build
file for unsupported architectures, and set "build = false" for those
platforms.

/Bruce

<snip>
> diff --git a/drivers/net/sfc/meson.build b/drivers/net/sfc/meson.build
> new file mode 100644
> index 0000000..b603579
> --- /dev/null
> +++ b/drivers/net/sfc/meson.build
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +#
> +# Copyright (c) 2016-2018 Solarflare Communications Inc.
> +# All rights reserved.
> +#
> +# This software was jointly developed between OKTET Labs (under contract
> +# for Solarflare) and Solarflare Communications, Inc.
> +
> +allow_experimental_apis = true
> +
> +extra_flags = []
> +extra_flags += '-I' + meson.current_source_dir() + '/base'
> +extra_flags += '-I' + meson.current_source_dir()

The driver's own directory is already set in the include path, so it
should not necessary to add it as a cflag. For the base folder, the
"includes" variable should be used rather than the cflags one. These
two lines should just be replaced by:

includes += include_directories('base')

You may also need to put "include_directories: includes," into the
static_library call in the base folder if you have things being included
by base files from the root folder too.

> +extra_flags += '-O3'

The optimisation level is set for the project as a whole, and should not
be overridden in the driver. Otherwise a debug build will not be a debug
build for your driver.