diff mbox series

wic: bootimg-efi-isar: fix generating of grub.cfg for custom initrd

Message ID 20250110115028.256057-1-peter.czegledy@evosoft.com
State Superseded, archived
Headers show
Series wic: bootimg-efi-isar: fix generating of grub.cfg for custom initrd | expand

Commit Message

peter.czegledy Jan. 10, 2025, 11:50 a.m. UTC 
From: Peter Czegledy <peter.czegledy@evosoft.com>

Specifying the initrd location via WICs sourceparams initrd argument doesn't generate the initrd location properly.

This fixes that and furthermore cleans up some related redundant assignments to kernel variable as they do not impact the final value.

Signed-off-by: Peter Czegledy <peter.czegledy@evosoft.com>
---
 .../lib/wic/plugins/source/bootimg-efi-isar.py   | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

Comments

Jan Kiszka Jan. 10, 2025, 4:05 p.m. UTC | #1 
On 10.01.25 12:50, peter.czegledy via isar-users wrote:
> From: Peter Czegledy <peter.czegledy@evosoft.com>
> 
> Specifying the initrd location via WICs sourceparams initrd argument doesn't generate the initrd location properly.
> 
> This fixes that and furthermore cleans up some related redundant assignments to kernel variable as they do not impact the final value.
> 
> Signed-off-by: Peter Czegledy <peter.czegledy@evosoft.com>
> ---
>  .../lib/wic/plugins/source/bootimg-efi-isar.py   | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> index 50f4187d..243824ae 100644
> --- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> +++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> @@ -97,20 +97,18 @@ class BootimgEFIPlugin(SourcePlugin):
>              grubefi_conf += "timeout=%s\n" % bootloader.timeout
>              grubefi_conf += "menuentry '%s'{\n" % (title if title else "boot")
>  
> -            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
> -            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
> -                if get_bitbake_var("INITRAMFS_IMAGE"):
> -                    kernel = "%s-%s.bin" % \
> -                        (get_bitbake_var("KERNEL_IMAGETYPE"), get_bitbake_var("INITRAMFS_LINK_NAME"))
> -

Please split the cleanups into a separate patch.

BTW, that dead code comes from taking OE's bootimg-efi.py as template
and then adding isar specifics to it. See
scripts/lib/wic/plugins/source/bootimg-efi.py.

>              label = source_params.get('label')
>              label_conf = "root=%s" % creator.rootdev
>              if label:
>                  label_conf = "LABEL=%s" % label
>  
> +            temp_initrd = initrd
>              kernel, initrd = isar_get_filenames(
>                  get_bitbake_var("IMAGE_ROOTFS"), get_bitbake_var("KERNEL_FILE")
>              )
> +            if temp_initrd:
> +                initrd = temp_initrd
> +

This is a bit strange pattern, even if we have it elsewhere already.
Better (and shorter):

kernel, default_initrd = isar_get_filenames(...)
if not initrd:
    initrd = default_initrd

>              grubefi_conf += "linux /%s %s rootwait %s\n" \
>                  % (kernel, label_conf, bootloader.append)
>  
> @@ -179,12 +177,6 @@ class BootimgEFIPlugin(SourcePlugin):
>  
>          if not custom_cfg:
>              # Create systemd-boot configuration using parameters from wks file
> -            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
> -            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
> -                if get_bitbake_var("INITRAMFS_IMAGE"):
> -                    kernel = "%s-%s.bin" % \
> -                        (get_bitbake_var("KERNEL_IMAGETYPE"), get_bitbake_var("INITRAMFS_LINK_NAME"))
> -
>              title = source_params.get('title')
>  
>              temp_initrd = initrd

Jan
Quirin Gylstorff Jan. 10, 2025, 4:42 p.m. UTC | #2 
On 1/10/25 12:50, peter.czegledy via isar-users wrote:
> From: Peter Czegledy <peter.czegledy@evosoft.com>
> 
> Specifying the initrd location via WICs sourceparams initrd argument doesn't generate the initrd location properly.
> 
> This fixes that and furthermore cleans up some related redundant assignments to kernel variable as they do not impact the final value.
> 
> Signed-off-by: Peter Czegledy <peter.czegledy@evosoft.com>
> ---
>   .../lib/wic/plugins/source/bootimg-efi-isar.py   | 16 ++++------------
>   1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> index 50f4187d..243824ae 100644
> --- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> +++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
> @@ -97,20 +97,18 @@ class BootimgEFIPlugin(SourcePlugin):
>               grubefi_conf += "timeout=%s\n" % bootloader.timeout
>               grubefi_conf += "menuentry '%s'{\n" % (title if title else "boot")
>   
> -            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
> -            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
> -                if get_bitbake_var("INITRAMFS_IMAGE"):
> -                    kernel = "%s-%s.bin" % \
> -                        (get_bitbake_var("KERNEL_IMAGETYPE"), get_bitbake_var("INITRAMFS_LINK_NAME"))
> -

This is a recipe fork from open-embedded and we intentionally try to 
keep the changes a small as possible to allow merging.

Quirin

