diff mbox series

[1/1] linux: use host pkg-config when host libelf is set

Message ID 20190408184233.10697-1-stuart.summers@intel.com
State Accepted
Headers show
Series [1/1] linux: use host pkg-config when host libelf is set | expand

Commit Message

Summers, Stuart April 8, 2019, 6:42 p.m. UTC
A patch was added to the Linux kernel in 5.1.0-rc3 which
adds a requirement that the host build environment include
pkg-config. Add the correct host-pkgconf dependency and
environment variables to ensure Linux picks up the correct
libraries.

v2: Use LINUX_MAKE_ENV to pass PKG_CONFIG_* options (Thomas)
v3: Add PKG_CONFIG_ALLOW_SYSTEM_{CFLAGS,LIBS} to kconfig
    blacklist and move LINUX_MAKE_ENV initial assign in
    linux.mk before DEP* assignments (Yann)

Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Yann E. MORIN <yann.morin.1998@free.fr>
Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Signed-off-by: Stuart Summers <stuart.summers@intel.com>
---
 linux/linux.mk         | 16 +++++++++++-----
 package/pkg-kconfig.mk |  4 +++-
 2 files changed, 14 insertions(+), 6 deletions(-)

Comments

Yann E. MORIN April 8, 2019, 7:28 p.m. UTC | #1
Stuart, All,

On 2019-04-08 11:42 -0700, Stuart Summers spake thusly:
> A patch was added to the Linux kernel in 5.1.0-rc3 which
> adds a requirement that the host build environment include
> pkg-config. Add the correct host-pkgconf dependency and
> environment variables to ensure Linux picks up the correct
> libraries.

Additionally:

    Move the existing LINUX_MAKE_ENV assignment earlier, to simplify
    the append-assignment in the libelf conditional block.

And since you posted your patch, someone opened a bug in our tracker,
so:

    Fixes: #11761

> v2: Use LINUX_MAKE_ENV to pass PKG_CONFIG_* options (Thomas)
> v3: Add PKG_CONFIG_ALLOW_SYSTEM_{CFLAGS,LIBS} to kconfig
>     blacklist and move LINUX_MAKE_ENV initial assign in
>     linux.mk before DEP* assignments (Yann)

This v2-v3 changelog should be after the --- line, below...

> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> ---

... here.

>  linux/linux.mk         | 16 +++++++++++-----
>  package/pkg-kconfig.mk |  4 +++-
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk
> index 6bf2b88038..d209a5cf59 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -61,6 +61,10 @@ LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
>  # be directories in the patch list (unlike for other packages).
>  LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES))
>  
> +LINUX_MAKE_ENV = \
> +	$(TARGET_MAKE_ENV) \
> +	BR_BINARIES_DIR=$(BINARIES_DIR)
> +
>  LINUX_INSTALL_IMAGES = YES
>  LINUX_DEPENDENCIES = host-kmod
>  
> @@ -97,7 +101,13 @@ LINUX_DEPENDENCIES += host-openssl
>  endif
>  
>  ifeq ($(BR2_LINUX_KERNEL_NEEDS_HOST_LIBELF),y)
> -LINUX_DEPENDENCIES += host-elfutils
> +LINUX_DEPENDENCIES += host-elfutils host-pkgconf
> +LINUX_MAKE_ENV += \
> +	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
> +	PKG_CONFIG_SYSROOT_DIR="/" \
> +	PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> +	PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 \
> +	PKG_CONFIG_LIBDIR="$(HOST_DIR)/lib/pkgconfig:$(HOST_DIR)/share/pkgconfig"
>  endif
>  
>  # If host-uboot-tools is selected by the user, assume it is needed to
> @@ -121,10 +131,6 @@ LINUX_MAKE_FLAGS = \
>  	CROSS_COMPILE="$(TARGET_CROSS)" \
>  	DEPMOD=$(HOST_DIR)/sbin/depmod
>  
> -LINUX_MAKE_ENV = \
> -	$(TARGET_MAKE_ENV) \
> -	BR_BINARIES_DIR=$(BINARIES_DIR)
> -
>  ifeq ($(BR2_REPRODUCIBLE),y)
>  LINUX_MAKE_ENV += \
>  	KBUILD_BUILD_VERSION=1 \
> diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> index d6c95b897e..e53cc9002e 100644
> --- a/package/pkg-kconfig.mk
> +++ b/package/pkg-kconfig.mk
> @@ -175,7 +175,9 @@ endif
>  # nconfig, gconfig, xconfig).
>  # So we simply remove our PATH and PKG_CONFIG_* variables.
>  $(2)_CONFIGURATOR_MAKE_ENV = \
> -	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=% PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
> +	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=% \
> +	   PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=% PKG_CONFIG_ALLOW_SYSTEM_LIBS=% \
> +	   PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \

