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

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

[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.

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 | 38 +++++++++++++++++++++
 doc/guides/linux_gsg/sys_reqs.rst           |  5 +++
 3 files changed, 54 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..57ee096988 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
\ No newline at end of file
diff --git a/buildtools/pkg-config/pkgconfig-validate.sh b/buildtools/pkg-config/pkgconfig-validate.sh
new file mode 100755
index 0000000000..b606bde908
--- /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
+
+ret=0
+PKGCONF="$1"
+
+# take the first result only
+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"
+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"
+exit $ret
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

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

[Thomas Monjalon]

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?

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

[Gregory Etelson]

Hello Thomas,

> -----Original Message-----
> 
> 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.
>

In our case, pkg-config produced wrong output for statically linked
external application.
General case of error in pkg-config output can lead to any kind of build
or execution fault. 
 
> > 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?
>

If DPDK build infrastructure detects wrong pkg-config, it must
notify all applications, internal and external, about the
potential build fault.
 
> 
> > --- /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.
> 

Host can use ether pkg-config or pkgconf utility for
build configuration. The script let the meson build
system to query host what exact utility is used.
Meson passes the query result to the validation script. 

> > +
> > +ret=0
> > +PKGCONF="$1"
> 
> PKGCONF=${1:-pkg-config}
> 

Please see above.

> > +
> > +# 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.
>

Meson stores libdpdk.pc template under MESON_BUILD_ROOT. That template has enough
information to run validation queries.
 
> > +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.

Not sure I understand what did mean here.

> 
> > +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?
> 

We can both warn about pkg-config versions with known issues and validate
host configuration utility during run-time.
   

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

[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.

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 | 40 +++++++++++++++++++++
 doc/guides/linux_gsg/sys_reqs.rst           |  5 +++
 3 files changed, 56 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..61ddd6d5a3
--- /dev/null
+++ b/buildtools/pkg-config/pkgconfig-validate.sh
@@ -0,0 +1,40 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+
+# 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.
+test1_static_libs_order () {
+	PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:$pc_dir" \
+	"$PKGCONF" --libs --static libdpdk | \
+	grep -q 'whole-archive.*l:lib.*no-whole-archive'
+	if test "$?" -ne 0 ; then
+		echo "WARNING: invalid static libraries order"
+		ret=1
+	fi
+	return $ret
+}
+
+if [ "$#" -ne 1 ]; then
+	echo "$0: no pkg-config parameter"
+	exit 1
+fi
+PKGCONF="$1"
+
+# take the first result only
+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")
+
+ret=0
+
+test1_static_libs_order
+if [ $ret -ne 0 ]; then
+	exit $ret
+fi
+
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

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

[Bruce Richardson]

On Mon, Nov 02, 2020 at 08:45:48AM +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>
> ---
>  buildtools/pkg-config/meson.build           | 11 ++++++
>  buildtools/pkg-config/pkgconfig-validate.sh | 40 +++++++++++++++++++++
>  doc/guides/linux_gsg/sys_reqs.rst           |  5 +++
>  3 files changed, 56 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..61ddd6d5a3
> --- /dev/null
> +++ b/buildtools/pkg-config/pkgconfig-validate.sh
> @@ -0,0 +1,40 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +# 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.
> +test1_static_libs_order () {
> +	PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:$pc_dir" \

Should the $pc_dir not be put first, in case there are older .pc files
around for DPDK?

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

[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.

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 | 43 +++++++++++++++++++++
 doc/guides/linux_gsg/sys_reqs.rst           |  5 +++
 3 files changed, 59 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..4b3bd2a9e3
--- /dev/null
+++ b/buildtools/pkg-config/pkgconfig-validate.sh
@@ -0,0 +1,43 @@
+#! /bin/sh
+# SPDX-License-Identifier: BSD-3-Clause
+
+# 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.
+test1_static_libs_order () {
+	PKG_CONFIG_PATH="$PKG_CONFIG_PATH" \
+	"$PKGCONF" --libs --static libdpdk | \
+	grep -q 'whole-archive.*l:lib.*no-whole-archive'
+	if test "$?" -ne 0 ; then
+		echo "WARNING: invalid static libraries order"
+		ret=1
+	fi
+	return $ret
+}
+
+if [ "$#" -ne 1 ]; then
+	echo "$0: no pkg-config parameter"
+	exit 1
+fi
+PKGCONF="$1"
+
+$PKGCONF --exists libdpdk
+if [ $? -ne 0 ]; then
+	# pkgconf could not locate libdpdk.pc from existing PKG_CONFIG_PATH
+	# check meson template instead
+	pc_file=$(find "$MESON_BUILD_ROOT" -type f -name 'libdpdk.pc' -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"
+fi
+
+ret=0
+test1_static_libs_order
+if [ $ret -ne 0 ]; then
+	exit $ret
+fi
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

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

[Gregory Etelson]

Hello Bruce,

> -----Original Message-----

[snip]

> > +put it # next to --whole-archive.
> > +test1_static_libs_order () {
> > +     PKG_CONFIG_PATH="${PKG_CONFIG_PATH}:$pc_dir" \
> 
> Should the $pc_dir not be put first, in case there are older .pc files
> around for DPDK?

I posted a v3 update for the patch.
PKG_CONFIG_PATH will be updated if pkgconf could not locate libdpdk.pc

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

[Bruce Richardson]

On Mon, Nov 02, 2020 at 09:34:26PM +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>

All looks reasonably ok to me. Some suggestions inline below which might
shorten and simplify the script a bit.

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

> ---
>  buildtools/pkg-config/meson.build           | 11 ++++++
>  buildtools/pkg-config/pkgconfig-validate.sh | 43 +++++++++++++++++++++
>  doc/guides/linux_gsg/sys_reqs.rst           |  5 +++
>  3 files changed, 59 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..4b3bd2a9e3
> --- /dev/null
> +++ b/buildtools/pkg-config/pkgconfig-validate.sh
> @@ -0,0 +1,43 @@
> +#! /bin/sh
> +# SPDX-License-Identifier: BSD-3-Clause
> +
> +# 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.
> +test1_static_libs_order () {
> +	PKG_CONFIG_PATH="$PKG_CONFIG_PATH" \
> +	"$PKGCONF" --libs --static libdpdk | \
> +	grep -q 'whole-archive.*l:lib.*no-whole-archive'
> +	if test "$?" -ne 0 ; then
> +		echo "WARNING: invalid static libraries order"
> +		ret=1

Why not just set "ret=$?" before the condition check? Save having to
pre-init ret to 0 and having it as a global variable.

Also, since the meson.build file has the error printout, you can consider
dropping the warning text too, in which case you can have the function just
return the return-code from grep itself.

> +	fi
> +	return $ret
> +}
> +
> +if [ "$#" -ne 1 ]; then
> +	echo "$0: no pkg-config parameter"
> +	exit 1
> +fi
> +PKGCONF="$1"
> +
> +$PKGCONF --exists libdpdk
> +if [ $? -ne 0 ]; then
> +	# pkgconf could not locate libdpdk.pc from existing PKG_CONFIG_PATH
> +	# check meson template instead

Why bother checking first? Since all we care about is the pkg-config
behaviour, we can just always add on the path to PKG_CONFIG_PATH and
guarantee that way a dpdk file will be found.

> +	pc_file=$(find "$MESON_BUILD_ROOT" -type f -name 'libdpdk.pc' -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"
> +fi
> +
> +ret=0
> +test1_static_libs_order
> +if [ $ret -ne 0 ]; then
> +	exit $ret
> +fi

Rather than branching, why not just call "exit $ret"?

Given that the return code from the script will be the result of the last
command run, and if we are ok to dropping the print of the warning, I think
the test function can be dropped and the last line of the script just made
the call to pkg-config and grep.

> 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
> 

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

[Gregory Etelson]

Hello Bruce,

Thank you for the review.
I'll update the script and post a new patch to the mailing list.

Regards,
Gregory

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, November 3, 2020 12:09
> To: Gregory Etelson <getelson@nvidia.com>
> Cc: dev@dpdk.org; bluca@debian.org; christian.ehrhardt@canonical.com;
> david.marchand@redhat.com; ktraynor@redhat.com; Matan Azrad
> <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; NBU-Contact-
> Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [PATCH v3] build: add pkg-config validation
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, Nov 02, 2020 at 09:34:26PM +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>
> 
> All looks reasonably ok to me. Some suggestions inline below which might
> shorten and simplify the script a bit.
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> > ---
> >  buildtools/pkg-config/meson.build           | 11 ++++++
> >  buildtools/pkg-config/pkgconfig-validate.sh | 43 +++++++++++++++++++++
> >  doc/guides/linux_gsg/sys_reqs.rst           |  5 +++
> >  3 files changed, 59 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..4b3bd2a9e3
> > --- /dev/null
> > +++ b/buildtools/pkg-config/pkgconfig-validate.sh
> > @@ -0,0 +1,43 @@
> > +#! /bin/sh
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +
> > +# 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.
> > +test1_static_libs_order () {
> > +     PKG_CONFIG_PATH="$PKG_CONFIG_PATH" \
> > +     "$PKGCONF" --libs --static libdpdk | \
> > +     grep -q 'whole-archive.*l:lib.*no-whole-archive'
> > +     if test "$?" -ne 0 ; then
> > +             echo "WARNING: invalid static libraries order"
> > +             ret=1
> 
> Why not just set "ret=$?" before the condition check? Save having to
> pre-init ret to 0 and having it as a global variable.
> 
> Also, since the meson.build file has the error printout, you can consider
> dropping the warning text too, in which case you can have the function
> just
> return the return-code from grep itself.
> 
> > +     fi
> > +     return $ret
> > +}
> > +
> > +if [ "$#" -ne 1 ]; then
> > +     echo "$0: no pkg-config parameter"
> > +     exit 1
> > +fi
> > +PKGCONF="$1"
> > +
> > +$PKGCONF --exists libdpdk
> > +if [ $? -ne 0 ]; then
> > +     # pkgconf could not locate libdpdk.pc from existing
> PKG_CONFIG_PATH
> > +     # check meson template instead
> 
> Why bother checking first? Since all we care about is the pkg-config
> behaviour, we can just always add on the path to PKG_CONFIG_PATH and
> guarantee that way a dpdk file will be found.
> 
> > +     pc_file=$(find "$MESON_BUILD_ROOT" -type f -name 'libdpdk.pc' -
> 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"
> > +fi
> > +
> > +ret=0
> > +test1_static_libs_order
> > +if [ $ret -ne 0 ]; then
> > +     exit $ret
> > +fi
> 
> Rather than branching, why not just call "exit $ret"?
> 
> Given that the return code from the script will be the result of the last
> command run, and if we are ok to dropping the print of the warning, I
> think
> the test function can be dropped and the last line of the script just made
> the call to pkg-config and grep.
> 
> > 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
> >

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

[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.

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

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

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

[Bruce Richardson]

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
> 

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

[David Marchand]

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.


-- 
David Marchand

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

[Gregory Etelson]

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 

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

[David Marchand]

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.


-- 
David Marchand

[dpdk-dev] [PATCH] doc: notify bug in pkg-config v0.27

[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.

Update system requirements doc about a bug in pkg-config v0.27 used in
RH-7.

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
---
 doc/guides/linux_gsg/sys_reqs.rst | 5 +++++
 1 file changed, 5 insertions(+)

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.29.2

[dpdk-dev] [PATCH v2 1/1] doc: add pkg-config requirement for applications

[Thomas Monjalon]

From: Gregory Etelson <getelson@nvidia.com>

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.

Update system requirements doc about a bug in pkg-config v0.27
used in RHEL-7.

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---

v2: make a more global note about the need for pkg-config in app build

---
 doc/guides/linux_gsg/sys_reqs.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/linux_gsg/sys_reqs.rst b/doc/guides/linux_gsg/sys_reqs.rst
index 6ecdc04aa9..ab38284950 100644
--- a/doc/guides/linux_gsg/sys_reqs.rst
+++ b/doc/guides/linux_gsg/sys_reqs.rst
@@ -94,6 +94,19 @@ found in that driver's documentation in the relevant DPDK guide document,
 e.g. :doc:`../nics/index`
 
 
+Building DPDK Applications
+--------------------------
+
+The tool pkg-config or pkgconf, integrated in most build systems,
+must be used to parse options and dependencies from libdpdk.pc.
+
+.. note::
+
+   pkg-config 0.27, supplied with RHEL-7,
+   does not process correctly Libs.private section,
+   resulting in misses in statically linked applications.
+
+
 Running DPDK Applications
 -------------------------
 
-- 
2.28.0

Re: [dpdk-dev] [PATCH v2 1/1] doc: add pkg-config requirement for applications

[Bruce Richardson]

On Thu, Nov 26, 2020 at 04:42:18PM +0100, Thomas Monjalon wrote:
> From: Gregory Etelson <getelson@nvidia.com>
> 
> 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.
> 
> Update system requirements doc about a bug in pkg-config v0.27
> used in RHEL-7.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> 
> v2: make a more global note about the need for pkg-config in app build
> 
> ---
>  doc/guides/linux_gsg/sys_reqs.rst | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/doc/guides/linux_gsg/sys_reqs.rst b/doc/guides/linux_gsg/sys_reqs.rst
> index 6ecdc04aa9..ab38284950 100644
> --- a/doc/guides/linux_gsg/sys_reqs.rst
> +++ b/doc/guides/linux_gsg/sys_reqs.rst
> @@ -94,6 +94,19 @@ found in that driver's documentation in the relevant DPDK guide document,
>  e.g. :doc:`../nics/index`
>  
>  
> +Building DPDK Applications
> +--------------------------
> +
> +The tool pkg-config or pkgconf, integrated in most build systems,
> +must be used to parse options and dependencies from libdpdk.pc.
> +
> +.. note::
> +
> +   pkg-config 0.27, supplied with RHEL-7,
> +   does not process correctly Libs.private section,

"correctly process the Lib.private..." or "process the Libs.private section
correctly..." would read better, I think.

> +   resulting in misses in statically linked applications.

"misses" is a strange term to use here, and I think it needs clarification
on what exactly is being missed. How about generalizing it a bit:
"resulting in statically linked applications not being linked properly."

> +
> +
>  Running DPDK Applications
>  -------------------------
>  
> -- 
> 2.28.0
> 

Re: [dpdk-dev] [PATCH v2 1/1] doc: add pkg-config requirement for applications

[Thomas Monjalon]

26/11/2020 17:24, Bruce Richardson:
> On Thu, Nov 26, 2020 at 04:42:18PM +0100, Thomas Monjalon wrote:
> > From: Gregory Etelson <getelson@nvidia.com>
> > 
> > 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.
> > 
> > Update system requirements doc about a bug in pkg-config v0.27
> > used in RHEL-7.
> > 
> > Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > 
> > v2: make a more global note about the need for pkg-config in app build
> > 
> > ---
> >  doc/guides/linux_gsg/sys_reqs.rst | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/doc/guides/linux_gsg/sys_reqs.rst b/doc/guides/linux_gsg/sys_reqs.rst
> > index 6ecdc04aa9..ab38284950 100644
> > --- a/doc/guides/linux_gsg/sys_reqs.rst
> > +++ b/doc/guides/linux_gsg/sys_reqs.rst
> > @@ -94,6 +94,19 @@ found in that driver's documentation in the relevant DPDK guide document,
> >  e.g. :doc:`../nics/index`
> >  
> >  
> > +Building DPDK Applications
> > +--------------------------
> > +
> > +The tool pkg-config or pkgconf, integrated in most build systems,
> > +must be used to parse options and dependencies from libdpdk.pc.
> > +
> > +.. note::
> > +
> > +   pkg-config 0.27, supplied with RHEL-7,
> > +   does not process correctly Libs.private section,
> 
> "correctly process the Lib.private..." or "process the Libs.private section
> correctly..." would read better, I think.
> 
> > +   resulting in misses in statically linked applications.
> 
> "misses" is a strange term to use here, and I think it needs clarification
> on what exactly is being missed. How about generalizing it a bit:
> "resulting in statically linked applications not being linked properly."

I agree, I'll do a v3, thanks.

[dpdk-dev] [PATCH v3 1/1] doc: add pkg-config requirement for applications

[Thomas Monjalon]

From: Gregory Etelson <getelson@nvidia.com>

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.

Update system requirements doc about a bug in pkg-config v0.27
used in RHEL-7.

Signed-off-by: Gregory Etelson <getelson@nvidia.com>
Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
v3: improve wording with Bruce's suggestions
v2: make a more global note about the need for pkg-config in app build
---
 doc/guides/linux_gsg/sys_reqs.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/linux_gsg/sys_reqs.rst b/doc/guides/linux_gsg/sys_reqs.rst
index dadb23fc89..9116616fe9 100644
--- a/doc/guides/linux_gsg/sys_reqs.rst
+++ b/doc/guides/linux_gsg/sys_reqs.rst
@@ -94,6 +94,19 @@ found in that driver's documentation in the relevant DPDK guide document,
 e.g. :doc:`../nics/index`
 
 
+Building DPDK Applications
+--------------------------
+
+The tool pkg-config or pkgconf, integrated in most build systems,
+must be used to parse options and dependencies from libdpdk.pc.
+
+.. note::
+
+   pkg-config 0.27, supplied with RHEL-7,
+   does not process the Libs.private section correctly,
+   resulting in statically linked applications not being linked properly.
+
+
 Running DPDK Applications
 -------------------------
 
-- 
2.28.0

Re: [dpdk-dev] [PATCH v3 1/1] doc: add pkg-config requirement for applications

[Bruce Richardson]

On Thu, Nov 26, 2020 at 05:43:28PM +0100, Thomas Monjalon wrote:
> From: Gregory Etelson <getelson@nvidia.com>
> 
> 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.
> 
> Update system requirements doc about a bug in pkg-config v0.27
> used in RHEL-7.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
> v3: improve wording with Bruce's suggestions
> v2: make a more global note about the need for pkg-config in app build
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v3 1/1] doc: add pkg-config requirement for applications

[Thomas Monjalon]

26/11/2020 17:46, Bruce Richardson:
> On Thu, Nov 26, 2020 at 05:43:28PM +0100, Thomas Monjalon wrote:
> > From: Gregory Etelson <getelson@nvidia.com>
> > 
> > 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.
> > 
> > Update system requirements doc about a bug in pkg-config v0.27
> > used in RHEL-7.
> > 
> > Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> > v3: improve wording with Bruce's suggestions
> > v2: make a more global note about the need for pkg-config in app build
> > ---
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied