Re: [Buildroot] [PATCH v3 2/7] boot/grub2: add support to build multiple Grub2 configurations in the same build

From: "Yann E. MORIN" <yann.morin.1998@free.fr>
To: "Köry Maincent" <kory.maincent@bootlin.com>
Cc: Adam Duskett <aduskett@gmail.com>,
	thomas.petazzoni@bootlin.com, buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH v3 2/7] boot/grub2: add support to build multiple Grub2 configurations in the same build
Date: Thu, 14 Oct 2021 22:02:07 +0200	[thread overview]
Message-ID: <20211014200207.GC368016@scaer> (raw)
In-Reply-To: <20211011122736.3b0df127@kmaincent-XPS-13-7390>

Köry, All,

Sorry for my late reply, I've been pretty busy here...

On 2021-10-11 12:27 +0200, Köry Maincent spake thusly:
> On Thu, 7 Oct 2021 18:29:31 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> > On 2021-10-07 14:43 +0200, Köry Maincent spake thusly:
> > > On Thu, 7 Oct 2021 11:53:26 +0200
> > > "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:  
> > [--SNIP--]
> 
> > In this case, this should not be too complex either; we just need a
> > post-image hook that copies all modules into TARGET_DIR, like:
> 
> Is something like the following code seems okay for you.
> I use temporary install-$(tuple) folder for the different platforms.
> We need to do the CONFIGURE_CMD_PTF as a PRE_CONFIGURE_HOOK because the
> CONFIGURE_CMDS from autotools is configuring inside the GRUB2_SRCDIR folder.
> Then if the ./configure has been run in the GRUB2_SRCDIR I can not run other
> configure command without receive this error message:
>   configure: error: source directory already configured; run "make distclean"
>   there first

Arg, I hadn't thought about this...

> To follow the logic I move all the *_CMDS_PTF as PRE_*_HOOK.
> I have tested it and it seems functioning.

But I'm afraid it is going to fail if you try to reconfigure the thing,
like:

    $ make grub2-reconfigure

So, in the end, your original proposal, to install each and every tuples
into target/, is probably the best option we have, and it can be as
simple as something like:

    diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk
    index e01ebb2edb..a18696b6cc 100644
    --- a/boot/grub2/grub2.mk
    +++ b/boot/grub2/grub2.mk
    @@ -195,5 +195,13 @@ define GRUB2_INSTALL_IMAGES_CMDS
        )
     endef

    +ifeq ($(BR2_TARGET_GRUB2_INSTALL_TOOLS),y)
    +define GRUB2_INSTALL_TARGET_CMDS
    +	$(foreach tuple, $(GRUB2_TUPLES-y), \
    +		$(TARGET_MAKE_ENV) $(MAKE) -C $(@D)/build-$(tuple) DESTDIR=$(TARGET_DIR) install
    +	)
    +endef
    +endif
    +
     $(eval $(generic-package))
     $(eval $(host-autotools-package))

Yes, this will indeed install the same tools more than once, but this is
not a problem: it is _not_ a case of a package overwriting files from
another package, but overwriting its own files, so we don't care.

Test-built with:

    BR2_x86_pentium4=y
    BR2_TOOLCHAIN_EXTERNAL=y
    BR2_TARGET_GRUB2=y
    BR2_TARGET_GRUB2_I386_PC=y
    BR2_TARGET_GRUB2_I386_EFI=y
    BR2_TARGET_GRUB2_INSTALL_TOOLS=y

Side note, unrelated to this issue: we have a parallel build issue with
grub2:

    2021-10-14 21:45:52 cd . && /bin/sh ./config.status config-util.h
    2021-10-14 21:45:52 config.status: creating config-util.h
    2021-10-14 21:45:52 bison -d -p grub_script_yy -b grub_script ../grub-core/script/parser.y
    2021-10-14 21:45:52 ../grub-core/script/parser.y:92.1-12: warning: deprecated directive: ‘%pure-parser’, use ‘%define api.pure’ [-Wdeprecated]
    2021-10-14 21:45:52    92 | %pure-parser
    2021-10-14 21:45:52       | ^~~~~~~~~~~~
    2021-10-14 21:45:52       | %define api.pure
    2021-10-14 21:45:52 ../grub-core/script/parser.y: warning: fix-its can be applied.  Rerun with option '--update'. [-Wother]
    2021-10-14 21:45:52 /home/ymorin/dev/buildroot/O/host/bin/i686-linux-gcc -E -DHAVE_CONFIG_H -I. -I..  -Wall -W -DGRUB_UTIL=1 -D_FILE_OFFSET_BITS=64 -I./include -DGRUB_FILE="util/grub-fstest.c" -I. -I.. -I. -I.. -I../include -I./include -I../grub-core/lib/libgcrypt-grub/src/ -I./grub-core/lib/gnulib -I../grub-core/lib/gnulib  -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -fno-stack-protector -D_FILE_OFFSET_BITS=64   -D'GRUB_MOD_INIT(x)=@MARKER@x@' ../util/grub-fstest.c ../grub-core/kern/emu/hostfs.c ../ grub-core/disk/host.c ../grub-core/osdep/init.c > grub_fstest.pp || (rm -f grub_fstest.pp; exit 1)
    2021-10-14 21:45:52 ../grub-core/kern/emu/hostfs.c:20:10: fatal error: config-util.h: No such file or directory
    2021-10-14 21:45:52    20 | #include <config-util.h>
    2021-10-14 21:45:52       |          ^~~~~~~~~~~~~~~
    2021-10-14 21:45:52 compilation terminated.
    2021-10-14 21:45:52 make[2]: *** [Makefile:13107: grub_fstest.pp] Error 1
    2021-10-14 21:45:52 make[2]: *** Waiting for unfinished jobs....

And re-starting the build succeeds.

But just looking at how config-util.h is actually created (in Makefile.in)
makes me cry:

   3389 config-util.h: stamp-h1
   3390     @test -f $@ || rm -f stamp-h1
   3391     @test -f $@ || $(MAKE) $(AM_MAKEFLAGS) stamp-h1
   3392
   3393 stamp-h1: $(srcdir)/config-util.h.in $(top_builddir)/config.status
   3394     @rm -f stamp-h1
   3395     cd $(top_builddir) && $(SHELL) ./config.status config-util.h

Whaaa?

But fortunately, it looks liek it is fixed with upstream commit
42f4054faf3c (Makefile: Make libgrub.pp depend on config-util.h).

Could you look into both the tools and the backport of the commit?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot