diff mbox series

configs/kontron_bl_imx8mm_defconfig: fix build failure with u-boot 2022.04

Message ID 20220807210317.9415-1-heiko.thiery@gmail.com
State Changes Requested
Headers show
Series configs/kontron_bl_imx8mm_defconfig: fix build failure with u-boot 2022.04 | expand

Commit Message

Heiko Thiery Aug. 7, 2022, 9:03 p.m. UTC 
With U-Boot 2022.04 libuuid is required for building the host tool
mkeficapsule. The lib is included in the util-linux package. Thus the
BR2_TARGET_UBOOT_NEEDS_UTIL_LINUX config is needed.

In addition an U-boot patch is required to fix an issue in U-Boot for
linking the mkeficapsule tool against -luuid and -lgnutls.

Fixes: https://gitlab.com/buildroot.org/buildroot/-/jobs/2812053608

Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 ...le-use-pkg-config-to-get-luuid-and-l.patch | 33 +++++++++++++++++++
 configs/kontron_bl_imx8mm_defconfig           |  2 ++
 2 files changed, 35 insertions(+)
 create mode 100644 board/kontron/bl-imx8mm/patches/uboot/2022.04/0001-tools-mkeficapsule-use-pkg-config-to-get-luuid-and-l.patch

Comments

Thomas Petazzoni Aug. 8, 2022, 8:41 a.m. UTC | #1 
Hello Heiko,

Thanks for the patch!

On Sun,  7 Aug 2022 23:03:18 +0200
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> +-HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
> ++HOSTLDLIBS_mkeficapsule += \
> ++	$(shell pkg-config --libs gnutls uuid 2> /dev/null || echo "-lgnutls -luuid")

Do you understand why this is needed?

In the build failure reported by Gitlab CI, it fails to find libgnutls,
even though host-gnutls has been built before. So the -lgnutls that was
in HOSTLDLIBS_mkeficapsule should have been sufficient, provided
$(HOST_LDFLAGS) are properly used.

Of course, I have nothing against using pkg-config, but I was just
wondering if we have an explanation on what was going on. And is
pkg-config fixing it because it's return a full path to the library?

Obviously, one issue with your patch is that it's going to fix the
problem only for the kontron defconfig, which is why I was interested
to see if there wasn't a better solution.

Thanks!

Thomas
Heiko Thiery Aug. 9, 2022, 6:45 a.m. UTC | #2 
Hi Thomas,

Am Mo., 8. Aug. 2022 um 10:41 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> Hello Heiko,
>
> Thanks for the patch!
>
> On Sun,  7 Aug 2022 23:03:18 +0200
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > +-HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
> > ++HOSTLDLIBS_mkeficapsule += \
> > ++    $(shell pkg-config --libs gnutls uuid 2> /dev/null || echo "-lgnutls -luuid")
>
> Do you understand why this is needed?
>
> In the build failure reported by Gitlab CI, it fails to find libgnutls,
> even though host-gnutls has been built before. So the -lgnutls that was
> in HOSTLDLIBS_mkeficapsule should have been sufficient, provided
> $(HOST_LDFLAGS) are properly used.

Without the patch:
--- 8< ---
  /usr/bin/gcc -O2 -isystem
/home/hthiery/sources/mainline/buildroot/output/host/include
-Wp,-MD,tools/.mkeficapsule.d -Wall -Wstrict-prototypes -O2
-fomit-frame-pointer   -std=gnu11   -DCONFIG_FIT_SIGNATURE
-DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff -DCONFIG_FIT_CIPHER
-include ./include/compiler.h -idirafterinclude
-idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools
-DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE  -o
tools/mkeficapsule tools/mkeficapsule.c   -lgnutls -luuid
--- 8< ---

With the patch:
--- 8< ---
  /usr/bin/gcc -O2 -isystem
/home/hthiery/sources/mainline/buildroot/output/host/include
-Wp,-MD,tools/.mkeficapsule.d -Wall -Wstrict-prototypes -O2
-fomit-frame-pointer   -std=gnu11   -DCONFIG_FIT_SIGNATURE
-DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff -DCONFIG_FIT_CIPHER
-include ./include/compiler.h -idirafterinclude
-idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools
-DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE  -o
tools/mkeficapsule tools/mkeficapsule.c
-L/home/hthiery/sources/mainline/buildroot/output/host/lib -lgnutls
-luuid
--- 8< ---

The -lgnuts and -luuid seems not to be sufficient. Using pkg-config
will also add the -L<PATH-TO-LIBS>.


