diff mbox series

[v2,4/6] devtools/test-meson-builds: add testing of pkg-config file

Message ID 20190426165043.17268-5-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series add testing of libdpdk pkg-config file |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Bruce Richardson April 26, 2019, 4:50 p.m. UTC 
  The pkg-config file generated as part of the build of DPDK should allow
applications to be built with an installed DPDK. We can test this as
part of the build by doing an install of DPDK to a temporary directory
within the build folder, and by then compiling up a few sample apps
using make working off that directory.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Luca Boccassi <bluca@debian.org>
---
 devtools/test-meson-builds.sh | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

Comments

Thomas Monjalon May 2, 2019, 12:38 p.m. UTC | #1 
Hi,

I will probably have a ton of comments about adding a new compilation tests,
and I think it is a bit late for such an addition.
However, all the fixes should go in 19.05.

26/04/2019 18:50, Bruce Richardson:
> The pkg-config file generated as part of the build of DPDK should allow
> applications to be built with an installed DPDK. We can test this as
> part of the build by doing an install of DPDK to a temporary directory
> within the build folder, and by then compiling up a few sample apps
> using make working off that directory.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Luca Boccassi <bluca@debian.org>
> ---
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> +##############
> +# Test installation of the x86-default target, to be used for checking
> +# the sample apps build using the pkg-config file for cflags and libs
> +###############

I would prefer simpler comment formatting.
It makes this test very special.

> +build_path=build-x86-default
> +DESTDIR=`pwd`/$build_path/install-root ; export DESTDIR

export DESTDIR=... is not supported everywhere?
I prefer new shell substitution syntax $() instead of backquotes.

> +$ninja_cmd -C $build_path install
> +
> +pc_file=$(find $DESTDIR -name libdpdk.pc)
> +PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
> +
> +# rather than hacking our environment, just edit the .pc file prefix value
> +sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file

What is the alternative?
Cannot we configure meson with the right prefix?

> +for example in helloworld l2fwd l3fwd skeleton timer; do
> +	echo "## Building $example"
> +	$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example
> +done
> +
> +echo ""
> +echo "## $0: Completed OK"

This last log is uncommon.
Luca Boccassi May 2, 2019, 12:54 p.m. UTC | #2 
On Thu, 2019-05-02 at 14:38 +0200, Thomas Monjalon wrote:
> > +$ninja_cmd -C $build_path install
> > +
> > +pc_file=$(find $DESTDIR -name libdpdk.pc)
> > +PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
> > +
> > +# rather than hacking our environment, just edit the .pc file
> > prefix value
> > +sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file
> 
> What is the alternative?
> Cannot we configure meson with the right prefix?

Please see the email exchange between myself and Bruce on v1 3/4 - the
short version is that it can, but it gets messy due to the kernel
modules.
Bruce Richardson May 2, 2019, 1:21 p.m. UTC | #3 
On Thu, May 02, 2019 at 02:38:49PM +0200, Thomas Monjalon wrote:
> Hi,
> 
> I will probably have a ton of comments about adding a new compilation tests,
> and I think it is a bit late for such an addition.
> However, all the fixes should go in 19.05.
> 
> 26/04/2019 18:50, Bruce Richardson:
> > The pkg-config file generated as part of the build of DPDK should allow
> > applications to be built with an installed DPDK. We can test this as
> > part of the build by doing an install of DPDK to a temporary directory
> > within the build folder, and by then compiling up a few sample apps
> > using make working off that directory.
> > 
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Luca Boccassi <bluca@debian.org>
> > ---
> > --- a/devtools/test-meson-builds.sh
> > +++ b/devtools/test-meson-builds.sh
> > +##############
> > +# Test installation of the x86-default target, to be used for checking
> > +# the sample apps build using the pkg-config file for cflags and libs
> > +###############
> 
> I would prefer simpler comment formatting.
> It makes this test very special.

Your comment implies that it is not :-)
Sure, I can reduce the highlighting.

> 
> > +build_path=build-x86-default
> > +DESTDIR=`pwd`/$build_path/install-root ; export DESTDIR
> 
> export DESTDIR=... is not supported everywhere?

No 100% sure, so left it like this just in case.

> I prefer new shell substitution syntax $() instead of backquotes.
> 
Sure, I agree it's more readable.

> > +$ninja_cmd -C $build_path install
> > +
> > +pc_file=$(find $DESTDIR -name libdpdk.pc)
> > +PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
> > +
> > +# rather than hacking our environment, just edit the .pc file prefix value
> > +sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file
> 
> What is the alternative?
> Cannot we configure meson with the right prefix?
> 

See previous discussion.
Short answer, yes we can, but the prefix does not apply to kernel modules
as they always install in /lib/modules folder.

