[dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Thomas Monjalon]

PKG_CONFIG_PATH is specific to each target, so it must be empty
before configuring each build from the file according to DPDK_TARGET.
Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
and is prone to confusion.

DPDK_MESON_OPTIONS might take a global initial value from environment
to customize a build test from the shell. Example:
	DPDK_MESON_OPTIONS="b_lto=true"
Some target-specific options can be added in the configuration file:
	DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"

Fixes: 272236741258 ("devtools: load target-specific compilation environment")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/test-meson-builds.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 0c95d1cc98..6d265f6ab3 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -38,10 +38,10 @@ else
 fi
 
 default_path=$PATH
-default_pkgpath=$PKG_CONFIG_PATH
 default_cppflags=$CPPFLAGS
 default_cflags=$CFLAGS
 default_ldflags=$LDFLAGS
+default_meson_options=$DPDK_MESON_OPTIONS
 
 check_cc_flags () # <flag to check> <flag2> ...
 {
@@ -53,11 +53,11 @@ load_env () # <target compiler>
 {
 	targetcc=$1
 	export PATH=$default_path
-	export PKG_CONFIG_PATH=$default_pkgpath
+	export PKG_CONFIG_PATH= # global default makes no sense
 	export CPPFLAGS=$default_cppflags
 	export CFLAGS=$default_cflags
 	export LDFLAGS=$default_ldflags
-	unset DPDK_MESON_OPTIONS
+	export DPDK_MESON_OPTIONS=$default_meson_options
 	if [ -n "$target_override" ] ; then
 		DPDK_TARGET=$target_override
 	elif command -v $targetcc >/dev/null 2>&1 ; then
-- 
2.28.0

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Bruce Richardson]

On Mon, Nov 09, 2020 at 10:00:08PM +0100, Thomas Monjalon wrote:
> PKG_CONFIG_PATH is specific to each target, so it must be empty
> before configuring each build from the file according to DPDK_TARGET.
> Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> and is prone to confusion.
> 
> DPDK_MESON_OPTIONS might take a global initial value from environment
> to customize a build test from the shell. Example:
> 	DPDK_MESON_OPTIONS="b_lto=true"
> Some target-specific options can be added in the configuration file:
> 	DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"
> 
> Fixes: 272236741258 ("devtools: load target-specific compilation environment")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---

Patch looks fine, but I think the log (and comments in the code too,
perhaps) should make it clear that the PKG_CONFIG_PATH can be set in the
local config file, to make it available to the script.

/Bruce

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Thomas Monjalon]

10/11/2020 11:14, Bruce Richardson:
> On Mon, Nov 09, 2020 at 10:00:08PM +0100, Thomas Monjalon wrote:
> > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > before configuring each build from the file according to DPDK_TARGET.
> > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > and is prone to confusion.
> > 
> > DPDK_MESON_OPTIONS might take a global initial value from environment
> > to customize a build test from the shell. Example:
> > 	DPDK_MESON_OPTIONS="b_lto=true"
> > Some target-specific options can be added in the configuration file:
> > 	DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"
> > 
> > Fixes: 272236741258 ("devtools: load target-specific compilation environment")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> 
> Patch looks fine, but I think the log (and comments in the code too,
> perhaps) should make it clear that the PKG_CONFIG_PATH can be set in the
> local config file, to make it available to the script.

OK, I'll look at improving the "doc".
We could also provide a template for the config file.

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Bruce Richardson]

On Tue, Nov 10, 2020 at 11:45:52AM +0100, Thomas Monjalon wrote:
> 10/11/2020 11:14, Bruce Richardson:
> > On Mon, Nov 09, 2020 at 10:00:08PM +0100, Thomas Monjalon wrote:
> > > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > > before configuring each build from the file according to DPDK_TARGET.
> > > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > > and is prone to confusion.
> > > 
> > > DPDK_MESON_OPTIONS might take a global initial value from environment
> > > to customize a build test from the shell. Example:
> > > 	DPDK_MESON_OPTIONS="b_lto=true"
> > > Some target-specific options can be added in the configuration file:
> > > 	DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"
> > > 
> > > Fixes: 272236741258 ("devtools: load target-specific compilation environment")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > 
> > Patch looks fine, but I think the log (and comments in the code too,
> > perhaps) should make it clear that the PKG_CONFIG_PATH can be set in the
> > local config file, to make it available to the script.
> 
> OK, I'll look at improving the "doc".
> We could also provide a template for the config file.
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-stable] [dpdk-dev] [PATCH 1/1] devtools: fix build test config inheritance from env

