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

[Bruce Richardson]

On Sat, May 26, 2018 at 11:32:53AM +0200, Thomas Monjalon wrote:
> 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.
> 

We don't need DPDK_TARGET variable any more, however, environmental vars
can be used by this script for setting up PATH and CFLAGS/LDFLAGS etc. as
needed. However, environmental vars bar those standard ones are ignored by
a meson build, so they would be only for script use. Therefore I'm
suggesting removing only the unused ones.

> > $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.

You can use CROSS in this script to set PATH for each build. Again, though
it would only be for script, not meson use.

> 
> > > +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.

We would need to pass the base to meson, because otherwise meson assumes
the top-level meson.build file is in the current directory, i.e. calling
"meson build-dir" is the same as "meson . build-dir". If we want to allow
using this script from other places on filesystem, we need to either "cd"
to the base dir as you do now, or else explicitly pass in the basedir. The
latter I think is a better option as it would allow building from any
location on the filesystem.

> 
> > > +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).
> 
How would it be different, it's the same command called with the same
environment each time?

/Bruce