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.richard...@intel.com> > > > Acked-by: Luca Boccassi <bl...@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.