[Jerin Jacob]

On Tue, Nov 10, 2020 at 4:51 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Tue, Nov 10, 2020 at 11:45:52AM +0100, Thomas Monjalon wrote:
> > 10/11/2020 11:14, Bruce Richardson:
> > > On Mon, Nov 09, 2020 at 10:00:08PM +0100, Thomas Monjalon wrote:
> > > > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > > > before configuring each build from the file according to DPDK_TARGET.
> > > > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > > > and is prone to confusion.
> > > >
> > > > DPDK_MESON_OPTIONS might take a global initial value from environment
> > > > to customize a build test from the shell. Example:
> > > >   DPDK_MESON_OPTIONS="b_lto=true"
> > > > Some target-specific options can be added in the configuration file:
> > > >   DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"
> > > >
> > > > Fixes: 272236741258 ("devtools: load target-specific compilation environment")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > > ---
> > >
> > > Patch looks fine, but I think the log (and comments in the code too,
> > > perhaps) should make it clear that the PKG_CONFIG_PATH can be set in the
> > > local config file, to make it available to the script.
> >
> > OK, I'll look at improving the "doc".
> > We could also provide a template for the config file.
> >
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Tested-by: Jerin Jacob <jerinj@marvell.com>



>

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Ferruh Yigit]

On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
> PKG_CONFIG_PATH is specific to each target, so it must be empty
> before configuring each build from the file according to DPDK_TARGET.
> Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> and is prone to confusion.
> 
> DPDK_MESON_OPTIONS might take a global initial value from environment
> to customize a build test from the shell. Example:
> 	DPDK_MESON_OPTIONS="b_lto=true"
> Some target-specific options can be added in the configuration file:
> 	DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"
> 
> Fixes: 272236741258 ("devtools: load target-specific compilation environment")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>   devtools/test-meson-builds.sh | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 0c95d1cc98..6d265f6ab3 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -38,10 +38,10 @@ else
>   fi
>   
>   default_path=$PATH
> -default_pkgpath=$PKG_CONFIG_PATH
>   default_cppflags=$CPPFLAGS
>   default_cflags=$CFLAGS
>   default_ldflags=$LDFLAGS
> +default_meson_options=$DPDK_MESON_OPTIONS
>   
>   check_cc_flags () # <flag to check> <flag2> ...
>   {
> @@ -53,11 +53,11 @@ load_env () # <target compiler>
>   {
>   	targetcc=$1
>   	export PATH=$default_path
> -	export PKG_CONFIG_PATH=$default_pkgpath
> +	export PKG_CONFIG_PATH= # global default makes no sense
>   	export CPPFLAGS=$default_cppflags
>   	export CFLAGS=$default_cflags
>   	export LDFLAGS=$default_ldflags
> -	unset DPDK_MESON_OPTIONS
> +	export DPDK_MESON_OPTIONS=$default_meson_options
>   	if [ -n "$target_override" ] ; then
>   		DPDK_TARGET=$target_override
>   	elif command -v $targetcc >/dev/null 2>&1 ; then
> 

In same run both 64bit and 32bit builds are done,

At least for my environment,
for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'

What should I set in the config file to support both?

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Thomas Monjalon]

10/11/2020 18:18, Ferruh Yigit:
> On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
> > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > before configuring each build from the file according to DPDK_TARGET.
> > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > and is prone to confusion.
[...]
> In same run both 64bit and 32bit builds are done,

Multiple targets can be built yes.

> At least for my environment,
> for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
> for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'

Not sure you need to set these values in PKG_CONFIG_PATH.
At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.

Let me rephrase the man page of pkg-config:
PKG_CONFIG_LIBDIR is the primary paths list
PKG_CONFIG_PATH is the secondary paths list

> What should I set in the config file to support both?

The standard paths for your 64-bit machine should be built-in
in your pkg-config.
The standard path for 32-bit is already set automatically
in devtools/test-meson-builds.sh.
Only additional specific paths should be set in a config file.

What is a config file? It is loaded by devtools/load-devel-config:
	- /etc/dpdk/devel.config (system-wide)
	- or ~/.config/dpdk/devel.config (user config)
	- or .develconfig (project directory config)
Personally I set all my configs in ~/.config/dpdk/devel.config.
Note that the same file is used to configure multiple tools.

For each build, some variables are reset the variable DPDK_TARGET is set,
and the config file is sourced.
The typical values of DPDK_TARGET are:
	- i386-pc-linux-gnu
	- x86_64-pc-linux-gnu
	- x86_64-w64-mingw32
	- aarch64-linux-gnu
	- powerpc64le-linux-gcc

TLDR, I assume you just want to set an additional 64-bit path,
so the config file should look like:

if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
fi

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Ferruh Yigit]