I'm not too happy about the look the above has, but I have no better
formatting suggestion to provide...

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

>  	PKG_CONFIG_PATH="$(HOST_PKG_CONFIG_PATH)"
>  
>  # Configuration editors (menuconfig, ...)
> -- 
> 2.20.1
>
Summers, Stuart April 8, 2019, 7:41 p.m. UTC | #2
On Mon, 2019-04-08 at 21:28 +0200, Yann E. MORIN wrote:
> Stuart, All,
> 
> On 2019-04-08 11:42 -0700, Stuart Summers spake thusly:
> > A patch was added to the Linux kernel in 5.1.0-rc3 which
> > adds a requirement that the host build environment include
> > pkg-config. Add the correct host-pkgconf dependency and
> > environment variables to ensure Linux picks up the correct
> > libraries.
> 
> Additionally:
> 
>     Move the existing LINUX_MAKE_ENV assignment earlier, to simplify
>     the append-assignment in the libelf conditional block.

Makes sense.

> 
> And since you posted your patch, someone opened a bug in our tracker,
> so:
> 
>     Fixes: #11761

Ah, didn't see this bug until after I sent out the patch. Thanks for
the update there!

> 
> > v2: Use LINUX_MAKE_ENV to pass PKG_CONFIG_* options (Thomas)
> > v3: Add PKG_CONFIG_ALLOW_SYSTEM_{CFLAGS,LIBS} to kconfig
> >     blacklist and move LINUX_MAKE_ENV initial assign in
> >     linux.mk before DEP* assignments (Yann)
> 
> This v2-v3 changelog should be after the --- line, below...

In the future, I'll format this way, thanks for the pointer.

> 
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> > Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> > ---
> 
> ... here.
> 
> >  linux/linux.mk         | 16 +++++++++++-----
> >  package/pkg-kconfig.mk |  4 +++-
> >  2 files changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/linux/linux.mk b/linux/linux.mk
> > index 6bf2b88038..d209a5cf59 100644
> > --- a/linux/linux.mk
> > +++ b/linux/linux.mk
> > @@ -61,6 +61,10 @@ LINUX_PATCHES = $(call
> > qstrip,$(BR2_LINUX_KERNEL_PATCH))
> >  # be directories in the patch list (unlike for other packages).
> >  LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES)
> > )
> >  
> > +LINUX_MAKE_ENV = \
> > +	$(TARGET_MAKE_ENV) \
> > +	BR_BINARIES_DIR=$(BINARIES_DIR)
> > +
> >  LINUX_INSTALL_IMAGES = YES
> >  LINUX_DEPENDENCIES = host-kmod
> >  
> > @@ -97,7 +101,13 @@ LINUX_DEPENDENCIES += host-openssl
> >  endif
> >  
> >  ifeq ($(BR2_LINUX_KERNEL_NEEDS_HOST_LIBELF),y)
> > -LINUX_DEPENDENCIES += host-elfutils
> > +LINUX_DEPENDENCIES += host-elfutils host-pkgconf
> > +LINUX_MAKE_ENV += \
> > +	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
> > +	PKG_CONFIG_SYSROOT_DIR="/" \
> > +	PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
> > +	PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 \
> > +	PKG_CONFIG_LIBDIR="$(HOST_DIR)/lib/pkgconfig:$(HOST_DIR)/share/
> > pkgconfig"
> >  endif
> >  
> >  # If host-uboot-tools is selected by the user, assume it is needed
> > to
> > @@ -121,10 +131,6 @@ LINUX_MAKE_FLAGS = \
> >  	CROSS_COMPILE="$(TARGET_CROSS)" \
> >  	DEPMOD=$(HOST_DIR)/sbin/depmod
> >  
> > -LINUX_MAKE_ENV = \
> > -	$(TARGET_MAKE_ENV) \
> > -	BR_BINARIES_DIR=$(BINARIES_DIR)
> > -
> >  ifeq ($(BR2_REPRODUCIBLE),y)
> >  LINUX_MAKE_ENV += \
> >  	KBUILD_BUILD_VERSION=1 \
> > diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
> > index d6c95b897e..e53cc9002e 100644
> > --- a/package/pkg-kconfig.mk
> > +++ b/package/pkg-kconfig.mk
> > @@ -175,7 +175,9 @@ endif
> >  # nconfig, gconfig, xconfig).
> >  # So we simply remove our PATH and PKG_CONFIG_* variables.
> >  $(2)_CONFIGURATOR_MAKE_ENV = \
> > -	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=%
> > PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
> > +	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=% \
> > +	   PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=%
> > PKG_CONFIG_ALLOW_SYSTEM_LIBS=% \
> > +	   PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
> 
> I'm not too happy abfor the poout the look the above has, but I have
> no better
> formatting suggestion to provide...

