[dpdk-dev,v2] devtools: add test script for meson builds
Checks
Commit Message
From: Bruce Richardson <bruce.richardson@intel.com>
To simplify testing with the meson and ninja builds, we can add a script
to set up and do multiple builds. Currently this script sets up:
* clang and gcc builds
* builds using static and shared linkage for binaries (libs are always
built as both)
* a build using the lowest instruction-set level for x86 (-march=nehalem)
* cross-builds for each cross-file listed in config/arm
Each build is configured in a directory ending in *-build, and then for
the build stage, we just call ninja in each directory in turn. [i.e. we
assume every directory starting with "build-" is a meson build, which is
probably an ok assumption].
It can use the same configuration file as for the legacy test-build.sh.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v2: it is a rework with 3 major changes
- automatically stop on error thanks to -e
- directory name starts with "build-"
- optionally load a config file to get some environment variables
---
MAINTAINERS | 1 +
devtools/test-meson-builds.sh | 75 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
create mode 100755 devtools/test-meson-builds.sh
Comments
On Fri, May 25, 2018 at 04:51:58PM +0200, Thomas Monjalon wrote:
> From: Bruce Richardson <bruce.richardson@intel.com>
>
> To simplify testing with the meson and ninja builds, we can add a script
> to set up and do multiple builds. Currently this script sets up:
>
> * clang and gcc builds
> * builds using static and shared linkage for binaries (libs are always
> built as both)
> * a build using the lowest instruction-set level for x86 (-march=nehalem)
> * cross-builds for each cross-file listed in config/arm
>
> Each build is configured in a directory ending in *-build, and then for
> the build stage, we just call ninja in each directory in turn. [i.e. we
> assume every directory starting with "build-" is a meson build, which is
> probably an ok assumption].
>
> It can use the same configuration file as for the legacy test-build.sh.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v2: it is a rework with 3 major changes
> - automatically stop on error thanks to -e
> - directory name starts with "build-"
> - optionally load a config file to get some environment variables
> ---
> MAINTAINERS | 1 +
> devtools/test-meson-builds.sh | 75 +++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
> create mode 100755 devtools/test-meson-builds.sh
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e56c72687..4d015fe53 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -86,6 +86,7 @@ F: devtools/get-maintainer.sh
> F: devtools/git-log-fixes.sh
> F: devtools/load-devel-config
> F: devtools/test-build.sh
> +F: devtools/test-meson-builds.sh
> F: license/
>
>
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> new file mode 100755
> index 000000000..8ce0a1d31
> --- /dev/null
> +++ b/devtools/test-meson-builds.sh
> @@ -0,0 +1,75 @@
> +#! /bin/sh -e
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2018 Intel Corporation
> +
> +# Run meson to auto-configure the various builds.
> +# * all builds get put in a directory whose name starts with "build-"
> +# * if a build-directory already exists we assume it was properly configured
> +# Run ninja after configuration is done.
> +
> +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.
> +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. $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.
> +}
> +
> +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.
> +load_config ()
> +{
> + reset_env
> + . $(dirname $(readlink -e $0))/load-devel-config
> + MESON=${MESON:-meson}
> +}
Why does this need to be done each time?
> +
> +build () # <directory> <meson options>
> +{
> + dir=$1
> + shift
> + if [ ! -d "$dir" ] ; then
> + options="--werror -Dexamples=all $*"
> + # TODO: the configuration variables $DPDK_DEP_CFLAGS
> + # and $DPDK_DEP_LDFLAGS are not handled in this script
> + echo "$MESON $options $dir"
> + $MESON $options $dir
> + unset CC
> + fi
> + echo "ninja -C $dir"
> + ninja -C $dir
> +}
> +
> +# shared and static linked builds with gcc and clang
> +for c in gcc clang ; do
> + for s in static shared ; do
> + load_config
> + export CC="ccache $c"
> + build build-$c-$s --default-library=$s
> + done
> +done
> +
> +# test compilation with minimal x86 instruction set
> +load_config
> +build build-x86-default -Dmachine=nehalem
> +
> +# enable cross compilation if gcc cross-compiler is found
> +for f in config/arm/arm*gcc ; do
> + DPDK_TARGET=$(basename $f | tr '_' '-')
> + load_config
> + CROSS=${CROSS:-aarch64-linux-gnu-}
> + if ! command -v ${CROSS}gcc >/dev/null 2>&1 ; then
> + continue
> + fi
> + build build-$(echo $DPDK_TARGET | cut -d'-' -f-2) --cross-file $f
> +done
> --
> 2.16.2
>
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).
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
28/05/2018 11:33, 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:
> > > > +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.
I agree.
I don't understand the meson syntax:
meson [ options ] [ source directory ] [ build directory ]
If there is only one argument, it is the build directory?
I could send a v5 to add the source directory in the meson command.
It would be:
srcdir=$(dirname $(readlink -m $0))/..
$MESON $options $srcdir $builddir
> > > > +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?
No, the idea is to adapt the environment to the build target.
As an example, the dependencies can be different for 32-bit and 64-bit.
On Mon, May 28, 2018 at 12:26:24PM +0200, Thomas Monjalon wrote:
> 28/05/2018 11:33, 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:
> > > > > +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.
>
> I agree.
>
> I don't understand the meson syntax:
> meson [ options ] [ source directory ] [ build directory ]
> If there is only one argument, it is the build directory?
>
> I could send a v5 to add the source directory in the meson command.
> It would be:
> srcdir=$(dirname $(readlink -m $0))/..
> $MESON $options $srcdir $builddir
>
Yes, exactly.
>
> > > > > +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?
>
> No, the idea is to adapt the environment to the build target. As an
> example, the dependencies can be different for 32-bit and 64-bit.
>
I would hope that dependency detection should solve that, but since you
already have support for that in existing build script via environment
vars, I have no objection to leveraging that in the meson scripts. Overall,
though, I'd prefer to ensure that the detection works so that everyone only
needs one environment setup in order to get all builds working
simultaneously.
/Bruce
28/05/2018 15:20, Bruce Richardson:
> On Mon, May 28, 2018 at 12:26:24PM +0200, Thomas Monjalon wrote:
> > 28/05/2018 11:33, 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:
> > > > > > +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?
> >
> > No, the idea is to adapt the environment to the build target. As an
> > example, the dependencies can be different for 32-bit and 64-bit.
> >
> I would hope that dependency detection should solve that, but since you
> already have support for that in existing build script via environment
> vars, I have no objection to leveraging that in the meson scripts. Overall,
> though, I'd prefer to ensure that the detection works so that everyone only
> needs one environment setup in order to get all builds working
> simultaneously.
The dependency detection cannot work if I have dependencies in uncommon
directories.
I think it is important to allow testing compilation with dependencies
which are available but not installed, by providing paths.
On Tue, May 29, 2018 at 12:38:14PM +0200, Thomas Monjalon wrote:
> 28/05/2018 15:20, Bruce Richardson:
> > On Mon, May 28, 2018 at 12:26:24PM +0200, Thomas Monjalon wrote:
> > > 28/05/2018 11:33, 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:
> > > > > > > +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?
> > >
> > > No, the idea is to adapt the environment to the build target. As an
> > > example, the dependencies can be different for 32-bit and 64-bit.
> > >
> > I would hope that dependency detection should solve that, but since you
> > already have support for that in existing build script via environment
> > vars, I have no objection to leveraging that in the meson scripts. Overall,
> > though, I'd prefer to ensure that the detection works so that everyone only
> > needs one environment setup in order to get all builds working
> > simultaneously.
>
> The dependency detection cannot work if I have dependencies in uncommon
> directories.
> I think it is important to allow testing compilation with dependencies
> which are available but not installed, by providing paths.
>
Yes, but that should be done via meson configure options to specify the
paths, rather than via environmental variables. AFAIK there is no support in
meson for querying the environment*, every variable that could affect the
build should be explicitly specified
/Bruce
* Ok, no direct way. There are probably ways in which it could be done,
but it's not the meson way to do things.
@@ -86,6 +86,7 @@ F: devtools/get-maintainer.sh
F: devtools/git-log-fixes.sh
F: devtools/load-devel-config
F: devtools/test-build.sh
+F: devtools/test-meson-builds.sh
F: license/
new file mode 100755
@@ -0,0 +1,75 @@
+#! /bin/sh -e
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2018 Intel Corporation
+
+# Run meson to auto-configure the various builds.
+# * all builds get put in a directory whose name starts with "build-"
+# * if a build-directory already exists we assume it was properly configured
+# Run ninja after configuration is done.
+
+default_path=$PATH
+
+# Load config options
+. $(dirname $(readlink -e $0))/load-devel-config
+
+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
+}
+
+cd $(dirname $(readlink -m $0))/..
+
+load_config ()
+{
+ reset_env
+ . $(dirname $(readlink -e $0))/load-devel-config
+ MESON=${MESON:-meson}
+}
+
+build () # <directory> <meson options>
+{
+ dir=$1
+ shift
+ if [ ! -d "$dir" ] ; then
+ options="--werror -Dexamples=all $*"
+ # TODO: the configuration variables $DPDK_DEP_CFLAGS
+ # and $DPDK_DEP_LDFLAGS are not handled in this script
+ echo "$MESON $options $dir"
+ $MESON $options $dir
+ unset CC
+ fi
+ echo "ninja -C $dir"
+ ninja -C $dir
+}
+
+# shared and static linked builds with gcc and clang
+for c in gcc clang ; do
+ for s in static shared ; do
+ load_config
+ export CC="ccache $c"
+ build build-$c-$s --default-library=$s
+ done
+done
+
+# test compilation with minimal x86 instruction set
+load_config
+build build-x86-default -Dmachine=nehalem
+
+# enable cross compilation if gcc cross-compiler is found
+for f in config/arm/arm*gcc ; do
+ DPDK_TARGET=$(basename $f | tr '_' '-')
+ load_config
+ CROSS=${CROSS:-aarch64-linux-gnu-}
+ if ! command -v ${CROSS}gcc >/dev/null 2>&1 ; then
+ continue
+ fi
+ build build-$(echo $DPDK_TARGET | cut -d'-' -f-2) --cross-file $f
+done