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