On 11/10/2020 5:55 PM, Thomas Monjalon wrote:
> 10/11/2020 18:18, Ferruh Yigit:
>> On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
>>> PKG_CONFIG_PATH is specific to each target, so it must be empty
>>> before configuring each build from the file according to DPDK_TARGET.
>>> Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
>>> and is prone to confusion.
> [...]
>> In same run both 64bit and 32bit builds are done,
> 
> Multiple targets can be built yes.
> 
>> At least for my environment,
>> for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
>> for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'
> 
> Not sure you need to set these values in PKG_CONFIG_PATH.
> At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.
> 
> Let me rephrase the man page of pkg-config:
> PKG_CONFIG_LIBDIR is the primary paths list
> PKG_CONFIG_PATH is the secondary paths list
> 
>> What should I set in the config file to support both?
> 
> The standard paths for your 64-bit machine should be built-in
> in your pkg-config.
> The standard path for 32-bit is already set automatically
> in devtools/test-meson-builds.sh.
> Only additional specific paths should be set in a config file.
> 
> What is a config file? It is loaded by devtools/load-devel-config:
> 	- /etc/dpdk/devel.config (system-wide)
> 	- or ~/.config/dpdk/devel.config (user config)
> 	- or .develconfig (project directory config)
> Personally I set all my configs in ~/.config/dpdk/devel.config.
> Note that the same file is used to configure multiple tools.
> 
> For each build, some variables are reset the variable DPDK_TARGET is set,
> and the config file is sourced.
> The typical values of DPDK_TARGET are:
> 	- i386-pc-linux-gnu
> 	- x86_64-pc-linux-gnu
> 	- x86_64-w64-mingw32
> 	- aarch64-linux-gnu
> 	- powerpc64le-linux-gcc
> 
> TLDR, I assume you just want to set an additional 64-bit path,
> so the config file should look like:
> 
> if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
> 	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
> fi
> 
> 

Thanks for the clarification.

Standard paths seems should be covered already in current script, which I was 
trying to do with "export PKG_CONFIG_PATH=$CUSTOM_PKG_CONFIG_PATH" line in my patch,

I may be mixed 'PKG_CONFIG_LIBDIR' vs 'PKG_CONFIG_PATH' usage, let me check it 
in my environment.

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Bruce Richardson]

On Tue, Nov 10, 2020 at 06:09:45PM +0000, Ferruh Yigit wrote:
> On 11/10/2020 5:55 PM, Thomas Monjalon wrote:
> > 10/11/2020 18:18, Ferruh Yigit:
> > > On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
> > > > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > > > before configuring each build from the file according to DPDK_TARGET.
> > > > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > > > and is prone to confusion.
> > [...]
> > > In same run both 64bit and 32bit builds are done,
> > 
> > Multiple targets can be built yes.
> > 
> > > At least for my environment,
> > > for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
> > > for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'
> > 
> > Not sure you need to set these values in PKG_CONFIG_PATH.
> > At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.
> > 
> > Let me rephrase the man page of pkg-config:
> > PKG_CONFIG_LIBDIR is the primary paths list
> > PKG_CONFIG_PATH is the secondary paths list
> > 
> > > What should I set in the config file to support both?
> > 
> > The standard paths for your 64-bit machine should be built-in
> > in your pkg-config.
> > The standard path for 32-bit is already set automatically
> > in devtools/test-meson-builds.sh.
> > Only additional specific paths should be set in a config file.
> > 
> > What is a config file? It is loaded by devtools/load-devel-config:
> > 	- /etc/dpdk/devel.config (system-wide)
> > 	- or ~/.config/dpdk/devel.config (user config)
> > 	- or .develconfig (project directory config)
> > Personally I set all my configs in ~/.config/dpdk/devel.config.
> > Note that the same file is used to configure multiple tools.
> > 
> > For each build, some variables are reset the variable DPDK_TARGET is set,
> > and the config file is sourced.
> > The typical values of DPDK_TARGET are:
> > 	- i386-pc-linux-gnu
> > 	- x86_64-pc-linux-gnu
> > 	- x86_64-w64-mingw32
> > 	- aarch64-linux-gnu
> > 	- powerpc64le-linux-gcc
> > 
> > TLDR, I assume you just want to set an additional 64-bit path,
> > so the config file should look like:
> > 
> > if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
> > 	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
> > fi
> > 
> > 
> 
> Thanks for the clarification.
> 
> Standard paths seems should be covered already in current script, which I
> was trying to do with "export PKG_CONFIG_PATH=$CUSTOM_PKG_CONFIG_PATH" line
> in my patch,
> 
> I may be mixed 'PKG_CONFIG_LIBDIR' vs 'PKG_CONFIG_PATH' usage, let me check
> it in my environment.
> 

