Re: [dpdk-dev] [PATCH v2] devtools: add test script for meson builds

[Thomas Monjalon <thomas@monjalon.net>]

25/05/2018 17:18, Bruce Richardson:
> On Fri, May 25, 2018 at 04:51:58PM +0200, Thomas Monjalon wrote:
> > +default_path=$PATH
> > +
> > +# Load config options
> > +. $(dirname $(readlink -e $0))/load-devel-config
> > +
> 
> Why is this needed here, it seems to be called before each individual
> config anyway.

Right, it can be removed from here.

> > +reset_env ()
> > +{
> > +	export PATH=$default_path
> > +	unset CROSS
> > +	unset ARMV8_CRYPTO_LIB_PATH
> > +	unset FLEXRAN_SDK
> > +	unset LIBMUSDK_PATH
> > +	unset LIBSSO_SNOW3G_PATH
> > +	unset LIBSSO_KASUMI_PATH
> > +	unset LIBSSO_ZUC_PATH
> > +	unset PQOS_INSTALL_PATH
> 
> These variables bar PATH are unused by meson build, so should be removed
> here to avoid giving the impression they are use.

Actually they should be used when compiling.
PATH can be used to allow a toolchain which is not in the standard path.
And dependencies _PATH variables can be specified only for some builds.
Example: I have libsso only for x86 64-bit, so I do not set it
for 32-bit or ARM builds. The config file reads DPDK_TARGET to know.
Note: DPDK_TARGET is not yet set correctly for every builds in this version.

> $CROSS is used by the
> script so perhaps it can be kept. However, the whole point of the
> cross-files is that you include all the needed details of your compiler
> there. I think we should move away from using the CROSS value completely,
> and use the cross-files properly.

Yes we can remove CROSS if it is redundant with config files in config/arm/.
But I do not understand why these files cannot be agnostic regarding the
name (CROSS) of the toolchain.
To me it is very strange to have the binary names hard-linked in the configs.
Anyway, this discussion is out of the scope of this script.
So I am for removing the CROSS variable and use aarch64-linux-gnu-gcc
as it is hard written in every ARM configs for now.

> > +cd $(dirname $(readlink -m $0))/..
> > +
> I don't think we should force the builds to be always put into the base
> directory. Instead of using cd, I think we should instead capture the base
> directory path and pass that to meson.

OK to not force the directory.
You want to build in the current directory?
If yes, we can just remove this "cd" and no need to pass a base directory
to meson.

> > +load_config ()
> > +{
> > +	reset_env
> > +	. $(dirname $(readlink -e $0))/load-devel-config
> > +	MESON=${MESON:-meson}
> > +}
> Why does this need to be done each time?

Because the config could be different for each build (see above).