> > +for example in helloworld l2fwd l3fwd skeleton timer; do
> > +	echo "## Building $example"
> > +	$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example
> > +done
> > +
> > +echo ""
> > +echo "## $0: Completed OK"
> 
> This last log is uncommon.
> 
Yes, it is. However, due to parallelism, sometimes there is an error
message printed out that scrolls off-screen as the other build processes
come to a halt. I felt it worthwhile to add this at the end to ensure it
was clear whether things succeeded or not. If you prefer, I can change it
to a print on failure?

/Bruce
Thomas Monjalon May 2, 2019, 1:57 p.m. UTC | #4 
02/05/2019 15:21, Bruce Richardson:
> On Thu, May 02, 2019 at 02:38:49PM +0200, Thomas Monjalon wrote:
> > Hi,
> > 
> > I will probably have a ton of comments about adding a new compilation tests,
> > and I think it is a bit late for such an addition.
> > However, all the fixes should go in 19.05.

It would be more reasonnable to leave it for 19.08
and re-spin with fixes only.

> > 26/04/2019 18:50, Bruce Richardson:
> > > The pkg-config file generated as part of the build of DPDK should allow
> > > applications to be built with an installed DPDK. We can test this as
> > > part of the build by doing an install of DPDK to a temporary directory
> > > within the build folder, and by then compiling up a few sample apps
> > > using make working off that directory.
> > > 
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Acked-by: Luca Boccassi <bluca@debian.org>
> > > ---
> > > --- a/devtools/test-meson-builds.sh
> > > +++ b/devtools/test-meson-builds.sh
> > > +##############
> > > +# Test installation of the x86-default target, to be used for checking
> > > +# the sample apps build using the pkg-config file for cflags and libs
> > > +###############
> > 
> > I would prefer simpler comment formatting.
> > It makes this test very special.
> 
> Your comment implies that it is not :-)
> Sure, I can reduce the highlighting.
> 
> > 
> > > +build_path=build-x86-default
> > > +DESTDIR=`pwd`/$build_path/install-root ; export DESTDIR
> > 
> > export DESTDIR=... is not supported everywhere?
> 
> No 100% sure, so left it like this just in case.

Hmmm, i would prefer "export DESTDIR="

> > I prefer new shell substitution syntax $() instead of backquotes.
> > 
> Sure, I agree it's more readable.
> 
> > > +$ninja_cmd -C $build_path install
> > > +
> > > +pc_file=$(find $DESTDIR -name libdpdk.pc)
> > > +PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
> > > +
> > > +# rather than hacking our environment, just edit the .pc file prefix value
> > > +sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file
> > 
> > What is the alternative?
> > Cannot we configure meson with the right prefix?
> 
> See previous discussion.
> Short answer, yes we can, but the prefix does not apply to kernel modules
> as they always install in /lib/modules folder.

We could do better by providing this ability in our
build system without hack.

> > > +for example in helloworld l2fwd l3fwd skeleton timer; do
> > > +	echo "## Building $example"
> > > +	$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example
> > > +done
> > > +
> > > +echo ""
> > > +echo "## $0: Completed OK"
> > 
> > This last log is uncommon.
> > 
> Yes, it is. However, due to parallelism, sometimes there is an error
> message printed out that scrolls off-screen as the other build processes
> come to a halt. I felt it worthwhile to add this at the end to ensure it
> was clear whether things succeeded or not. If you prefer, I can change it
> to a print on failure?

Failures are caught by 'set -e'.
Personnally I don't need such log because my shell prints me an error
when $? is not 0, but I can understand the need.
If you want to print $0, basename may render prettier.
diff mbox series

Patch

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 630a1a6fe..73f993b98 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -15,6 +15,11 @@  srcdir=$(dirname $(readlink -f $0))/..
 MESON=${MESON:-meson}
 use_shared="--default-library=shared"
 
+if command -v gmake >/dev/null 2>&1 ; then
+	MAKE=gmake
+else
+	MAKE=make
+fi
 if command -v ninja >/dev/null 2>&1 ; then
 	ninja_cmd=ninja
 elif command -v ninja-build >/dev/null 2>&1 ; then
@@ -90,3 +95,25 @@  if command -v $c >/dev/null 2>&1 ; then
 			$use_shared --cross-file $f
 	done
 fi
+
+##############
+# Test installation of the x86-default target, to be used for checking
+# the sample apps build using the pkg-config file for cflags and libs
+###############
+build_path=build-x86-default
+DESTDIR=`pwd`/$build_path/install-root ; export DESTDIR
+$ninja_cmd -C $build_path install
+
+pc_file=$(find $DESTDIR -name libdpdk.pc)
+PKG_CONFIG_PATH=$(dirname $pc_file) ; export PKG_CONFIG_PATH
+
+# rather than hacking our environment, just edit the .pc file prefix value
+sed -i -e "s|prefix=|prefix=$DESTDIR|" $pc_file
+
+for example in helloworld l2fwd l3fwd skeleton timer; do
+	echo "## Building $example"
+	$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example
+done
+
+echo ""
+echo "## $0: Completed OK"