Re: [bitbake-devel] [PATCH 1/1] data_smart: fix operations lost when an override name contains a variable

Minh Tiến Nguyễn <[email protected]> Sun, 26 Jul 2026 21:53:54 +0700
Newsgroups org.openembedded.lists.bitbake-devel
Message-ID <CAH9t78za_ncy=tnBv--DFqBvsxU-VCX8xGZaj_P-ef3PGvircA@mail.gmail.com>
> I think this is because expandKeys() calls renameVar and then
> renameVar itself also calls renameVar() on the same element, which
> breaks things. [...] then I think that resolve the issue?

Yes, it does, and it is the better fix. Both keys are already in the
todolist with their expanded names, so expandKeys() can rename them
both. My patch only worked around the second rename with expand().

I tested your diff on a fresh clone at c251833d2. It fixes the exact
form from the bug report, :prepend and :remove, a conditional override
with no operation keyword, several variables in one override name, and a
variable in the key as well as in the override name, all of which fail
on master. bb.tests.data passes, oe-core parses clean at 952 recipes
with no new warnings, and both v1 test cases pass on your diff unchanged.

> I'm less sure that three different key variables would work with plain
> renameVar, that may also need to set the "no recurse" option.

It does not need it, and it must not have it. In the native.bbclass
shape a key is renamed to another unexpanded key before expandKeys()
runs. recurse=3DTrue gives the right answer with no warnings. I tried
recurse=3DFalse there and the override value is lost: the dependent key is
left behind, and expandKeys() then expands it from the old parent name,
so it no longer matches.

Two things before I send a v2.

First, the new argument breaks recipe_sanity.bbclass, which replaces
DataSmart.renameVar with a wrapper taking a fixed set of arguments. Any
build using INHERIT +=3D "recipe_sanity" now dies before parsing starts:

    TypeError: myrename() got an unexpected keyword argument 'recurse'