>
> Of course, I have nothing against using pkg-config, but I was just
> wondering if we have an explanation on what was going on. And is
> pkg-config fixing it because it's return a full path to the library?
>
> Obviously, one issue with your patch is that it's going to fix the
> problem only for the kontron defconfig, which is why I was interested
> to see if there wasn't a better solution.
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
Thomas Petazzoni Aug. 9, 2022, 8:06 a.m. UTC | #3 
Hello Heiko,

On Tue, 9 Aug 2022 08:45:09 +0200
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Without the patch:
> --- 8< ---
>   /usr/bin/gcc -O2 -isystem
> /home/hthiery/sources/mainline/buildroot/output/host/include
> -Wp,-MD,tools/.mkeficapsule.d -Wall -Wstrict-prototypes -O2
> -fomit-frame-pointer   -std=gnu11   -DCONFIG_FIT_SIGNATURE
> -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff -DCONFIG_FIT_CIPHER
> -include ./include/compiler.h -idirafterinclude
> -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools
> -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE  -o
> tools/mkeficapsule tools/mkeficapsule.c   -lgnutls -luuid
> --- 8< ---
> 
> With the patch:
> --- 8< ---
>   /usr/bin/gcc -O2 -isystem
> /home/hthiery/sources/mainline/buildroot/output/host/include
> -Wp,-MD,tools/.mkeficapsule.d -Wall -Wstrict-prototypes -O2
> -fomit-frame-pointer   -std=gnu11   -DCONFIG_FIT_SIGNATURE
> -DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff -DCONFIG_FIT_CIPHER
> -include ./include/compiler.h -idirafterinclude
> -idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools
> -DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE  -o
> tools/mkeficapsule tools/mkeficapsule.c
> -L/home/hthiery/sources/mainline/buildroot/output/host/lib -lgnutls
> -luuid
> --- 8< ---
> 
> The -lgnuts and -luuid seems not to be sufficient. Using pkg-config
> will also add the -L<PATH-TO-LIBS>.

But this is not normal, because:

UBOOT_MAKE_OPTS += \
        CROSS_COMPILE="$(TARGET_CROSS)" \
        ARCH=$(UBOOT_ARCH) \
        HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \
        HOSTLDFLAGS="$(HOST_LDFLAGS)" \
        $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS))

see we're passing HOSTLDFLAGS here? It contains -L$(HOST_DIR)/lib. So
it means there is also something wrong in U-Boot in that it doesn't use
$(HOSTLDFLAGS).

Best regards,

Thomas
Thomas Petazzoni Aug. 15, 2022, 10:14 a.m. UTC | #4 
Hello Heiko,

On Tue, 9 Aug 2022 10:06:02 +0200
Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:

> But this is not normal, because:
> 
> UBOOT_MAKE_OPTS += \
>         CROSS_COMPILE="$(TARGET_CROSS)" \
>         ARCH=$(UBOOT_ARCH) \
>         HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \
>         HOSTLDFLAGS="$(HOST_LDFLAGS)" \
>         $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS))
> 
> see we're passing HOSTLDFLAGS here? It contains -L$(HOST_DIR)/lib. So
> it means there is also something wrong in U-Boot in that it doesn't use
> $(HOSTLDFLAGS).

Could you try this change in U-Boot:

diff --git a/scripts/Makefile.host b/scripts/Makefile.host
index 69983a19a44..7624304e3e9 100644
--- a/scripts/Makefile.host
+++ b/scripts/Makefile.host
@@ -89,7 +89,7 @@ hostcxx_flags  = -Wp,-MD,$(depfile) $(__hostcxx_flags)
 # Create executable from a single .c file
 # host-csingle -> Executable
 quiet_cmd_host-csingle         = HOSTCC  $@
-      cmd_host-csingle = $(HOSTCC) $(hostc_flags) -o $@ $< \
+      cmd_host-csingle = $(HOSTCC) $(hostc_flags) $(KBUILD_HOSTLDFLAGS) -o $@ $< \
                $(KBUILD_HOSTLDLIBS) $(HOSTLDLIBS_$(@F))
 $(host-csingle): $(obj)/%: $(src)/%.c FORCE
        $(call if_changed_dep,host-csingle)

(without your pkg-config patch, of course).

Indeed, scripts/Makefile.host is a copy of the kernel's
scripts/Makefile.host, and the kernel copy does use KBUILD_HOSTLDFLAGS
when building host tools that are based on a single source file, but
for some reason the U-Boot copy does not have it, which doesn't make
much sense.

