Re: [OE-core] [PATCH v6 1/3] bootimg_pcbios: initial import of grub legacy bios boot

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Sat, 2025-08-02 at 18:02 -0400, Vincent Davis Jr via lists.openembedded.org wrote:
> Moves syslinux config file creation,
> partition prepare, and mbr install
> tasks into there own seperate functions
> before removal from SourcePlugin class
> functions.
> 
> Add 3 new functions for creation of
> grub config, preparing boot partition
> to contain grub config and modules,
> and install grub boot.img to resulting
> wic image.
> 
> Signed-off-by: Vincent Davis Jr <vince@underview.tech>
> ---
>  .../lib/wic/plugins/source/bootimg_pcbios.py  | 351 +++++++++++++++++-
>  1 file changed, 350 insertions(+), 1 deletion(-)

Thanks, this is definitely getting better and more readable. We
discussed this a bit on the patch review call and there are a few more
tweaks which would help this be more understandable. We really need the
commits to show isolated specific changes and also explain both what
they're doing, and why.

I'd suggest the first commit should create the new grub functions and
call them from the existing call sites. This way we can just see the
code move without new code (such as the syslinux code) or any
functionality changes.

The second commit can then add the loader selection code (just calling
into grub).

The third commit would then add the new syslinux functions and their
calls.

The fourth patch adding a test is good.

The commit messages (and summary) should make it clear that the
existing code only supports grub and that the changes add syslinux
support alongside grub. It would also be helpful to mention what
happens to the defaults, I know that has changed as the patch series
has evolved.

Cheers,

Richard