>               label = source_params.get('label')
>               label_conf = "root=%s" % creator.rootdev
>               if label:
>                   label_conf = "LABEL=%s" % label
>   
> +            temp_initrd = initrd
>               kernel, initrd = isar_get_filenames(
>                   get_bitbake_var("IMAGE_ROOTFS"), get_bitbake_var("KERNEL_FILE")
>               )
> +            if temp_initrd:
> +                initrd = temp_initrd
> +
>               grubefi_conf += "linux /%s %s rootwait %s\n" \
>                   % (kernel, label_conf, bootloader.append)
>   
> @@ -179,12 +177,6 @@ class BootimgEFIPlugin(SourcePlugin):
>   
>           if not custom_cfg:
>               # Create systemd-boot configuration using parameters from wks file
> -            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
> -            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
> -                if get_bitbake_var("INITRAMFS_IMAGE"):
> -                    kernel = "%s-%s.bin" % \
> -                        (get_bitbake_var("KERNEL_IMAGETYPE"), get_bitbake_var("INITRAMFS_LINK_NAME"))
> -
>               title = source_params.get('title')
>   
>               temp_initrd = initrd
Niedermayr, BENEDIKT Jan. 13, 2025, 4:16 p.m. UTC | #3 
On 10.01.25 17:42, 'Quirin Gylstorff' via isar-users wrote:
>
>
> On 1/10/25 12:50, peter.czegledy via isar-users wrote:
>> From: Peter Czegledy <peter.czegledy@evosoft.com>
>>
>> Specifying the initrd location via WICs sourceparams initrd argument 
>> doesn't generate the initrd location properly.
>>
>> This fixes that and furthermore cleans up some related redundant 
>> assignments to kernel variable as they do not impact the final value.
>>
>> Signed-off-by: Peter Czegledy <peter.czegledy@evosoft.com>
>> ---
>>   .../lib/wic/plugins/source/bootimg-efi-isar.py   | 16 ++++------------
>>   1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py 
>> b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>> index 50f4187d..243824ae 100644
>> --- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>> +++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
>> @@ -97,20 +97,18 @@ class BootimgEFIPlugin(SourcePlugin):
>>               grubefi_conf += "timeout=%s\n" % bootloader.timeout
>>               grubefi_conf += "menuentry '%s'{\n" % (title if title 
>> else "boot")
>>   -            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
>> -            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
>> -                if get_bitbake_var("INITRAMFS_IMAGE"):
>> -                    kernel = "%s-%s.bin" % \
>> -                        (get_bitbake_var("KERNEL_IMAGETYPE"), 
>> get_bitbake_var("INITRAMFS_LINK_NAME"))
>> -
>
> This is a recipe fork from open-embedded and we intentionally try to 
> keep the changes a small as possible to allow merging.
>
> Quirin

I think this would be worth a comment section? I also missed the fact 
that this is a OE fork.

Benedikt


>
>>               label = source_params.get('label')
>>               label_conf = "root=%s" % creator.rootdev
>>               if label:
>>                   label_conf = "LABEL=%s" % label
>>   +            temp_initrd = initrd
>>               kernel, initrd = isar_get_filenames(
>>                   get_bitbake_var("IMAGE_ROOTFS"), 
>> get_bitbake_var("KERNEL_FILE")
>>               )
>> +            if temp_initrd:
>> +                initrd = temp_initrd
>> +
>>               grubefi_conf += "linux /%s %s rootwait %s\n" \
>>                   % (kernel, label_conf, bootloader.append)
>>   @@ -179,12 +177,6 @@ class BootimgEFIPlugin(SourcePlugin):
>>             if not custom_cfg:
>>               # Create systemd-boot configuration using parameters 
>> from wks file
>> -            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
>> -            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
>> -                if get_bitbake_var("INITRAMFS_IMAGE"):
>> -                    kernel = "%s-%s.bin" % \
>> -                        (get_bitbake_var("KERNEL_IMAGETYPE"), 
>> get_bitbake_var("INITRAMFS_LINK_NAME"))
>> -
>>               title = source_params.get('title')
>>                 temp_initrd = initrd
>
diff mbox series

Patch

diff --git a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
index 50f4187d..243824ae 100644
--- a/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
+++ b/meta/scripts/lib/wic/plugins/source/bootimg-efi-isar.py
@@ -97,20 +97,18 @@  class BootimgEFIPlugin(SourcePlugin):
             grubefi_conf += "timeout=%s\n" % bootloader.timeout
             grubefi_conf += "menuentry '%s'{\n" % (title if title else "boot")
 
-            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
-            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
-                if get_bitbake_var("INITRAMFS_IMAGE"):
-                    kernel = "%s-%s.bin" % \
-                        (get_bitbake_var("KERNEL_IMAGETYPE"), get_bitbake_var("INITRAMFS_LINK_NAME"))
-
             label = source_params.get('label')
             label_conf = "root=%s" % creator.rootdev
             if label:
                 label_conf = "LABEL=%s" % label
 
+            temp_initrd = initrd
             kernel, initrd = isar_get_filenames(
                 get_bitbake_var("IMAGE_ROOTFS"), get_bitbake_var("KERNEL_FILE")
             )
+            if temp_initrd:
+                initrd = temp_initrd
+
             grubefi_conf += "linux /%s %s rootwait %s\n" \
                 % (kernel, label_conf, bootloader.append)
 
@@ -179,12 +177,6 @@  class BootimgEFIPlugin(SourcePlugin):
 
         if not custom_cfg:
             # Create systemd-boot configuration using parameters from wks file
-            kernel = get_bitbake_var("KERNEL_IMAGETYPE")
-            if get_bitbake_var("INITRAMFS_IMAGE_BUNDLE") == "1":
-                if get_bitbake_var("INITRAMFS_IMAGE"):
-                    kernel = "%s-%s.bin" % \
-                        (get_bitbake_var("KERNEL_IMAGETYPE"), get_bitbake_var("INITRAMFS_LINK_NAME"))
-
             title = source_params.get('title')
 
             temp_initrd = initrd