A one line oe-core patch passing *args/**kwargs through fixes it, it
just has to go in at the same time.

Second, and this is why I have not sent a v2 yet. There is an ordering
bug here and it is not yours. Master already has it, in the case where
nothing gets dropped:

    ABC =3D "123"
    ABC:append:pn-linux-${XYZ} =3D " 456"
    ABC:append:cfg-${SFX} =3D " 789"

    master gives   "123 789 456"
    should be      "123 456 789"

Both appends are applied, just in the wrong order, and your diff does
not change that case at all.

It matters because of the case your diff does fix. Master drops those
operations, so nobody ever sees the order being wrong. Once they stop
being dropped, it shows:

    PN =3D "packagegroup-cross-canadian-qemux86"

    RDEPENDS:${PN} =3D "base"
    RDEPENDS:${PN}:append:pn-packagegroup-cross-canadian-${MACHINE} =3D " a=
"
    RDEPENDS:${PN}:append:libc-${TCLIBC} =3D " b"
    RDEPENDS:${PN}:append:class-target =3D " c"

    master      "base c"      a and b dropped
    your diff   "base c b a"
    my v1       "base c a b"
    should be   "base a b c"

pn-${PN}, libc-glibc and class-target are all in OVERRIDES at once, so
all three appends are meant to apply.

The reason seems to be __setvar_regexp__, which ends in
(:(?P<add>[^A-Z]*))?$. That came from 6eb56624e, where you stopped
capitalised overrides being processed. ${MACHINE} has uppercase in it,
so an override name holding it fails that test and is stored as a
variable of its own, while a plain override name passes and becomes an
entry in the :append flag on the base variable. The operations for one
variable then live in two places, and the flag group is applied first
regardless of what was written first. With your diff all six orderings
of a, b and c give "base c b a". Mine keeps the order within the second
group but still puts c first, so it is wrong as well.

That line leaves a second gap neither patch closes. A lowercase variable
name passes the regexp, so an append written with ${machine} rather than
${MACHINE} becomes a flag whose override name is still
pn-gizmo-${machine}, and that name is never expanded when overrides are
matched. Master, your diff and mine all return "base" for it. Nobody
writes ${machine} in practice, but it is another way to lose the same
operation.

Would you like me to fix the drop now and report the ordering as its own
bug, or look at these together? I did not want to send a v2 that changes
ordering without asking first.

Best regards,
Tien


V=C3=A0o CN, 26 thg 7, 2026 va=CC=80o lu=CC=81c 19:21 Richard Purdie
<[email protected]> =C4=91=C3=A3 vi=E1=BA=BFt:

>
> On Sat, 2026-07-25 at 21:30 +0700, Zk47T via lists.openembedded.org wrote=
:
> > An operation whose override name needs key expansion is silently droppe=
d:
> >
> >     RDEPENDS:${PN}:append:pn-foo-${MACHINE} =3D " bar"
> >
> > renameVar() rebuilds the dependent override keys with a plain string
> > replace, so renaming RDEPENDS:${PN} leaves the append attached to
> > RDEPENDS:foo:append:pn-foo-${MACHINE}, which never matches an active
> > override. expandKeys() cannot fix that up afterwards either, as it work=
s
> > from a list of keys collected before any renaming happened.
> >
> > Expand the derived name before renaming it. Only do so once newkey is
> > itself expanded, otherwise expandKeys() has still to rename newkey and
> > handles the dependent keys along with it.
> >
> > Add regression tests for both cases.
> >
> > Fixes [YOCTO #14867]
> >
> > Signed-off-by: Nguyen Minh Tien <[email protected]>
> > ---
> >  lib/bb/data_smart.py | 11 +++++++++--
> >  lib/bb/tests/data.py | 25 +++++++++++++++++++++++++
> >  2 files changed, 34 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
> > index 9961269a3..5738aff95 100644
> > --- a/lib/bb/data_smart.py
> > +++ b/lib/bb/data_smart.py
> > @@ -697,8 +697,15 @@ class DataSmart(MutableMapping):
> >              found =3D True
> >              self.overridedata[newkey] =3D []
> >              for (v, o) in self.overridedata[key]:
> > -                self.overridedata[newkey].append([v.replace(key, newke=
y), o])
> > -                self.renameVar(v, v.replace(key, newkey))
> > +                newv =3D v.replace(key, newkey)
> > +                # The derived name may still hold a variable reference=
 which
> > +                # expandKeys() will never revisit, so expand it here. =
Only once
> > +                # newkey is expanded though, otherwise expandKeys() ha=
s still to
> > +                # rename newkey and handles the dependent keys along w=
ith it.
> > +                if '${' in newv and '${' not in newkey:
> > +                    newv =3D self.expand(newv)
> > +                self.overridedata[newkey].append([newv, o])
> > +                self.renameVar(v, newv)
> >
>
> Thanks for the patch and the test cases. I had a deeper look at this as
> I was a bit puzzled how this would work without your patch:
>
> XYZ =3D "yocto"
> ABC =3D "123"
> ABC:append:pn-linux-${XYZ} =3D " 456"
> $ bitbake-getvar -r linux-yocto ABC
> ABC=3D"123 456"
>
> as if the code can handle that, it should be able to handle the other
> case too. I think this is because expandKeys() calls renameVar and then
> renameVar itself also calls renameVar() on the same element, which
> breaks things. That means that if you do:
>
> diff --git a/lib/bb/data.py b/lib/bb/data.py
> index 5fdcdb04a..e3af12a35 100644
> --- a/lib/bb/data.py
> +++ b/lib/bb/data.py
> @@ -94,7 +94,7 @@ def expandKeys(alterdata, readdata =3D None):
>              val =3D alterdata.getVar(key, False)
>              if val is not None:
>                  bb.warn("Variable key %s (%s) replaces original key %s (=
%s)." % (key, val, ekey, newval))
> -        alterdata.renameVar(key, ekey)
> +        alterdata.renameVar(key, ekey, recurse=3DFalse)
>
>  def inheritFromOS(d, savedenv, permitted):
>      """Inherit variables from the initial environment."""
> diff --git a/lib/bb/data_smart.py b/lib/bb/data_smart.py
> index 110dfa111..78ff9b961 100644
> --- a/lib/bb/data_smart.py
> +++ b/lib/bb/data_smart.py
> @@ -661,7 +661,7 @@ class DataSmart(MutableMapping):
>      def getVar(self, var, expand=3DTrue, noweakdefault=3DFalse, parsing=
=3DFalse):
>          return self.getVarFlag(var, "_content", expand, noweakdefault, p=
arsing)
>
> -    def renameVar(self, key, newkey, **loginfo):
> +    def renameVar(self, key, newkey, recurse=3DTrue, **loginfo):
>          """
>          Rename the variable key to newkey
>          """
> @@ -693,7 +693,8 @@ class DataSmart(MutableMapping):
>              self.overridedata[newkey] =3D []
>              for (v, o) in self.overridedata[key]:
>                  self.overridedata[newkey].append([v.replace(key, newkey)=
, o])
> -                self.renameVar(v, v.replace(key, newkey))
> +                if recurse:
> +                    self.renameVar(v, v.replace(key, newkey))
>
>          if ':' in newkey and val is None:
>              self._setvar_update_overrides(newkey, **loginfo)
>
>
>
> then I think that resolve the issue? It would also perhaps resolve an
> issue where a variable with three different key expansions in it might
> not work! I'm less sure that three different key variables would work
> with plain renameVar, that may also need to set the "no recurse"
> option, I'm not sure.
>
> Could you see if this makes sense to you?
>
> Cheers,
>
> Richard
>
>
>
>