build: add pkg-config validation
PATCH 1/1 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.
--- 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
buildtools/pkg-config/meson.build +11 -0
@@ -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
buildtools/pkg-config/pkgconfig-validate.sh new file +38 -0
@@ -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
doc/guides/linux_gsg/sys_reqs.rst +5 -0
@@ -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
Signature · 1 line
2.28.0
29/10/2020 10:16, Gregory Etelson:
Quoted 5 lines
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.
Quoted 7 lines
This patch validates host pkg-config utility and notifies about known issues.
Signed-off-by Gregory Etelson <[email protected]>---
buildtools/pkg-config/meson.build This is required for a number of static inline functions in the public headers.' · line 66
Quoted 10 lines
buildtools/pkg-config/meson.build +10
-0+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?
buildtools/pkg-config/pkgconfig-validate.sh line 7
Quoted 10 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +7
-0@@ -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.
buildtools/pkg-config/pkgconfig-validate.sh line 10
Quoted 3 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +3
-0+ +ret=0 +PKGCONF="$1"
PKGCONF=${1:-pkg-config}
buildtools/pkg-config/pkgconfig-validate.sh line 13
Quoted 3 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +3
-0+ +# 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.
Quoted 14 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +14
-0+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
buildtools/pkg-config/pkgconfig-validate.sh line 37
Quoted 10 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +10
-0+# 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.
doc/guides/linux_gsg/sys_reqs.rst Compilation of the DPDK · line 66
Quoted 8 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +1
-0+exit $retdoc/guides/linux_gsg/sys_reqs.rst +3
-0+**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?
Hello Thomas,
Quoted 11 lines
-----Original Message-----
29/10/2020 10:16, Gregory Etelson:
Quoted 5 lines
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.
Quoted 20 lines
Quoted 7 lines
This patch validates host pkg-config utility and notifies about known issues.
Signed-off-by Gregory Etelson <[email protected]>---
buildtools/pkg-config/meson.build This is required for a number of static inline functions in the public headers.' · line 66
Quoted 10 lines
buildtools/pkg-config/meson.build +10
-0+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 +endifShould 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.
Quoted 14 lines
buildtools/pkg-config/pkgconfig-validate.sh line 7
Quoted 11 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +7
-0@@ -0,0 +1,38 @@ +#! /bin/sh +# SPDX-License-Identifier: BSD-3-Clause + +if [ "$#" -ne 1 ]; then + echo "$0: no pkg-config parameter" + exit 1 +fiThe 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.
Quoted 6 lines
buildtools/pkg-config/pkgconfig-validate.sh line 10
Quoted 3 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +3
-0+ +ret=0 +PKGCONF="$1"PKGCONF=${1:-pkg-config}
Please see above.
Quoted 7 lines
buildtools/pkg-config/pkgconfig-validate.sh
Quoted 4 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +4
-0+ +# 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.
Quoted 19 lines
Quoted 19 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +19
-0+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"
Quoted 8 lines
buildtools/pkg-config/pkgconfig-validate.sh line 37
Quoted 5 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +5
-0+ 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.
Quoted 11 lines
doc/guides/linux_gsg/sys_reqs.rst Compilation of the DPDK
Quoted 9 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +1
-0+exit $retdoc/guides/linux_gsg/sys_reqs.rst +3
-0+**Known Issues:** + +* pkg-config v0.27 supplied with RHEL-7 does not process correctlylibdpdk.pc Libs.private section.
Quoted 5 lines
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.
PATCH v2 1/1 build: add pkg-config validation
In reply to Gregory EtelsonDPDK 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.
--- 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
buildtools/pkg-config/meson.build +11 -0
@@ -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
buildtools/pkg-config/pkgconfig-validate.sh new file +40 -0
@@ -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 +
doc/guides/linux_gsg/sys_reqs.rst +5 -0
@@ -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
Signature · 1 line
2.28.0
Quoted 42 lines · 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 <[email protected]>--- 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.shbuildtools/pkg-config/meson.build +11
-0@@ -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 +endifbuildtools/pkg-config/pkgconfig-validate.sh new file +0
-0
buildtools/pkg-config/pkgconfig-validate.sh line 10
Quoted 10 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +10
-0@@ -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?
Hello Bruce,
Quoted 1 line
-----Original Message-----
[snip]
Quoted 6 lines
buildtools/pkg-config/pkgconfig-validate.sh line 10
Quoted 3 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +3
-0+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
PATCH v3 1/1 build: add pkg-config validation
In reply to Gregory EtelsonDPDK 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.
--- 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
buildtools/pkg-config/meson.build +11 -0
@@ -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
buildtools/pkg-config/pkgconfig-validate.sh new file +43 -0
43 lines not on this page · raw message
doc/guides/linux_gsg/sys_reqs.rst +5 -0
@@ -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
Signature · 1 line
2.28.0
Quoted 10 lines · 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 <[email protected]>
All looks reasonably ok to me. Some suggestions inline below which might shorten and simplify the script a bit.
Quoted 37 lines
--- 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.shbuildtools/pkg-config/meson.build +11
-0@@ -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 +endifbuildtools/pkg-config/pkgconfig-validate.sh new file +5
-0@@ -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
buildtools/pkg-config/pkgconfig-validate.sh line 15
Quoted 10 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +10
-0+# 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.
Quoted 4 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +4
-0+ fi + return $ret +} +
buildtools/pkg-config/pkgconfig-validate.sh line 29
Quoted 10 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +10
-0+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.
Quoted 4 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +4
-0+ 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
buildtools/pkg-config/pkgconfig-validate.sh line 43
Quoted 10 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +10
-0+ 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.
Quoted 19 lines
doc/guides/linux_gsg/sys_reqs.rst +5
-0@@ -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 librariesSignature · 1 line
2.28.0
Hello Bruce,
Thank you for the review.
I'll update the script and post a new patch to the mailing list.
Regards,
Gregory
Quoted 157 lines
-----Original Message-----
From: Bruce Richardson <[email protected]>
Sent: Tuesday, November 3, 2020 12:09
To: Gregory Etelson <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; NBU-Contact- Thomas Monjalon <[email protected]>
Subject: Re: [PATCH v3] build: add pkg-config validationExternal email: Use caution opening links or attachments
Quoted 10 lines · 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 <[email protected]>All looks reasonably ok to me. Some suggestions inline below which might shorten and simplify the script a bit.
Acked-by Bruce Richardson <[email protected]>buildtools/pkg-
Quoted 8 lines
--- 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.shbuildtools/pkg- renamed from buildtools/pkg-config/meson.build +0
-0config/meson.build
buildtools/pkg-config/meson.build This is required for a number of static inline
Quoted 4 lines
index
5f19304289..4f907d7638100644buildtools/pkg-config/meson.build +0
-0@@ -53,3 +53,14 @@ This is required for a number of static inlinefunctions in the public headers.'
Review
Quoted 15 lines
buildtools/pkg-config/meson.build +11
-0# 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(quoted diff) +0
-0
b/buildtools/pkg-config/pkgconfig-validate.shbuildtools/pkg-config/pkgconfig-validate.sh line 5
Quoted 10 lines
new file mode 100755
index 0000000000..4b3bd2a9e3buildtools/pkg-config/pkgconfig-validate.sh new file +5
-0@@ -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
buildtools/pkg-config/pkgconfig-validate.sh line 7
Quoted 2 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +2
-0+# linker parameters. +# Old pkg-config versions misplace --no-whole-archive parameter and putit
buildtools/pkg-config/pkgconfig-validate.sh line 15
Quoted 8 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +8
-0+# 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=1Why 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.buildtools/pkg-config/pkgconfig-validate.sh
Quoted 13 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +13
-0+ 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_PATHbuildtools/pkg-config/pkgconfig-validate.sh line 29
Quoted 1 line
buildtools/pkg-config/pkgconfig-validate.sh new file +1
-0+ # check meson template insteadWhy bother checking first? Since all we care about is the pkg-config behaviour, we can just always add on the path to
PKG_CONFIG_PATHand guarantee that way a dpdk file will be found.buildtools/pkg-config/pkgconfig-validate.sh line 30
Quoted 1 line
buildtools/pkg-config/pkgconfig-validate.sh new file +1
-0+ pc_file=$(find "$MESON_BUILD_ROOT" -type f -name 'libdpdk.pc' -quit)
buildtools/pkg-config/pkgconfig-validate.sh line 43
Quoted 13 lines
buildtools/pkg-config/pkgconfig-validate.sh new file +13
-0+ 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 +fiRather 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.Review
Quoted 1 line
(quoted diff) +0
-0
b/doc/guides/linux_gsg/sys_reqs.rstdoc/guides/linux_gsg/sys_reqs.rst Compilation of the DPDK · line 66
Quoted 11 lines
index
6ecdc04aa9..b67da05e13100644doc/guides/linux_gsg/sys_reqs.rst +4
-0@@ -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 correctlylibdpdk.pc Libs.private section.
doc/guides/linux_gsg/sys_reqs.rst Compilation of the DPDK
Quoted 4 lines
doc/guides/linux_gsg/sys_reqs.rst +1
-0+ .. note:: Please ensure that the latest patches are applied to third partylibraries
Quoted 3 lines
--
2.28.0
PATCH v4 1/1 build: add pkg-config validation
In reply to Gregory EtelsonDPDK 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.
--- 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
buildtools/pkg-config/meson.build +11 -0
@@ -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
buildtools/pkg-config/pkgconfig-validate.sh new file +29 -0
@@ -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 "$?"
doc/guides/linux_gsg/sys_reqs.rst +5 -0
@@ -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
Signature · 1 line
2.28.0
Quoted 11 lines · 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 <[email protected]>---
You can keep my ack from previous version.
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.
Quoted 79 lines
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.shbuildtools/pkg-config/meson.build +11
-0@@ -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 +endifbuildtools/pkg-config/pkgconfig-validate.sh new file +29
-0@@ -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 "$?"doc/guides/linux_gsg/sys_reqs.rst +5
-0@@ -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 librariesSignature · 1 line
2.28.0
Quoted 9 lines · 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.
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.
Signature · 1 line
David Marchand
Hello David
Quoted 10 lines
-----Original Message-----
Quoted 8 lines
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.
Quoted 8 lines
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
Quoted 6 lines · Gregory Etelson
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.
Signature · 1 line
David Marchand
PATCH 1/1 doc: notify bug in pkg-config v0.27
In reply to Gregory EtelsonDPDK 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.
--- doc/guides/linux_gsg/sys_reqs.rst | 5 +++++ 1 file changed, 5 insertions(+)
doc/guides/linux_gsg/sys_reqs.rst +5 -0
@@ -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
Signature · 1 line
2.29.2
PATCH v2 1/1 doc: add pkg-config requirement for applications
From: Gregory Etelson <[email protected]>
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.
---
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(+)
doc/guides/linux_gsg/sys_reqs.rst +13 -0
@@ -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 -------------------------
Signature · 1 line
2.28.0
Re: [dpdk-dev] [PATCH v2 1/1] doc: add pkg-config requirement for applications
Quoted 29 lines · Thomas Monjalon
From: Gregory Etelson <[email protected]>
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 <[email protected]>Signed-off-by Thomas Monjalon <[email protected]>---
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(+)doc/guides/linux_gsg/sys_reqs.rst +0
-0@@ -94,6 +94,19 @@ found in that driver's documentation in the relevant DPDK guide document, e.g. :doc:`../nics/index`
doc/guides/linux_gsg/sys_reqs.rst found in that driver's documentation in the relevant DPDK guide document, · line 106
Quoted 10 lines
doc/guides/linux_gsg/sys_reqs.rst +10
-0+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.
doc/guides/linux_gsg/sys_reqs.rst found in that driver's documentation in the relevant DPDK guide document, · line 107
Quoted 1 line
doc/guides/linux_gsg/sys_reqs.rst +1
-0+ 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."
Quoted 8 lines
doc/guides/linux_gsg/sys_reqs.rst +2
-0+ + Running DPDK Applications -------------------------Signature · 1 line
2.28.0
Re: [dpdk-dev] [PATCH v2 1/1] doc: add pkg-config requirement for applications
26/11/2020 17:24, Bruce Richardson:
Quoted 44 lines · Thomas Monjalon
Quoted 29 lines
From: Gregory Etelson <[email protected]>
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 <[email protected]>Signed-off-by Thomas Monjalon <[email protected]>---
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(+)doc/guides/linux_gsg/sys_reqs.rst +0
-0@@ -94,6 +94,19 @@ found in that driver's documentation in the relevant DPDK guide document, e.g. :doc:`../nics/index`doc/guides/linux_gsg/sys_reqs.rst found in that driver's documentation in the relevant DPDK guide document, · line 106
Quoted 10 lines
doc/guides/linux_gsg/sys_reqs.rst +10
-0+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.
Quoted 5 lines
doc/guides/linux_gsg/sys_reqs.rst found in that driver's documentation in the relevant DPDK guide document, · line 107
Quoted 1 line
doc/guides/linux_gsg/sys_reqs.rst +1
-0+ 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.
PATCH v3 1/1 doc: add pkg-config requirement for applications
In reply to Gregory EtelsonFrom: Gregory Etelson <[email protected]>
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.
---
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(+)
doc/guides/linux_gsg/sys_reqs.rst +13 -0
@@ -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 -------------------------
Signature · 1 line
2.28.0
Re: [dpdk-dev] [PATCH v3 1/1] doc: add pkg-config requirement for applications
Quoted 11 lines · Thomas Monjalon
From: Gregory Etelson <[email protected]>
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.
Quoted 6 lines
Signed-off-by Gregory Etelson <[email protected]>Signed-off-by Thomas Monjalon <[email protected]>---
v3: improve wording with Bruce's suggestions
v2: make a more global note about the need for pkg-config in app build
---
Thomas Monjalon
AppliedRe: [dpdk-dev] [PATCH v3 1/1] doc: add pkg-config requirement for applications
26/11/2020 17:46, Bruce Richardson:
Quoted 12 lines · Thomas Monjalon
Quoted 11 lines
From: Gregory Etelson <[email protected]>
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.
Quoted 7 lines
Quoted 6 lines
Signed-off-by Gregory Etelson <[email protected]>Signed-off-by Thomas Monjalon <[email protected]>---
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 <[email protected]>
Applied