Yeah, I agree it looks ugly.. If you have suggestions, I'm happy to
take a look.

> 
> Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Thanks for the review and the time here!

Given this is my first patch, I'm not sure I have push rights. Can you
help with merging? Also, would you like a re-post with the commit
message recommendations above?

Thanks,
Stuart

> 
> Regards,
> Yann E. MORIN.
> 
> >  	PKG_CONFIG_PATH="$(HOST_PKG_CONFIG_PATH)"
> >  
> >  # Configuration editors (menuconfig, ...)
> > -- 
> > 2.20.1
> > 
> 
>
Yann E. MORIN April 8, 2019, 8:01 p.m. UTC | #3
Stuart, All,

On 2019-04-08 19:41 +0000, Summers, Stuart spake thusly:
> On Mon, 2019-04-08 at 21:28 +0200, Yann E. MORIN wrote:
[--SNIP--]
> > >  $(2)_CONFIGURATOR_MAKE_ENV = \
> > > -	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=%
> > > PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
> > > +	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=% \
> > > +	   PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=%
> > > PKG_CONFIG_ALLOW_SYSTEM_LIBS=% \
> > > +	   PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
> > 
> > I'm not too happy abfor the poout the look the above has, but I have
> > no better
> > formatting suggestion to provide...
> Yeah, I agree it looks ugly.. If you have suggestions, I'm happy to
> take a look.

No, I ahve no better suggestion, really, hence I gave my:

> > Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Thanks for the review and the time here!
> 
> Given this is my first patch, I'm not sure I have push rights. Can you
> help with merging? Also, would you like a re-post with the commit
> message recommendations above?

When one of the three maintainer have time, they'll pick up that patch
and apply; it's not going to be forgotten: all patches are tracked on
patchwork:
    https://patchwork.ozlabs.org/project/buildroot/list/

The committer can probably apply the tweaks I suggested when they apply,
so no need to respin unless requested, now.

Thanks!

Regards,
Yann E. MORIN.
Arnout Vandecappelle April 13, 2019, 3:09 p.m. UTC | #4
On 08/04/2019 20:42, Stuart Summers wrote:
> A patch was added to the Linux kernel in 5.1.0-rc3 which
> adds a requirement that the host build environment include
> pkg-config. Add the correct host-pkgconf dependency and
> environment variables to ensure Linux picks up the correct
> libraries.
> 
> v2: Use LINUX_MAKE_ENV to pass PKG_CONFIG_* options (Thomas)
> v3: Add PKG_CONFIG_ALLOW_SYSTEM_{CFLAGS,LIBS} to kconfig
>     blacklist and move LINUX_MAKE_ENV initial assign in
>     linux.mk before DEP* assignments (Yann)
> 
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Signed-off-by: Stuart Summers <stuart.summers@intel.com>

 Applied to master with Yann's suggested changes to the commit message, thank.

 Regards,
 Arnout