For just adding a new custom path for pkg-config using PKG_CONFIG_PATH. For
building anything other than a native 64-bit build you need to override
PKG_CONFIG_LIBDIR and clear PKG_CONFIG_PATH, otherwise the 64-bit packages
will be found from the standard paths if not found in a 32-bit one.

/Bruce

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Thomas Monjalon]

11/11/2020 10:18, Bruce Richardson:
> On Tue, Nov 10, 2020 at 06:09:45PM +0000, Ferruh Yigit wrote:
> > On 11/10/2020 5:55 PM, Thomas Monjalon wrote:
> > > 10/11/2020 18:18, Ferruh Yigit:
> > > > On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
> > > > > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > > > > before configuring each build from the file according to DPDK_TARGET.
> > > > > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > > > > and is prone to confusion.
> > > [...]
> > > > In same run both 64bit and 32bit builds are done,
> > > 
> > > Multiple targets can be built yes.
> > > 
> > > > At least for my environment,
> > > > for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
> > > > for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'
> > > 
> > > Not sure you need to set these values in PKG_CONFIG_PATH.
> > > At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.
> > > 
> > > Let me rephrase the man page of pkg-config:
> > > PKG_CONFIG_LIBDIR is the primary paths list
> > > PKG_CONFIG_PATH is the secondary paths list
> > > 
> > > > What should I set in the config file to support both?
> > > 
> > > The standard paths for your 64-bit machine should be built-in
> > > in your pkg-config.
> > > The standard path for 32-bit is already set automatically
> > > in devtools/test-meson-builds.sh.
> > > Only additional specific paths should be set in a config file.
> > > 
> > > What is a config file? It is loaded by devtools/load-devel-config:
> > > 	- /etc/dpdk/devel.config (system-wide)
> > > 	- or ~/.config/dpdk/devel.config (user config)
> > > 	- or .develconfig (project directory config)
> > > Personally I set all my configs in ~/.config/dpdk/devel.config.
> > > Note that the same file is used to configure multiple tools.
> > > 
> > > For each build, some variables are reset the variable DPDK_TARGET is set,
> > > and the config file is sourced.
> > > The typical values of DPDK_TARGET are:
> > > 	- i386-pc-linux-gnu
> > > 	- x86_64-pc-linux-gnu
> > > 	- x86_64-w64-mingw32
> > > 	- aarch64-linux-gnu
> > > 	- powerpc64le-linux-gcc
> > > 
> > > TLDR, I assume you just want to set an additional 64-bit path,
> > > so the config file should look like:
> > > 
> > > if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
> > > 	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
> > > fi
> > > 
> > > 
> > 
> > Thanks for the clarification.
> > 
> > Standard paths seems should be covered already in current script, which I
> > was trying to do with "export PKG_CONFIG_PATH=$CUSTOM_PKG_CONFIG_PATH" line
> > in my patch,
> > 
> > I may be mixed 'PKG_CONFIG_LIBDIR' vs 'PKG_CONFIG_PATH' usage, let me check
> > it in my environment.
> > 
> 
> For just adding a new custom path for pkg-config using PKG_CONFIG_PATH. For
> building anything other than a native 64-bit build you need to override
> PKG_CONFIG_LIBDIR and clear PKG_CONFIG_PATH, otherwise the 64-bit packages
> will be found from the standard paths if not found in a 32-bit one.

