[PATCH 1/3] uboot-config: fix devtool modify with kernel-fitimage

[PATCH 1/3] uboot-config: fix devtool modify with kernel-fitimage

[Adrian Freihofer]

From: Adrian Freihofer <adrian.freihofer@siemens.com>

How to reproduce:
- UBOOT_CONFIG must be used. With UBOOT_MACHINE it works fine.
  A simple example based on oe-core is to modify the
  beaglebone-yocto.conf file like this:
  -UBOOT_MACHINE = "am335x_evm_defconfig"
  +UBOOT_CONFIG = "foo"
  +UBOOT_CONFIG[foo] = "am335x_evm_defconfig"
- A build configuration which inherits the kernel-fitimage.bbclass is
  needed. For example:
  MACHINE = "beaglebone-yocto"
  KERNEL_IMAGETYPE = "Image"
  KERNEL_IMAGETYPES += " fitImage "
  KERNEL_CLASSES = " kernel-fitimage "

devtool modify linux-yocto
devtool build linux-yocto
...
| cp: cannot stat '.../linux-yocto-6.6.21+git/am335x_evm_defconfig/.config':
  No such file or directory
| WARNING: .../linux-yocto/6.6.21+git/temp/run.do_configure.2081673:172 exit 1
  from 'cp .../linux-yocto-6.6.21+git/am335x_evm_defconfig/.config
  .../build/workspace/sources/linux-yocto/.config.baseline'

The reason for this problem is that the uboot-config.bbclass sets the
variable KCONFIG_CONFIG_ROOTDIR to a path that makes sense for u-boot,
but not for other recipes. However, the kernel-fitimage.bbclasse, for
example, inherits the uboot-config.bbclass, which brings the
u-boot-specific path into the kernel build context.