Summers, Stuart April 15, 2019, 2:29 p.m. UTC | #5
On Sat, 2019-04-13 at 17:09 +0200, Arnout Vandecappelle wrote:
> 
> On 08/04/2019 20:42, Stuart Summers wrote:
> > A patch was added to the Linux kernel in 5.1.0-rc3 which
> > adds a requirement that the host build environment include
> > pkg-config. Add the correct host-pkgconf dependency and
> > environment variables to ensure Linux picks up the correct
> > libraries.
> > 
> > v2: Use LINUX_MAKE_ENV to pass PKG_CONFIG_* options (Thomas)
> > v3: Add PKG_CONFIG_ALLOW_SYSTEM_{CFLAGS,LIBS} to kconfig
> >     blacklist and move LINUX_MAKE_ENV initial assign in
> >     linux.mk before DEP* assignments (Yann)
> > 
> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Cc: Yann E. MORIN <yann.morin.1998@free.fr>
> > Suggested-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> > Signed-off-by: Stuart Summers <stuart.summers@intel.com>
> 
>  Applied to master with Yann's suggested changes to the commit
> message, thank.

Thanks!

-Stuart

> 
>  Regards,
>  Arnout
diff mbox series

Patch

diff --git a/linux/linux.mk b/linux/linux.mk
index 6bf2b88038..d209a5cf59 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -61,6 +61,10 @@  LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
 # be directories in the patch list (unlike for other packages).
 LINUX_PATCH = $(filter ftp://% http://% https://%,$(LINUX_PATCHES))
 
+LINUX_MAKE_ENV = \
+	$(TARGET_MAKE_ENV) \
+	BR_BINARIES_DIR=$(BINARIES_DIR)
+
 LINUX_INSTALL_IMAGES = YES
 LINUX_DEPENDENCIES = host-kmod
 
@@ -97,7 +101,13 @@  LINUX_DEPENDENCIES += host-openssl
 endif
 
 ifeq ($(BR2_LINUX_KERNEL_NEEDS_HOST_LIBELF),y)
-LINUX_DEPENDENCIES += host-elfutils
+LINUX_DEPENDENCIES += host-elfutils host-pkgconf
+LINUX_MAKE_ENV += \
+	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
+	PKG_CONFIG_SYSROOT_DIR="/" \
+	PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 \
+	PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 \
+	PKG_CONFIG_LIBDIR="$(HOST_DIR)/lib/pkgconfig:$(HOST_DIR)/share/pkgconfig"
 endif
 
 # If host-uboot-tools is selected by the user, assume it is needed to
@@ -121,10 +131,6 @@  LINUX_MAKE_FLAGS = \
 	CROSS_COMPILE="$(TARGET_CROSS)" \
 	DEPMOD=$(HOST_DIR)/sbin/depmod
 
-LINUX_MAKE_ENV = \
-	$(TARGET_MAKE_ENV) \
-	BR_BINARIES_DIR=$(BINARIES_DIR)
-
 ifeq ($(BR2_REPRODUCIBLE),y)
 LINUX_MAKE_ENV += \
 	KBUILD_BUILD_VERSION=1 \
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk
index d6c95b897e..e53cc9002e 100644
--- a/package/pkg-kconfig.mk
+++ b/package/pkg-kconfig.mk
@@ -175,7 +175,9 @@  endif
 # nconfig, gconfig, xconfig).
 # So we simply remove our PATH and PKG_CONFIG_* variables.
 $(2)_CONFIGURATOR_MAKE_ENV = \
-	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=% PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
+	$$(filter-out PATH=% PKG_CONFIG=% PKG_CONFIG_SYSROOT_DIR=% \
+	   PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=% PKG_CONFIG_ALLOW_SYSTEM_LIBS=% \
+	   PKG_CONFIG_LIBDIR=%,$$($(2)_MAKE_ENV)) \
 	PKG_CONFIG_PATH="$(HOST_PKG_CONFIG_PATH)"
 
 # Configuration editors (menuconfig, ...)