Yes, this is what is done in this patch (clearing PKG_CONFIG_PATH).
May I assume you are all OK with this patch now?

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Bruce Richardson]

On Wed, Nov 11, 2020 at 11:37:41AM +0100, Thomas Monjalon wrote:
> 11/11/2020 10:18, Bruce Richardson:
> > On Tue, Nov 10, 2020 at 06:09:45PM +0000, Ferruh Yigit wrote:
> > > On 11/10/2020 5:55 PM, Thomas Monjalon wrote:
> > > > 10/11/2020 18:18, Ferruh Yigit:
> > > > > On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
> > > > > > PKG_CONFIG_PATH is specific to each target, so it must be empty
> > > > > > before configuring each build from the file according to DPDK_TARGET.
> > > > > > Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> > > > > > and is prone to confusion.
> > > > [...]
> > > > > In same run both 64bit and 32bit builds are done,
> > > > 
> > > > Multiple targets can be built yes.
> > > > 
> > > > > At least for my environment,
> > > > > for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
> > > > > for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'
> > > > 
> > > > Not sure you need to set these values in PKG_CONFIG_PATH.
> > > > At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.
> > > > 
> > > > Let me rephrase the man page of pkg-config:
> > > > PKG_CONFIG_LIBDIR is the primary paths list
> > > > PKG_CONFIG_PATH is the secondary paths list
> > > > 
> > > > > What should I set in the config file to support both?
> > > > 
> > > > The standard paths for your 64-bit machine should be built-in
> > > > in your pkg-config.
> > > > The standard path for 32-bit is already set automatically
> > > > in devtools/test-meson-builds.sh.
> > > > Only additional specific paths should be set in a config file.
> > > > 
> > > > What is a config file? It is loaded by devtools/load-devel-config:
> > > > 	- /etc/dpdk/devel.config (system-wide)
> > > > 	- or ~/.config/dpdk/devel.config (user config)
> > > > 	- or .develconfig (project directory config)
> > > > Personally I set all my configs in ~/.config/dpdk/devel.config.
> > > > Note that the same file is used to configure multiple tools.
> > > > 
> > > > For each build, some variables are reset the variable DPDK_TARGET is set,
> > > > and the config file is sourced.
> > > > The typical values of DPDK_TARGET are:
> > > > 	- i386-pc-linux-gnu
> > > > 	- x86_64-pc-linux-gnu
> > > > 	- x86_64-w64-mingw32
> > > > 	- aarch64-linux-gnu
> > > > 	- powerpc64le-linux-gcc
> > > > 
> > > > TLDR, I assume you just want to set an additional 64-bit path,
> > > > so the config file should look like:
> > > > 
> > > > if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
> > > > 	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
> > > > fi
> > > > 
> > > > 
> > > 
> > > Thanks for the clarification.
> > > 
> > > Standard paths seems should be covered already in current script, which I
> > > was trying to do with "export PKG_CONFIG_PATH=$CUSTOM_PKG_CONFIG_PATH" line
> > > in my patch,
> > > 
> > > I may be mixed 'PKG_CONFIG_LIBDIR' vs 'PKG_CONFIG_PATH' usage, let me check
> > > it in my environment.
> > > 
> > 
> > For just adding a new custom path for pkg-config using PKG_CONFIG_PATH. For
> > building anything other than a native 64-bit build you need to override
> > PKG_CONFIG_LIBDIR and clear PKG_CONFIG_PATH, otherwise the 64-bit packages
> > will be found from the standard paths if not found in a 32-bit one.
> 
> Yes, this is what is done in this patch (clearing PKG_CONFIG_PATH).
> May I assume you are all OK with this patch now?
>
Yes, I previously acked it, I believe.

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Ferruh Yigit]