I think it doesn't mean your pkg-config patch is bogus: it still makes
sense to use pkg-config when possible, but I also think not passing
KBUILD_HOSTLDFLAGS is incorrect.

Of course, if it works, I think both patches should be submitted to
U-Boot upstream.

Could you have a look?

Thanks a lot!

Thomas
Heiko Thiery Aug. 23, 2022, 5:30 p.m. UTC | #5 
Hi Thomas,

Am Mo., 15. Aug. 2022 um 12:14 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> Hello Heiko,
>
> On Tue, 9 Aug 2022 10:06:02 +0200
> Thomas Petazzoni via buildroot <buildroot@buildroot.org> wrote:
>
> > But this is not normal, because:
> >
> > UBOOT_MAKE_OPTS += \
> >         CROSS_COMPILE="$(TARGET_CROSS)" \
> >         ARCH=$(UBOOT_ARCH) \
> >         HOSTCC="$(HOSTCC) $(subst -I/,-isystem /,$(subst -I /,-isystem /,$(HOST_CFLAGS)))" \
> >         HOSTLDFLAGS="$(HOST_LDFLAGS)" \
> >         $(call qstrip,$(BR2_TARGET_UBOOT_CUSTOM_MAKEOPTS))
> >
> > see we're passing HOSTLDFLAGS here? It contains -L$(HOST_DIR)/lib. So
> > it means there is also something wrong in U-Boot in that it doesn't use
> > $(HOSTLDFLAGS).
>
> Could you try this change in U-Boot:
>
> diff --git a/scripts/Makefile.host b/scripts/Makefile.host
> index 69983a19a44..7624304e3e9 100644
> --- a/scripts/Makefile.host
> +++ b/scripts/Makefile.host
> @@ -89,7 +89,7 @@ hostcxx_flags  = -Wp,-MD,$(depfile) $(__hostcxx_flags)
>  # Create executable from a single .c file
>  # host-csingle -> Executable
>  quiet_cmd_host-csingle         = HOSTCC  $@
> -      cmd_host-csingle = $(HOSTCC) $(hostc_flags) -o $@ $< \
> +      cmd_host-csingle = $(HOSTCC) $(hostc_flags) $(KBUILD_HOSTLDFLAGS) -o $@ $< \
>                 $(KBUILD_HOSTLDLIBS) $(HOSTLDLIBS_$(@F))
>  $(host-csingle): $(obj)/%: $(src)/%.c FORCE
>         $(call if_changed_dep,host-csingle)
>
> (without your pkg-config patch, of course).
>
> Indeed, scripts/Makefile.host is a copy of the kernel's
> scripts/Makefile.host, and the kernel copy does use KBUILD_HOSTLDFLAGS
> when building host tools that are based on a single source file, but
> for some reason the U-Boot copy does not have it, which doesn't make
> much sense.

Indeed ... I just added this change to my local copy of u-boot rebuild
it without the previous patch and it works.

>
> I think it doesn't mean your pkg-config patch is bogus: it still makes
> sense to use pkg-config when possible, but I also think not passing
> KBUILD_HOSTLDFLAGS is incorrect.
>
> Of course, if it works, I think both patches should be submitted to
> U-Boot upstream.

I will prepare an U-Boot patch and send it to the mailing list. I saw
that you cahnged the patch in patchwork to "changes requested". Would
you accept it with the "new" patch?

> Could you have a look?

without the change:
--- 8< ---
  /usr/bin/gcc -O2 -isystem
/home/hthiery/sources/mainline/buildroot/output/host/include
-Wp,-MD,tools/.mkeficapsule.d -Wall -Wstrict-prototypes -O2
-fomit-frame-pointer   -std=gnu11    -DCONFIG_FIT_SIGNATURE
-DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff -DCONFIG_FIT_CIPHER
-include ./include/compiler.h -idirafterinclude
-idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools
-DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE  -o
tools/mkeficapsule tools/mkeficapsule.c   -lgnutls -luuid
--- 8< ---

with the change you suggested:
--- 8< ---
  /usr/bin/gcc -O2 -isystem
