diff mbox series

uboot-extlinux-config.bbclass: fix missed override syntax migration

Message ID 20231005-dev-uboot-extlinux-config-sstate-invalidate-v1-1-0f63ac16e38d@theobroma-systems.com
State Accepted, archived
Commit b4dd9d873508128adbbf5ff6cf0a3df3d2ffbcf6
Headers show
Series uboot-extlinux-config.bbclass: fix missed override syntax migration | expand

Commit Message

Quentin Schulz Oct. 5, 2023, 1:39 p.m. UTC
From: Quentin Schulz <quentin.schulz@theobroma-systems.com>

uboot-extlinux-config allows to specify multiple "labels" (entries in a
menu, à-la grub) and each of them have their own values for some fields.
Each "base" variable, e.g. UBOOT_EXTLINUX_FDT can be overridden for each
label. This is done via the OVERRIDES mechanism based on the label name,
e.g. UBOOT_EXTLINUX_FDT:linux if linux is a label.

However, OVERRIDES doesn't contain the label globally because it's only
necessary in one task. Therefore, the OVERRIDES itself is modified
within the task. This means that the sigdata will not be told the
dependency on UBOOT_EXTLINUX_FDT:linux, because it cannot know about it.

For this reason, we need to explicitly specify which variables this task
depends on via vardeps varflag for the task.

This was done in the past, but we missed updating it during the override
syntax migration so the cache wouldn't get invalidated if someone
modifies UBOOT_EXTLINUX_FDT:linux from a configuration file or a
bbappend for example.

Let's fix this by migrating it to the new syntax.

Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
---
Cc'ing Steve Sakoman for backporting to stable releases:
- kirkstone
- mickledore

Dunfell should be unnecessary since we support both syntaxes there.
---
 meta/classes-recipe/uboot-extlinux-config.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: 2b9044361f2855866eed831f8bdb770f2c7d42dc
change-id: 20231005-dev-uboot-extlinux-config-sstate-invalidate-c464136a168b

Best regards,

Comments

Quentin Schulz Oct. 16, 2023, 3:59 p.m. UTC | #1
Hi Steve,

I think this is appropriate for backporting to earlier branches. 
Considering that dunfell supports both old and new syntax, I don't think 
it matters for that branch.

https://git.openembedded.org/openembedded-core/commit/meta/classes-recipe/uboot-extlinux-config.bbclass?id=b4dd9d873508128adbbf5ff6cf0a3df3d2ffbcf6 
is the URL to the merged commit.

Lemme know how I can help,
Cheers,
Quentin

On 10/5/23 15:39, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> 
> uboot-extlinux-config allows to specify multiple "labels" (entries in a
> menu, à-la grub) and each of them have their own values for some fields.
> Each "base" variable, e.g. UBOOT_EXTLINUX_FDT can be overridden for each
> label. This is done via the OVERRIDES mechanism based on the label name,
> e.g. UBOOT_EXTLINUX_FDT:linux if linux is a label.
> 
> However, OVERRIDES doesn't contain the label globally because it's only
> necessary in one task. Therefore, the OVERRIDES itself is modified
> within the task. This means that the sigdata will not be told the
> dependency on UBOOT_EXTLINUX_FDT:linux, because it cannot know about it.
> 
> For this reason, we need to explicitly specify which variables this task
> depends on via vardeps varflag for the task.
> 
> This was done in the past, but we missed updating it during the override
> syntax migration so the cache wouldn't get invalidated if someone
> modifies UBOOT_EXTLINUX_FDT:linux from a configuration file or a
> bbappend for example.
> 
> Let's fix this by migrating it to the new syntax.
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> ---
> Cc'ing Steve Sakoman for backporting to stable releases:
> - kirkstone
> - mickledore
> 
> Dunfell should be unnecessary since we support both syntaxes there.
> ---
>   meta/classes-recipe/uboot-extlinux-config.bbclass | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes-recipe/uboot-extlinux-config.bbclass b/meta/classes-recipe/uboot-extlinux-config.bbclass
> index 30bbea57de..0413e760bd 100644
> --- a/meta/classes-recipe/uboot-extlinux-config.bbclass
> +++ b/meta/classes-recipe/uboot-extlinux-config.bbclass
> @@ -159,7 +159,7 @@ python do_create_extlinux_config() {
>           bb.fatal('Unable to open %s' % (cfile))
>   }
>   UBOOT_EXTLINUX_VARS = "CONSOLE MENU_DESCRIPTION ROOT KERNEL_IMAGE FDTDIR FDT KERNEL_ARGS INITRD"
> -do_create_extlinux_config[vardeps] += "${@' '.join(['UBOOT_EXTLINUX_%s_%s' % (v, l) for v in d.getVar('UBOOT_EXTLINUX_VARS').split() for l in d.getVar('UBOOT_EXTLINUX_LABELS').split()])}"
> +do_create_extlinux_config[vardeps] += "${@' '.join(['UBOOT_EXTLINUX_%s:%s' % (v, l) for v in d.getVar('UBOOT_EXTLINUX_VARS').split() for l in d.getVar('UBOOT_EXTLINUX_LABELS').split()])}"
>   do_create_extlinux_config[vardepsexclude] += "OVERRIDES"
>   
>   addtask create_extlinux_config before do_install do_deploy after do_compile
> 
> ---
> base-commit: 2b9044361f2855866eed831f8bdb770f2c7d42dc
> change-id: 20231005-dev-uboot-extlinux-config-sstate-invalidate-c464136a168b
> 
> Best regards,
Steve Sakoman Oct. 16, 2023, 4:16 p.m. UTC | #2
On Mon, Oct 16, 2023 at 5:59 AM Quentin Schulz
<quentin.schulz@theobroma-systems.com> wrote:
>
> Hi Steve,
>
> I think this is appropriate for backporting to earlier branches.
> Considering that dunfell supports both old and new syntax, I don't think
> it matters for that branch.
>
> https://git.openembedded.org/openembedded-core/commit/meta/classes-recipe/uboot-extlinux-config.bbclass?id=b4dd9d873508128adbbf5ff6cf0a3df3d2ffbcf6
> is the URL to the merged commit.