On 11/11/2020 11:00 AM, Bruce Richardson wrote:
> On Wed, Nov 11, 2020 at 11:37:41AM +0100, Thomas Monjalon wrote:
>> 11/11/2020 10:18, Bruce Richardson:
>>> On Tue, Nov 10, 2020 at 06:09:45PM +0000, Ferruh Yigit wrote:
>>>> On 11/10/2020 5:55 PM, Thomas Monjalon wrote:
>>>>> 10/11/2020 18:18, Ferruh Yigit:
>>>>>> On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
>>>>>>> PKG_CONFIG_PATH is specific to each target, so it must be empty
>>>>>>> before configuring each build from the file according to DPDK_TARGET.
>>>>>>> Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
>>>>>>> and is prone to confusion.
>>>>> [...]
>>>>>> In same run both 64bit and 32bit builds are done,
>>>>>
>>>>> Multiple targets can be built yes.
>>>>>
>>>>>> At least for my environment,
>>>>>> for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
>>>>>> for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'
>>>>>
>>>>> Not sure you need to set these values in PKG_CONFIG_PATH.
>>>>> At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.
>>>>>
>>>>> Let me rephrase the man page of pkg-config:
>>>>> PKG_CONFIG_LIBDIR is the primary paths list
>>>>> PKG_CONFIG_PATH is the secondary paths list
>>>>>
>>>>>> What should I set in the config file to support both?
>>>>>
>>>>> The standard paths for your 64-bit machine should be built-in
>>>>> in your pkg-config.
>>>>> The standard path for 32-bit is already set automatically
>>>>> in devtools/test-meson-builds.sh.
>>>>> Only additional specific paths should be set in a config file.
>>>>>
>>>>> What is a config file? It is loaded by devtools/load-devel-config:
>>>>> 	- /etc/dpdk/devel.config (system-wide)
>>>>> 	- or ~/.config/dpdk/devel.config (user config)
>>>>> 	- or .develconfig (project directory config)
>>>>> Personally I set all my configs in ~/.config/dpdk/devel.config.
>>>>> Note that the same file is used to configure multiple tools.
>>>>>
>>>>> For each build, some variables are reset the variable DPDK_TARGET is set,
>>>>> and the config file is sourced.
>>>>> The typical values of DPDK_TARGET are:
>>>>> 	- i386-pc-linux-gnu
>>>>> 	- x86_64-pc-linux-gnu
>>>>> 	- x86_64-w64-mingw32
>>>>> 	- aarch64-linux-gnu
>>>>> 	- powerpc64le-linux-gcc
>>>>>
>>>>> TLDR, I assume you just want to set an additional 64-bit path,
>>>>> so the config file should look like:
>>>>>
>>>>> if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
>>>>> 	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
>>>>> fi
>>>>>
>>>>>
>>>>
>>>> Thanks for the clarification.
>>>>
>>>> Standard paths seems should be covered already in current script, which I
>>>> was trying to do with "export PKG_CONFIG_PATH=$CUSTOM_PKG_CONFIG_PATH" line
>>>> in my patch,
>>>>
>>>> I may be mixed 'PKG_CONFIG_LIBDIR' vs 'PKG_CONFIG_PATH' usage, let me check
>>>> it in my environment.
>>>>
>>>
>>> For just adding a new custom path for pkg-config using PKG_CONFIG_PATH. For
>>> building anything other than a native 64-bit build you need to override
>>> PKG_CONFIG_LIBDIR and clear PKG_CONFIG_PATH, otherwise the 64-bit packages
>>> will be found from the standard paths if not found in a 32-bit one.
>>
>> Yes, this is what is done in this patch (clearing PKG_CONFIG_PATH).
>> May I assume you are all OK with this patch now?
>>
> Yes, I previously acked it, I believe.
> 

'PKG_CONFIG_LIBDIR' seems replaces the pkgconfig defaults,
'PKG_CONFIG_PATH' adds paths to process *before* built-in ones.

With adjustment according above, all looks good to me.

What about unset the variable "uset PKG_CONFIG_PATH", to set it to empty to get 
rid of the following line from logs:
"Using 'PKG_CONFIG_PATH' from environment with value: ''"

With or without about change,
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-stable] [PATCH 1/1] devtools: fix build test config inheritance from env

[Thomas Monjalon]

