Re: [Buildroot] [PATCH 1/8] boot/grub2: add support to build multiple Grub2 configurations in the same build

[Thomas Petazzoni <thomas.petazzoni@bootlin.com>]

Hello Yann,

On Fri, 17 Sep 2021 23:24:29 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> What you forgot to state in bold fat characters here, is that we can no
> longer use autotools-package as a consequence of this multi-build, and
> that we have to resort to generic-pacakge and a partial duplication of
> the autotools-infra.
> 
> This is not very nice... :-(

Indeed, but...

> IIRC, we had a similar problem with barbox, where we had to build two
> images for it, so we ended up duplicating the package, where the
> top-most boot/barebox/barebox.mk defines a minimalist barebox-package
> infra, and the barebox and barebox-aux are two barebox-package:
> 
>     boot/barebox/
>     ├── barebox/
>     │   ├── barebox.hash -> ../barebox.hash
>     │   ├── barebox.mk
>     │   └── Config.in
>     ├── barebox-aux/
>     │   ├── barebox-aux.hash -> ../barebox.hash
>     │   ├── barebox-aux.mk
>     │   └── Config.in
>     ├── barebox.hash
>     ├── barebox.mk
>     └── Config.in
> 
> If I understand correctly, we are in a similar situation with grub2, and
> there are only ever at most two "platforms" we can build, and it does
> not look like it will ever change:

Nope, that is not correct.

>     i386:
>         BR2_TARGET_GRUB2_I386_PC
>         BR2_TARGET_GRUB2_I386_EFI
> 
>     x86_64:
>         BR2_TARGET_GRUB2_I386_EFI
>         BR2_TARGET_GRUB2_X86_64_EFI

On x86-64, we can have:

	BR2_TARGET_GRUB2_I386_PC
	BR2_TARGET_GRUB2_I386_EFI
	BR2_TARGET_GRUB2_X86_64_EFI

to cover legacy BIOS platforms (32-bit), 32-bit EFI BIOS and 64-bit EFI
BIOS. Dans this is precisely the use-case we are in fact interested in.

So that means we would need 3 Grub packages, which is the reason why it
felt much more reasonable to just extend the grub2 package itself.

> >  GRUB2_CONF_OPTS = \
> > -	--target=$(GRUB2_TARGET) \
> > -	--with-platform=$(GRUB2_PLATFORM) \
> > +	--host=$(GNU_TARGET_NAME) \
> > +	--build=$(GNU_HOST_NAME) \
> > +	--target=$(GRUB2_$(call UPPERCASE,$(tuple))_TARGET) \
> > +	--with-platform=$(GRUB2_$(call UPPERCASE,$(tuple))_PLATFORM) \  
> 
>     --target=$(GRUB2_TARGET_$(tuple))
>     --with-platform=$(GRUB2_PLATFORM_$(tuple)) \
> 
> However, I don't like macros that are implicitly parameterised. We've
> gone in quite some changes in the past to fix similar cases (e.g. in the
> download infra).

Ah, yes, I missed that during the review, indeed the fact that we
reference $(tuple) in GRUB2_CONF_OPTS without it being an argument is a
bit annoying.

> So, I'd write a real parameterised marco:
> 
>     # $(1): tuple blablabla...
>     GRUB2_CONF_OPTS = \
>         --host=$(GNU_TARGET_NAME) \
>         --target=$(GRUB2_TARGET_$(1)) \
>         --with-platform=$(GRUB2_PLATFORM_$(1)) \
>         [...]
> 
>     $(foreach tuple, $(GRUB2_TUPLES_y), \
>         [...] \
>         ../configure $(call GRUB2_CONF_OPTS,$(tuple))

Or, just do:

	  ../configure $(GRUB2_CONF_OPTS) \
		--target=... \
		--with-platform=...

I.e, keep out of GRUB2_CONF_OPTS the options that are parameterized.


> > -$(eval $(autotools-package))
> > +$(eval $(generic-package))  
> 
> This is my main problem: we're mostly duplicating bits and pieces of the
> autotolls-package infra, which means we *will* fall behind very quickly
> when we have to update said infra, and those changes will not percolate
> to this grub2 package.

That is true, but on the other hand, grub2 is not the typical
user-space autotools package either.

See the stack of crap that we have to pass to make it work:

GRUB2_CONF_ENV = \
        CPP="$(TARGET_CC) -E" \
        TARGET_CC="$(TARGET_CC)" \
        CFLAGS="$(TARGET_CFLAGS) -Os" \
        TARGET_CFLAGS="$(TARGET_CFLAGS) -Os" \
        CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
        TARGET_CPPFLAGS="$(TARGET_CPPFLAGS) -Os -fno-stack-protector" \
        TARGET_LDFLAGS="$(TARGET_LDFLAGS) -Os" \
        TARGET_NM="$(TARGET_NM)" \
        TARGET_OBJCOPY="$(TARGET_OBJCOPY)" \
        TARGET_STRIP="$(TARGET_CROSS)strip"

GRUB2_CONF_OPTS = \
        --target=$(GRUB2_TARGET) \
        --with-platform=$(GRUB2_PLATFORM) \
        --prefix=/ \
        --exec-prefix=/ \
        --disable-grub-mkfont \
        --enable-efiemu=no \
        ac_cv_lib_lzma_lzma_code=no \
        --enable-device-mapper=no \
        --enable-libzfs=no \
        --disable-werror

Including the passing of --target, custom --prefix, custom
--exec-prefix.

So even though we do use autotools-package, it's not like we don't
already have to play quite a few tricks to get it to do the right thing
with the weirdness of grub2.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@lists.buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot