Re: [OE-core] [PATCH] meta: uboot-config: Fix broken menuconfig for "fitImage" kernel

[<mike.looijmans@topic.nl>]

On 22-05-2024 07:39, Richard Purdie wrote:
> On Wed, 2024-05-22 at 07:28 +0200, mike.looijmans@topic.nl wrote:
>> On 21-05-2024 16:51, Richard Purdie wrote:
>>> On Tue, 2024-05-21 at 15:57 +0200, Mike Looijmans via
>>> lists.openembedded.org wrote:
>>>> kernel-fitimage inherited uboot-config, which resulted in failure
>>>> to
>>>> run "menuconfig" on a system that has both multiple u-boot
>>>> configs and
>>>> wants a fitImage kernel.
>>>>
>>>> Fix the issue by splitting up uboot-config into uboot-config-vars
>>>> that
>>>> only contains the "harmless" variable settings, and inherit that
>>>> in a
>>>> new uboot-config class which contains the Python code to do the
>>>> U-boot
>>>> specific settings. Inherit only uboot-config-vars in kernel-
>>>> fitimage
>>>> so that it has what it needs but doesn't break.
>>>>
>>>> Fixes: f9e834e31788 "meta: introduce
>>>> KCONFIG_CONFIG_ENABLE_MENUCONFIG"
>>>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>>> ---
>>>>    meta/classes-recipe/kernel-fitimage.bbclass   |  2 +-
>>>>    meta/classes-recipe/uboot-config-vars.bbclass | 95
>>>> +++++++++++++++++++
>>>>    meta/classes-recipe/uboot-config.bbclass      | 80 +-----------
>>>> ----
>>>>    3 files changed, 97 insertions(+), 80 deletions(-)
>>>>    create mode 100644 meta/classes-recipe/uboot-config-
>>>> vars.bbclass
>>>
>>> This looks like it might be a good candidate to move to a file in
>>> meta/conf rather than a new class file?
>>
>> What does "this" refer to exactly?
> 
> The variable definitions you're placing into a separate class file. I'm
> saying they look more suited to a shared conf file, which is a
> direction I think the project needs to move in generally.

a "uboot.conf" then, okay.

> 
>> Note that this patch just moves code, it doesn't introduce anything
>> new.
>>
>>> We could add any functions needed to lib/oe/ if they can be made
>>> suitably generic.
>>
>> What "functions" would that be?
> 
> removesuffix is the one I saw that was needed in the new class file and
> wouldn't work in a conf file.

Interestingly, "removesuffix" is a built-in Python function since 3.8:

 >>> 'hello.all'.removesuffix('.all')
'hello'

And 3.8 is the minimum required version for Yocto anyway.


-- 
Mike Looijmans