11/11/2020 12:13, Ferruh Yigit:
> On 11/11/2020 11:00 AM, Bruce Richardson wrote:
> > On Wed, Nov 11, 2020 at 11:37:41AM +0100, Thomas Monjalon wrote:
> >> 11/11/2020 10:18, Bruce Richardson:
> >>> On Tue, Nov 10, 2020 at 06:09:45PM +0000, Ferruh Yigit wrote:
> >>>> On 11/10/2020 5:55 PM, Thomas Monjalon wrote:
> >>>>> 10/11/2020 18:18, Ferruh Yigit:
> >>>>>> On 11/9/2020 9:00 PM, Thomas Monjalon wrote:
> >>>>>>> PKG_CONFIG_PATH is specific to each target, so it must be empty
> >>>>>>> before configuring each build from the file according to DPDK_TARGET.
> >>>>>>> Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> >>>>>>> and is prone to confusion.
> >>>>> [...]
> >>>>>> In same run both 64bit and 32bit builds are done,
> >>>>>
> >>>>> Multiple targets can be built yes.
> >>>>>
> >>>>>> At least for my environment,
> >>>>>> for 64bit, PKG_CONFIG_PATH should be '/usr/local/lib64/pkgconfig/'
> >>>>>> for 32bit, PKG_CONFIG_PATH should be '/usr/lib/pkgconfig/'
> >>>>>
> >>>>> Not sure you need to set these values in PKG_CONFIG_PATH.
> >>>>> At least /usr/lib/pkgconfig/ is already set in PKG_CONFIG_LIBDIR.
> >>>>>
> >>>>> Let me rephrase the man page of pkg-config:
> >>>>> PKG_CONFIG_LIBDIR is the primary paths list
> >>>>> PKG_CONFIG_PATH is the secondary paths list
> >>>>>
> >>>>>> What should I set in the config file to support both?
> >>>>>
> >>>>> The standard paths for your 64-bit machine should be built-in
> >>>>> in your pkg-config.
> >>>>> The standard path for 32-bit is already set automatically
> >>>>> in devtools/test-meson-builds.sh.
> >>>>> Only additional specific paths should be set in a config file.
> >>>>>
> >>>>> What is a config file? It is loaded by devtools/load-devel-config:
> >>>>> 	- /etc/dpdk/devel.config (system-wide)
> >>>>> 	- or ~/.config/dpdk/devel.config (user config)
> >>>>> 	- or .develconfig (project directory config)
> >>>>> Personally I set all my configs in ~/.config/dpdk/devel.config.
> >>>>> Note that the same file is used to configure multiple tools.
> >>>>>
> >>>>> For each build, some variables are reset the variable DPDK_TARGET is set,
> >>>>> and the config file is sourced.
> >>>>> The typical values of DPDK_TARGET are:
> >>>>> 	- i386-pc-linux-gnu
> >>>>> 	- x86_64-pc-linux-gnu
> >>>>> 	- x86_64-w64-mingw32
> >>>>> 	- aarch64-linux-gnu
> >>>>> 	- powerpc64le-linux-gcc
> >>>>>
> >>>>> TLDR, I assume you just want to set an additional 64-bit path,
> >>>>> so the config file should look like:
> >>>>>
> >>>>> if [ "$DPDK_TARGET" = x86_64-pc-linux-gnu ] ; then
> >>>>> 	export PKG_CONFIG_PATH=$PKG_CONFIG_PATH:/usr/local/lib64/pkgconfig
> >>>>> fi
> >>>>>
> >>>>>
> >>>>
> >>>> Thanks for the clarification.
> >>>>
> >>>> Standard paths seems should be covered already in current script, which I
> >>>> was trying to do with "export PKG_CONFIG_PATH=$CUSTOM_PKG_CONFIG_PATH" line
> >>>> in my patch,
> >>>>
> >>>> I may be mixed 'PKG_CONFIG_LIBDIR' vs 'PKG_CONFIG_PATH' usage, let me check
> >>>> it in my environment.
> >>>>
> >>>
> >>> For just adding a new custom path for pkg-config using PKG_CONFIG_PATH. For
> >>> building anything other than a native 64-bit build you need to override
> >>> PKG_CONFIG_LIBDIR and clear PKG_CONFIG_PATH, otherwise the 64-bit packages
> >>> will be found from the standard paths if not found in a 32-bit one.
> >>
> >> Yes, this is what is done in this patch (clearing PKG_CONFIG_PATH).
> >> May I assume you are all OK with this patch now?
> >>
> > Yes, I previously acked it, I believe.
> > 
> 
> 'PKG_CONFIG_LIBDIR' seems replaces the pkgconfig defaults,
> 'PKG_CONFIG_PATH' adds paths to process *before* built-in ones.
> 
> With adjustment according above, all looks good to me.
> 
> What about unset the variable "uset PKG_CONFIG_PATH", to set it to empty to get 
> rid of the following line from logs:
> "Using 'PKG_CONFIG_PATH' from environment with value: ''"