This change removes the uboot-specific KCONFIG_CONFIG_ROOTDIR path from
recipes other than u-boot itself.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 meta/classes-recipe/uboot-config.bbclass | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/meta/classes-recipe/uboot-config.bbclass b/meta/classes-recipe/uboot-config.bbclass
index e55fc38b7c7..09001997d3d 100644
--- a/meta/classes-recipe/uboot-config.bbclass
+++ b/meta/classes-recipe/uboot-config.bbclass
@@ -101,9 +101,9 @@ python () {
     # The "doc" varflag is special, we don't want to see it here
     ubootconfigflags.pop('doc', None)
     ubootconfig = (d.getVar('UBOOT_CONFIG') or "").split()
+    PN = d.getVar("PN")
 
     if not ubootmachine and not ubootconfig:
-        PN = d.getVar("PN")
         FILE = os.path.basename(d.getVar("FILE"))
         bb.debug(1, "To build %s, see %s for instructions on \
                  setting up your machine config" % (PN, FILE))
@@ -140,9 +140,12 @@ python () {
             if not found:
                 raise bb.parse.SkipRecipe("The selected UBOOT_CONFIG key %s has no match in %s." % (ubootconfig, ubootconfigflags.keys()))
 
-            if len(ubootconfig) == 1:
-                d.setVar('KCONFIG_CONFIG_ROOTDIR', os.path.join(d.getVar("B"), d.getVar("UBOOT_MACHINE").strip()))
-            else:
-                # Disable menuconfig for multiple configs
-                d.setVar('KCONFIG_CONFIG_ENABLE_MENUCONFIG', "false")
+            # This recipe might be inherited e.g. by the kernel recipe via kernel-fitimage.bbclass
+            # Ensure the uboot specific menuconfig settings do not leak into other recipes
+            if 'u-boot' in PN:
+                if len(ubootconfig) == 1:
+                    d.setVar('KCONFIG_CONFIG_ROOTDIR', os.path.join(d.getVar("B"), d.getVar("UBOOT_MACHINE").strip()))
+                else:
+                    # Disable menuconfig for multiple configs
+                    d.setVar('KCONFIG_CONFIG_ENABLE_MENUCONFIG', "false")
 }
-- 
2.46.0

[PATCH 2/3] devtool: modify kernel adds append twice

[Adrian Freihofer]

From: Adrian Freihofer <adrian.freihofer@siemens.com>

Drop the redundant generation of the do_configure:append section for the
kernel. The same append is generated twice:

if bb.data.inherits_class('kernel', rd):
    f.write('\ndo_configure:append() {\n'
    '    cp ${B}/.config ${S}/.config.baseline\n'
    '    ln -sfT ${B}/.config ${S}/.config.new\n'
    '}\n')

KCONFIG_CONFIG_ENABLE_MENUCONFIG ??= "true"
KCONFIG_CONFIG_ROOTDIR ??= "${B}"
if rd.getVarFlag('do_menuconfig', 'task'):
    f.write('\ndo_configure:append() {\n'
    '    if [ ${@oe.types.boolean(d.getVar("KCONFIG_CONFIG_ENABLE_MENUCONFIG"))} = True ]; then\n'
    '        cp ${KCONFIG_CONFIG_ROOTDIR}/.config ${S}/.config.baseline\n'
    '        ln -sfT ${KCONFIG_CONFIG_ROOTDIR}/.config ${S}/.config.new\n'
    '    fi\n'
    '}\n')

In contradiction to the first code block the second code block considers
the variables which is correct.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 scripts/lib/devtool/standard.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 1d0fe137887..312eb8ab506 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -955,10 +955,6 @@ def modify(args, config, basepath, workspace):
                 f.write('SRCTREECOVEREDTASKS = "do_validate_branches do_kernel_checkout '
                         'do_fetch do_unpack do_kernel_configcheck"\n')
                 f.write('\ndo_patch[noexec] = "1"\n')
-                f.write('\ndo_configure:append() {\n'
-                        '    cp ${B}/.config ${S}/.config.baseline\n'
-                        '    ln -sfT ${B}/.config ${S}/.config.new\n'
-                        '}\n')
                 f.write('\ndo_kernel_configme:prepend() {\n'
                         '    if [ -e ${S}/.config ]; then\n'
                         '        mv ${S}/.config ${S}/.config.old\n'
-- 
2.46.0

[PATCH 3/3] devtool: remove obsolete SRCTREECOVEREDTASKS handling

[Adrian Freihofer]

From: Adrian Freihofer <adrian.freihofer@siemens.com>

The kernel-yocto.bbclass defines some tasks tasks and it also adds these
tasks to the SRCTREECOVEREDTASKS list. There is no need for devtool to
duplicate this code and override what the kernel-yocto.bbclass already
does.

devtool modify generates a linux-yocto.6.6.bbappend containing:
  SRCTREECOVEREDTASKS="\
    do_fetch \
    do_kernel_checkout \
    do_kernel_configcheck \
    do_unpack \
    do_validate_branches \
  "
  do_patch[noexec] = "1"

linux-yocto set SRCTREECOVEREDTASKS to
  SRCTREECOVEREDTASKS="\
    do_fetch \
    do_kernel_checkout \
    do_kernel_configcheck \
    do_patch \
    do_unpack \
    do_validate_branches \
  "

The code in devtool modify is therefore considered as redundant and
removed.

Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
---
 scripts/lib/devtool/standard.py | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 312eb8ab506..b2e1a6ca3a5 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -952,9 +952,6 @@ def modify(args, config, basepath, workspace):
                 f.write('EXTERNALSRC_BUILD:pn-%s = "%s"\n' % (pn, srctree))
 
             if bb.data.inherits_class('kernel', rd):
-                f.write('SRCTREECOVEREDTASKS = "do_validate_branches do_kernel_checkout '
-                        'do_fetch do_unpack do_kernel_configcheck"\n')
-                f.write('\ndo_patch[noexec] = "1"\n')
                 f.write('\ndo_kernel_configme:prepend() {\n'
                         '    if [ -e ${S}/.config ]; then\n'
                         '        mv ${S}/.config ${S}/.config.old\n'
-- 
2.46.0

Re: [OE-core] [PATCH 3/3] devtool: remove obsolete SRCTREECOVEREDTASKS handling

[Richard Purdie]

On Mon, 2024-09-02 at 22:31 +0200, Adrian Freihofer via lists.openembedded.org wrote:
> From: Adrian Freihofer <adrian.freihofer@siemens.com>
> 
> The kernel-yocto.bbclass defines some tasks tasks and it also adds these
> tasks to the SRCTREECOVEREDTASKS list. There is no need for devtool to
> duplicate this code and override what the kernel-yocto.bbclass already
> does.
> 
> devtool modify generates a linux-yocto.6.6.bbappend containing:
>   SRCTREECOVEREDTASKS="\
>     do_fetch \
>     do_kernel_checkout \
>     do_kernel_configcheck \
>     do_unpack \
>     do_validate_branches \
>   "
>   do_patch[noexec] = "1"
> 
> linux-yocto set SRCTREECOVEREDTASKS to
>   SRCTREECOVEREDTASKS="\
>     do_fetch \
>     do_kernel_checkout \
>     do_kernel_configcheck \
>     do_patch \
>     do_unpack \
>     do_validate_branches \
>   "
> 
> The code in devtool modify is therefore considered as redundant and
> removed.
> 
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  scripts/lib/devtool/standard.py | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
> index 312eb8ab506..b2e1a6ca3a5 100644
> --- a/scripts/lib/devtool/standard.py
> +++ b/scripts/lib/devtool/standard.py
> @@ -952,9 +952,6 @@ def modify(args, config, basepath, workspace):
>                  f.write('EXTERNALSRC_BUILD:pn-%s = "%s"\n' % (pn, srctree))
>  
>              if bb.data.inherits_class('kernel', rd):
> -                f.write('SRCTREECOVEREDTASKS = "do_validate_branches do_kernel_checkout '
> -                        'do_fetch do_unpack do_kernel_configcheck"\n')
> -                f.write('\ndo_patch[noexec] = "1"\n')
>                  f.write('\ndo_kernel_configme:prepend() {\n'
>                          '    if [ -e ${S}/.config ]; then\n'
>                          '        mv ${S}/.config ${S}/.config.old\n'


Note that "kernel" != "linux-yocto". Does a standard kernel still need fetch/unpack?

Cheers,

Richard

Re: [OE-core] [PATCH 1/3] uboot-config: fix devtool modify with kernel-fitimage

[Richard Purdie]

On Mon, 2024-09-02 at 22:31 +0200, Adrian Freihofer via lists.openembedded.org wrote:
> From: Adrian Freihofer <adrian.freihofer@siemens.com>
> 
> How to reproduce:
> - UBOOT_CONFIG must be used. With UBOOT_MACHINE it works fine.
>   A simple example based on oe-core is to modify the
>   beaglebone-yocto.conf file like this:
>   -UBOOT_MACHINE = "am335x_evm_defconfig"
>   +UBOOT_CONFIG = "foo"
>   +UBOOT_CONFIG[foo] = "am335x_evm_defconfig"
> - A build configuration which inherits the kernel-fitimage.bbclass is
>   needed. For example:
>   MACHINE = "beaglebone-yocto"
>   KERNEL_IMAGETYPE = "Image"
>   KERNEL_IMAGETYPES += " fitImage "
>   KERNEL_CLASSES = " kernel-fitimage "
> 
> devtool modify linux-yocto
> devtool build linux-yocto
> ...
> > cp: cannot stat '.../linux-yocto-6.6.21+git/am335x_evm_defconfig/.config':
>   No such file or directory
> > WARNING: .../linux-yocto/6.6.21+git/temp/run.do_configure.2081673:172 exit 1
>   from 'cp .../linux-yocto-6.6.21+git/am335x_evm_defconfig/.config
>   .../build/workspace/sources/linux-yocto/.config.baseline'
> 
> The reason for this problem is that the uboot-config.bbclass sets the
> variable KCONFIG_CONFIG_ROOTDIR to a path that makes sense for u-boot,
> but not for other recipes. However, the kernel-fitimage.bbclasse, for
> example, inherits the uboot-config.bbclass, which brings the
> u-boot-specific path into the kernel build context.
> 
> This change removes the uboot-specific KCONFIG_CONFIG_ROOTDIR path from
> recipes other than u-boot itself.
> 
> Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> ---
>  meta/classes-recipe/uboot-config.bbclass | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/meta/classes-recipe/uboot-config.bbclass b/meta/classes-recipe/uboot-config.bbclass
> index e55fc38b7c7..09001997d3d 100644
> --- a/meta/classes-recipe/uboot-config.bbclass
> +++ b/meta/classes-recipe/uboot-config.bbclass
> @@ -101,9 +101,9 @@ python () {
>      # The "doc" varflag is special, we don't want to see it here
>      ubootconfigflags.pop('doc', None)
>      ubootconfig = (d.getVar('UBOOT_CONFIG') or "").split()
> +    PN = d.getVar("PN")

Convention says uppercase strings are constants in python. I'd prefer
this to be something like recipename, particularly as the access isn't
immediately around the variable definition. This is mainly to try and
retain code consistency.


Cheers,

Richard

Re: [OE-core] [PATCH 3/3] devtool: remove obsolete SRCTREECOVEREDTASKS handling

[Adrian Freihofer]

On Tue, 2024-09-03 at 07:15 +0100, Richard Purdie wrote:
> On Mon, 2024-09-02 at 22:31 +0200, Adrian Freihofer via
> lists.openembedded.org wrote:
> > From: Adrian Freihofer <adrian.freihofer@siemens.com>
> > 
> > The kernel-yocto.bbclass defines some tasks tasks and it also adds
> > these
> > tasks to the SRCTREECOVEREDTASKS list. There is no need for devtool
> > to
> > duplicate this code and override what the kernel-yocto.bbclass
> > already
> > does.
> > 
> > devtool modify generates a linux-yocto.6.6.bbappend containing:
> >   SRCTREECOVEREDTASKS="\
> >     do_fetch \
> >     do_kernel_checkout \
> >     do_kernel_configcheck \
> >     do_unpack \
> >     do_validate_branches \
> >   "
> >   do_patch[noexec] = "1"
> > 
> > linux-yocto set SRCTREECOVEREDTASKS to
> >   SRCTREECOVEREDTASKS="\
> >     do_fetch \
> >     do_kernel_checkout \
> >     do_kernel_configcheck \
> >     do_patch \
> >     do_unpack \
> >     do_validate_branches \
> >   "
> > 
> > The code in devtool modify is therefore considered as redundant and
> > removed.
> > 
> > Signed-off-by: Adrian Freihofer <adrian.freihofer@siemens.com>
> > ---
> >  scripts/lib/devtool/standard.py | 3 ---
> >  1 file changed, 3 deletions(-)
> > 
> > diff --git a/scripts/lib/devtool/standard.py
> > b/scripts/lib/devtool/standard.py
> > index 312eb8ab506..b2e1a6ca3a5 100644
> > --- a/scripts/lib/devtool/standard.py
> > +++ b/scripts/lib/devtool/standard.py
> > @@ -952,9 +952,6 @@ def modify(args, config, basepath, workspace):
> >                  f.write('EXTERNALSRC_BUILD:pn-%s = "%s"\n' % (pn,
> > srctree))
> >  
> >              if bb.data.inherits_class('kernel', rd):
> > -                f.write('SRCTREECOVEREDTASKS =
> > "do_validate_branches do_kernel_checkout '
> > -                        'do_fetch do_unpack
> > do_kernel_configcheck"\n')
> > -                f.write('\ndo_patch[noexec] = "1"\n')
> >                  f.write('\ndo_kernel_configme:prepend() {\n'
> >                          '    if [ -e ${S}/.config ]; then\n'
> >                          '        mv ${S}/.config
> > ${S}/.config.old\n'
> 
> 
> Note that "kernel" != "linux-yocto". Does a standard kernel still
> need fetch/unpack?

externalsrc.bbclass defaults to:
SRCTREECOVEREDTASKS ?= "do_patch do_unpack do_fetch"
https://git.yoctoproject.org/poky/tree/meta/classes/externalsrc.bbclass#n28
This should be fine for "kernel" != "linux-yocto".

I will send a v2 which mentions this in the commit message and fixes
the other finding with the uppercase PN.

Thank you for the review.
Adrian


> 
> Cheers,
> 
> Richard
>