[v4] build: add pkg-config validation

DPDK relies on pkg-config(1) to provide correct parameters for
compiler and linker used in application build.
Inaccurate build parameters, produced by pkg-config from DPDK .pc
files could fail application build or cause unpredicted results
during application runtime.

This patch validates host pkg-config utility and notifies about
known issues.

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 buildtools/pkg-config/meson.build           | 11 ++++++++
 buildtools/pkg-config/pkgconfig-validate.sh | 29 +++++++++++++++++++++
 doc/guides/linux_gsg/sys_reqs.rst           |  5 ++++
 3 files changed, 45 insertions(+)
 create mode 100755 buildtools/pkg-config/pkgconfig-validate.sh
  

On Thu, Nov 05, 2020 at 02:37:03PM +0200, Gregory Etelson wrote:
> DPDK relies on pkg-config(1) to provide correct parameters for
> compiler and linker used in application build.
> Inaccurate build parameters, produced by pkg-config from DPDK .pc
> files could fail application build or cause unpredicted results
> during application runtime.
> 
> This patch validates host pkg-config utility and notifies about
> known issues.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---

You can keep my ack from previous version.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One small note is that I don't believe the 'exit $?' line at the end of the
script is necessary, that is default shell behaviour. However, it probably
doesn't hurt to have it explicitly there.

>  buildtools/pkg-config/meson.build           | 11 ++++++++
>  buildtools/pkg-config/pkgconfig-validate.sh | 29 +++++++++++++++++++++
>  doc/guides/linux_gsg/sys_reqs.rst           |  5 ++++
>  3 files changed, 45 insertions(+)
>  create mode 100755 buildtools/pkg-config/pkgconfig-validate.sh
> 
> diff --git a/buildtools/pkg-config/meson.build b/buildtools/pkg-config/meson.build
> index 5f19304289..4f907d7638 100644
> --- a/buildtools/pkg-config/meson.build
> +++ b/buildtools/pkg-config/meson.build
> @@ -53,3 +53,14 @@ This is required for a number of static inline functions in the public headers.'
>  # For static linking with dependencies as shared libraries,
>  # the internal static libraries must be flagged explicitly.
>  run_command(py3, 'set-static-linker-flags.py', check: true)
> +
> +pkgconf = find_program('pkg-config', 'pkgconf', required: false)
> +if (pkgconf.found())
> +	cmd = run_command('./pkgconfig-validate.sh', pkgconf.path(),
> +			   check:false)
> +	if cmd.returncode() != 0
> +		version = run_command(pkgconf, '--version')
> +		warning('invalid pkg-config version @0@'.format(
> +			version.stdout().strip()))
> +	endif
> +endif
> diff --git a/buildtools/pkg-config/pkgconfig-validate.sh b/buildtools/pkg-config/pkgconfig-validate.sh
> new file mode 100755
> index 0000000000..f5479f999f
> --- /dev/null
> +++ b/buildtools/pkg-config/pkgconfig-validate.sh
> @@ -0,0 +1,29 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +if [ "$#" -ne 1 ]; then
> +	echo "$0: no pkg-config parameter"
> +	exit 1
> +fi
> +PKGCONF="$1"
> +
> +# if pkgconf could not locate libdpdk.pc from existing PKG_CONFIG_PATH
> +# check meson template instead
> +# take the first located file
> +pc_file=$(find "$MESON_BUILD_ROOT" -type f -name 'libdpdk.pc' -print -quit)
> +if [ ! -f "$pc_file" ]; then
> +	echo "$0: cannot locate libdpdk.pc"
> +	exit 1
> +fi
> +pc_dir=$(dirname "$pc_file")
> +PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:$pc_dir"
> +
> +# Statically linked private DPDK objects of form
> +# -l:file.a must be positioned between --whole-archive … --no-whole-archive
> +# linker parameters.
> +# Old pkg-config versions misplace --no-whole-archive parameter and put it
> +# next to --whole-archive.
> +PKG_CONFIG_PATH="$PKG_CONFIG_PATH" \
> +"$PKGCONF" --libs --static libdpdk | \
> +grep -q 'whole-archive.*l:lib.*no-whole-archive'
> +exit "$?"
> diff --git a/doc/guides/linux_gsg/sys_reqs.rst b/doc/guides/linux_gsg/sys_reqs.rst
> index 6ecdc04aa9..b67da05e13 100644
> --- a/doc/guides/linux_gsg/sys_reqs.rst
> +++ b/doc/guides/linux_gsg/sys_reqs.rst
> @@ -60,6 +60,11 @@ Compilation of the DPDK
>  
>  *   Linux kernel headers or sources required to build kernel modules.
>  
> +
> +**Known Issues:**
> +
> +*   pkg-config v0.27 supplied with RHEL-7 does not process correctly libdpdk.pc Libs.private section.
> +
>  .. note::
>  
>     Please ensure that the latest patches are applied to third party libraries
> -- 
> 2.28.0
>
  

On Thu, Nov 5, 2020 at 1:37 PM Gregory Etelson <getelson@nvidia.com> wrote:
>
> DPDK relies on pkg-config(1) to provide correct parameters for
> compiler and linker used in application build.
> Inaccurate build parameters, produced by pkg-config from DPDK .pc
> files could fail application build or cause unpredicted results
> during application runtime.
>
> This patch validates host pkg-config utility and notifies about
> known issues.

I am skeptical about this patch.
You want to inform application developers that linking against this
dpdk will fail?
The warning is displayed at configure time of the dpdk.
  

Hello David

> -----Original Message-----
> > DPDK relies on pkg-config(1) to provide correct parameters for
> > compiler and linker used in application build.
> > Inaccurate build parameters, produced by pkg-config from DPDK .pc
> > files could fail application build or cause unpredicted results during
> > application runtime.
> >
> > This patch validates host pkg-config utility and notifies about known
> > issues.
> 
> I am skeptical about this patch.
> You want to inform application developers that linking against this dpdk
> will fail?
> The warning is displayed at configure time of the dpdk.
> 
> 
> --
> David Marchand

The patch notifies about invalid pkg-config. Application that uses such
pkg-config will receive wrong linker parameters. These parameters will not
fail link procedure, but may produce invalid output and as the result,
application will have unpredicted behavior.
Optimal notification would be during application build procedure, but
I cannot see how it's possible to interact with an external build.

Regards,
Gregory
  

On Fri, Nov 13, 2020 at 4:16 PM Gregory Etelson <getelson@nvidia.com> wrote:
> The patch notifies about invalid pkg-config. Application that uses such
> pkg-config will receive wrong linker parameters. These parameters will not
> fail link procedure, but may produce invalid output and as the result,
> application will have unpredicted behavior.
> Optimal notification would be during application build procedure, but
> I cannot see how it's possible to interact with an external build.

Me neither, so I don't see the point in adding a warning the final
user won't catch.
Documenting this known issue seems enough.