/home/hthiery/sources/mainline/buildroot/output/host/include
-Wp,-MD,tools/.mkeficapsule.d -Wall -Wstrict-prototypes -O2
-fomit-frame-pointer   -std=gnu11    -DCONFIG_FIT_SIGNATURE
-DCONFIG_FIT_SIGNATURE_MAX_SIZE=0xffffffff -DCONFIG_FIT_CIPHER
-include ./include/compiler.h -idirafterinclude
-idirafter./arch/arm/include -I./scripts/dtc/libfdt -I./tools
-DUSE_HOSTCC -D__KERNEL_STRICT_NAMES -D_GNU_SOURCE
-L/home/hthiery/sources/mainline/buildroot/output/host/lib
-Wl,-rpath,/home/hthiery/sources/mainline/buildroot/output/host/lib -o
tools/mkeficapsule tools/mkeficapsule.c   -lgnutls -luuid
--- 8< ---

> Thanks a lot!
>
> Thomas
> --
> Thomas Petazzoni, co-owner and CEO, Bootlin
> Embedded Linux and Kernel engineering and training
> https://bootlin.com
diff mbox series

Patch

diff --git a/board/kontron/bl-imx8mm/patches/uboot/2022.04/0001-tools-mkeficapsule-use-pkg-config-to-get-luuid-and-l.patch b/board/kontron/bl-imx8mm/patches/uboot/2022.04/0001-tools-mkeficapsule-use-pkg-config-to-get-luuid-and-l.patch
new file mode 100644
index 0000000000..9530a45efe
--- /dev/null
+++ b/board/kontron/bl-imx8mm/patches/uboot/2022.04/0001-tools-mkeficapsule-use-pkg-config-to-get-luuid-and-l.patch
@@ -0,0 +1,33 @@ 
+From f3523977e8f5f6b2173708777001332431ebc609 Mon Sep 17 00:00:00 2001
+From: Heiko Thiery <heiko.thiery@gmail.com>
+Date: Tue, 19 Jul 2022 16:17:09 +0200
+Subject: [PATCH 1/2] tools: mkeficapsule: use pkg-config to get -luuid and -lgnutls
+
+Instead of hardcoding -luuid -lgnutls as the flags needed to build
+mkeficapsule, use pkg-config when available.
+
+We gracefully fallback on the previous behavior of hardcoding -luuid
+-lgnutls if pkg-config is not available or fails with an error.
+
+Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
+---
+ tools/Makefile | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/tools/Makefile b/tools/Makefile
+index 9f2339666a..9f6b282ad8 100644
+--- a/tools/Makefile
++++ b/tools/Makefile
+@@ -242,7 +242,8 @@ hostprogs-$(CONFIG_MIPS) += mips-relocs
+ hostprogs-$(CONFIG_ASN1_COMPILER)	+= asn1_compiler
+ HOSTCFLAGS_asn1_compiler.o = -idirafter $(srctree)/include
+ 
+-HOSTLDLIBS_mkeficapsule += -lgnutls -luuid
++HOSTLDLIBS_mkeficapsule += \
++	$(shell pkg-config --libs gnutls uuid 2> /dev/null || echo "-lgnutls -luuid")
+ hostprogs-$(CONFIG_TOOLS_MKEFICAPSULE) += mkeficapsule
+ 
+ # We build some files with extra pedantic flags to try to minimize things
+-- 
+2.30.2
+
diff --git a/configs/kontron_bl_imx8mm_defconfig b/configs/kontron_bl_imx8mm_defconfig
index ff376662e9..c747ce02d7 100644
--- a/configs/kontron_bl_imx8mm_defconfig
+++ b/configs/kontron_bl_imx8mm_defconfig
@@ -5,6 +5,7 @@  BR2_ARM_FPU_VFPV3=y
 
 # System
 BR2_TARGET_GENERIC_GETTY_PORT="ttymxc2"
+BR2_GLOBAL_PATCH_DIR="board/kontron/bl-imx8mm/patches"
 
 # Kernel
 BR2_LINUX_KERNEL=y
@@ -48,6 +49,7 @@  BR2_TARGET_UBOOT_NEEDS_GNUTLS=y
 BR2_TARGET_UBOOT_NEEDS_ATF_BL31=y
 BR2_TARGET_UBOOT_NEEDS_ATF_BL31_BIN=y
 BR2_TARGET_UBOOT_NEEDS_IMX_FIRMWARE=y
+BR2_TARGET_UBOOT_NEEDS_UTIL_LINUX=y
 BR2_TARGET_UBOOT_FORMAT_CUSTOM=y
 BR2_TARGET_UBOOT_FORMAT_CUSTOM_NAME="flash.bin"
 BR2_TARGET_UBOOT_SPL=y