Re: [dpdk-dev] [PATCH 3/4] devtools/test-meson-builds: add testing of pkg-config file

[Bruce Richardson <bruce.richardson@intel.com>]

On Wed, Apr 24, 2019 at 02:37:58PM +0100, Luca Boccassi wrote:
> On Wed, 2019-04-24 at 13:31 +0100, Bruce Richardson wrote:
> > On Wed, Apr 24, 2019 at 12:02:24PM +0100, Luca Boccassi wrote:
> > > On Wed, 2019-04-24 at 11:41 +0100, Bruce Richardson wrote:
> > > > On Wed, Apr 24, 2019 at 10:22:04AM +0100, Luca Boccassi wrote:
> > > > > On Tue, 2019-04-23 at 23:06 +0100, Bruce Richardson wrote:
> > > > > > 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
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > --- devtools/test-meson-builds.sh | 17 +++++++++++++++++ 1
> > > > > > file
> > > > > > changed, 17 insertions(+)
> > > > > > 
> > > > > > diff --git a/devtools/test-meson-builds.sh b/devtools/test-
> > > > > > meson-
> > > > > > builds.sh index 630a1a6fe..dfba2a782 100755 ---
> > > > > > a/devtools/test-meson-builds.sh +++ b/devtools/test-meson-
> > > > > > builds.sh @@
> > > > > > -90,3 +90,20 @@ 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
> > > > > > +PKG_CONFIG_PATH=$DESTDIR/usr/local/lib64/pkgconfig ;
> > > > > > export PKG_CONFIG_PATH +$ninja_cmd -C $build_path install +
> > > > > > +#
> > > > > > rather
> > > > > > than hacking our environment, just edit the .pc file prefix
> > > > > > value
> > > > > > +sed
> > > > > > -i "s|prefix=|prefix=$DESTDIR|" $PKG_CONFIG_PATH/libdpdk.pc
> > > > > 
> > > > > What about just using meson's prefix option instead? Which is
> > > > > how
> > > > > it
> > > > > would be used in a real use case
> > > > > 
> > > > 
> > > > I don't think that would fully work, as my understanding is that
> > > > the
> > > > prefix
> > > > option would apply only to the /usr/local parts, but not to the
> > > > kernel
> > > > modules which would still try and install in /lib/.
> > > > 
> > > > /Bruce
> > > 
> > > What about doing a meson configure -Denable_kmods=false before the
> > > ninja install? The modules are not needed for that test anyway,
> > > right?
> > > Alternatively, the kernel src dir could be symlinked in the build
> > > path,
> > > and the kernel_dir option could be used
> > > 
> > > I'm just worried that the test should be as "realistic" as
> > > possible, to
> > > avoid missing something
> > > 
> > 
> > Yes, I did think of that too, but it does mean doing another
> > configuration
> > run in meson, and possibly a rebuild too if the rte_build_config.h
> > file
> > changes. Therefore I decided to use DESTDIR for the sake of speed
> > here. I
> > assumed there would be a pkg-config variable to adjust the output
> > paths
> > based on a sysroot, but couldn't find a suitable one.
> > 
> > In any case, I'll see about changing things as you suggest in V2 -
> > correctness is more important that speed here.
> > 
> > /Bruce
> 
> There actually is a pkg-config binary option, I just tried and it works
> (it seems to be disabled by default on Debian and derivatives): --
> define-prefix
> 
Any cmdline options to pkg-config don't solve the problem here as it means
that the makefiles for any app need to be modified to use all those.

Also, I've been looking at the option you suggest of disabling the kernel
modules for the install step - the problem that this brings is that it either:
* disables them permanently for the default build, meaning subsequent runs
  may fail to catch errors
* causes us to constantly reconfigure the build directory with/without
  the kmod setting, causing unnecessary work and slowdown in the script.

A third solution is to use a separate build folder for the pkg-config test
builds, but I think we have enough builds already in the setup without adding
another one.

All-in-all, I feel at this point that the original solution of making a small
change to the pkg-config file manually is the best solution for now. I don't
see it as being terribly fragile, and it should catch 95% of problems with
the pkg-config files. I suggest that any rework be looked at in a later set
to improve things.

Regards,
/Bruce