Re: [dpdk-dev] [PATCH] build: add pkg-config validation

[Thomas Monjalon <thomas@monjalon.net>]

29/10/2020 10:16, Gregory Etelson:
> 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.

Actually this is only for static linkage of examples with makefiles.

> This patch validates host pkg-config utility and notifies about
> known issues.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
> --- a/buildtools/pkg-config/meson.build
> +++ b/buildtools/pkg-config/meson.build
> +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

Should we restrict this warning to the example makefiles?


> --- /dev/null
> +++ b/buildtools/pkg-config/pkgconfig-validate.sh
> @@ -0,0 +1,38 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +if [ "$#" -ne 1 ]; then
> +	echo "$0: no pkg-config parameter"
> +	exit 1
> +fi

The default pkg-config from path could considered.

> +
> +ret=0
> +PKGCONF="$1"

PKGCONF=${1:-pkg-config}

> +
> +# take the first result only
> +pc_file=$(find "$MESON_BUILD_ROOT" -type f -name 'libdpdk.pc' -print -quit)

It does not look in PKG_CONFIG_PATH.

> +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"
> +PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:$pc_dir"
> +export PKG_CONFIG_PATH
> +
> +# Statically linked private DPDK objects of form
> +# -l:file.a must be positionned between --whole-archive … --no-whole-archive
> +# linker parameters.
> +# Old pkg-config versions misplace --no-whole-archive parameter and put it
> +# next to --whole-archive.
> +"$PKGCONF" --libs --static libdpdk | \
> +grep -q 'whole-archive.*l:lib.*no-whole-archive'
> +if test "$?" -ne 0 ; then
> +	echo "WARNING: invalid pkg-config"
> +	ret=1
> +fi
> +
> +# restore PKG_CONFIG_PATH
> +export PKG_CONFIG_PATH="$__pkg_config_path"

Instead of restoring, you could just set the variable on the
command line calling PKGCONF.

> +exit $ret


> --- a/doc/guides/linux_gsg/sys_reqs.rst
> +++ b/doc/guides/linux_gsg/sys_reqs.rst
> +**Known Issues:**
> +
> +*   pkg-config v0.27 supplied with RHEL-7 does not process correctly libdpdk.pc Libs.private section.

Is it only the RHEL version or all 0.27 packages?

Is it enough to warn on issue, or do we prefer recommend a minimal version?