Oh yes I didn't notice this message.
The reason why I didn't unset was to allow referencing empty variable
with shell option which forbids referencing unset ones: set -u.

Now I think it's better to unset.

> With or without about change,
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

[dpdk-stable] [PATCH v2 1/1] devtools: fix build test config inheritance from env

[Thomas Monjalon]

The variables DPDK_MESON_OPTIONS, PATH, PKG_CONFIG_PATH,
CPPFLAGS, CFLAGS and LDFLAGS can be customized in the config file
loaded by devtools/load-devel-config at each build.
The configuration can be adjusted per target thanks to the value set
in the DPDK_TARGET variable.

PKG_CONFIG_PATH is specific to each target, so it must be empty
before configuring each build from the file according to DPDK_TARGET.
Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
and is prone to confusion.

DPDK_MESON_OPTIONS might take a global initial value from environment
to customize a build test from the shell. Example:
	DPDK_MESON_OPTIONS="b_lto=true"
Some target-specific options can be added in the configuration file:
	DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"

Fixes: 272236741258 ("devtools: load target-specific compilation environment")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
Tested-by: Jerin Jacob <jerinj@marvell.com>
---
v2:
- unset PKG_CONFIG_PATH because empty is printed by meson
- add more comments
---
 devtools/test-meson-builds.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 0c95d1cc98..f32b5784f4 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -38,10 +38,10 @@ else
 fi
 
 default_path=$PATH
-default_pkgpath=$PKG_CONFIG_PATH
 default_cppflags=$CPPFLAGS
 default_cflags=$CFLAGS
 default_ldflags=$LDFLAGS
+default_meson_options=$DPDK_MESON_OPTIONS
 
 check_cc_flags () # <flag to check> <flag2> ...
 {
@@ -52,12 +52,14 @@ check_cc_flags () # <flag to check> <flag2> ...
 load_env () # <target compiler>
 {
 	targetcc=$1
+	# reset variables before target-specific config
 	export PATH=$default_path
-	export PKG_CONFIG_PATH=$default_pkgpath
+	unset PKG_CONFIG_PATH # global default makes no sense
 	export CPPFLAGS=$default_cppflags
 	export CFLAGS=$default_cflags
 	export LDFLAGS=$default_ldflags
-	unset DPDK_MESON_OPTIONS
+	export DPDK_MESON_OPTIONS=$default_meson_options
+	# set target hint for use in the loaded config file
 	if [ -n "$target_override" ] ; then
 		DPDK_TARGET=$target_override
 	elif command -v $targetcc >/dev/null 2>&1 ; then
-- 
2.28.0

Re: [dpdk-stable] [PATCH v2 1/1] devtools: fix build test config inheritance from env

[David Marchand]

On Thu, Nov 12, 2020 at 3:22 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> The variables DPDK_MESON_OPTIONS, PATH, PKG_CONFIG_PATH,
> CPPFLAGS, CFLAGS and LDFLAGS can be customized in the config file
> loaded by devtools/load-devel-config at each build.
> The configuration can be adjusted per target thanks to the value set
> in the DPDK_TARGET variable.
>
> PKG_CONFIG_PATH is specific to each target, so it must be empty
> before configuring each build from the file according to DPDK_TARGET.
> Inheriting a default PKG_CONFIG_PATH for all targets does not make sense
> and is prone to confusion.
>
> DPDK_MESON_OPTIONS might take a global initial value from environment
> to customize a build test from the shell. Example:
>         DPDK_MESON_OPTIONS="b_lto=true"
> Some target-specific options can be added in the configuration file:
>         DPDK_MESON_OPTIONS="$DPDK_MESON_OPTIONS kernel_dir=$MYKERNEL"
>
> Fixes: 272236741258 ("devtools: load target-specific compilation environment")
> Cc: stable@dpdk.org
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Tested-by: Jerin Jacob <jerinj@marvell.com>

Acked-by: David Marchand <david.marchand@redhat.com>


-- 
David Marchand