OK, will do.

Steve

> On 10/5/23 15:39, Quentin Schulz wrote:
> > From: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> >
> > uboot-extlinux-config allows to specify multiple "labels" (entries in a
> > menu, à-la grub) and each of them have their own values for some fields.
> > Each "base" variable, e.g. UBOOT_EXTLINUX_FDT can be overridden for each
> > label. This is done via the OVERRIDES mechanism based on the label name,
> > e.g. UBOOT_EXTLINUX_FDT:linux if linux is a label.
> >
> > However, OVERRIDES doesn't contain the label globally because it's only
> > necessary in one task. Therefore, the OVERRIDES itself is modified
> > within the task. This means that the sigdata will not be told the
> > dependency on UBOOT_EXTLINUX_FDT:linux, because it cannot know about it.
> >
> > For this reason, we need to explicitly specify which variables this task
> > depends on via vardeps varflag for the task.
> >
> > This was done in the past, but we missed updating it during the override
> > syntax migration so the cache wouldn't get invalidated if someone
> > modifies UBOOT_EXTLINUX_FDT:linux from a configuration file or a
> > bbappend for example.
> >
> > Let's fix this by migrating it to the new syntax.
> >
> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com>
> > ---
> > Cc'ing Steve Sakoman for backporting to stable releases:
> > - kirkstone
> > - mickledore
> >
> > Dunfell should be unnecessary since we support both syntaxes there.
> > ---
> >   meta/classes-recipe/uboot-extlinux-config.bbclass | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/meta/classes-recipe/uboot-extlinux-config.bbclass b/meta/classes-recipe/uboot-extlinux-config.bbclass
> > index 30bbea57de..0413e760bd 100644
> > --- a/meta/classes-recipe/uboot-extlinux-config.bbclass
> > +++ b/meta/classes-recipe/uboot-extlinux-config.bbclass
> > @@ -159,7 +159,7 @@ python do_create_extlinux_config() {
> >           bb.fatal('Unable to open %s' % (cfile))
> >   }
> >   UBOOT_EXTLINUX_VARS = "CONSOLE MENU_DESCRIPTION ROOT KERNEL_IMAGE FDTDIR FDT KERNEL_ARGS INITRD"
> > -do_create_extlinux_config[vardeps] += "${@' '.join(['UBOOT_EXTLINUX_%s_%s' % (v, l) for v in d.getVar('UBOOT_EXTLINUX_VARS').split() for l in d.getVar('UBOOT_EXTLINUX_LABELS').split()])}"
> > +do_create_extlinux_config[vardeps] += "${@' '.join(['UBOOT_EXTLINUX_%s:%s' % (v, l) for v in d.getVar('UBOOT_EXTLINUX_VARS').split() for l in d.getVar('UBOOT_EXTLINUX_LABELS').split()])}"
> >   do_create_extlinux_config[vardepsexclude] += "OVERRIDES"
> >
> >   addtask create_extlinux_config before do_install do_deploy after do_compile
> >
> > ---
> > base-commit: 2b9044361f2855866eed831f8bdb770f2c7d42dc
> > change-id: 20231005-dev-uboot-extlinux-config-sstate-invalidate-c464136a168b
> >
> > Best regards,
diff mbox series

Patch

diff --git a/meta/classes-recipe/uboot-extlinux-config.bbclass b/meta/classes-recipe/uboot-extlinux-config.bbclass
index 30bbea57de..0413e760bd 100644
--- a/meta/classes-recipe/uboot-extlinux-config.bbclass
+++ b/meta/classes-recipe/uboot-extlinux-config.bbclass
@@ -159,7 +159,7 @@  python do_create_extlinux_config() {
         bb.fatal('Unable to open %s' % (cfile))
 }
 UBOOT_EXTLINUX_VARS = "CONSOLE MENU_DESCRIPTION ROOT KERNEL_IMAGE FDTDIR FDT KERNEL_ARGS INITRD"
-do_create_extlinux_config[vardeps] += "${@' '.join(['UBOOT_EXTLINUX_%s_%s' % (v, l) for v in d.getVar('UBOOT_EXTLINUX_VARS').split() for l in d.getVar('UBOOT_EXTLINUX_LABELS').split()])}"
+do_create_extlinux_config[vardeps] += "${@' '.join(['UBOOT_EXTLINUX_%s:%s' % (v, l) for v in d.getVar('UBOOT_EXTLINUX_VARS').split() for l in d.getVar('UBOOT_EXTLINUX_LABELS').split()])}"
 do_create_extlinux_config[vardepsexclude] += "OVERRIDES"
 
 addtask create_extlinux_config before do_install do_deploy after do_compile