| Message ID | 20250912161917.844344-2-dwagenknecht@emlix.com |
|---|---|
| State | New |
| Headers | show |
| Series | os-release: do not add empty parentheses to VERSION | expand |
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Daniel Wagenknecht > Sent: den 12 september 2025 18:18 > To: openembedded-core@lists.openembedded.org > Cc: Daniel Wagenknecht <dwagenknecht@emlix.com> > Subject: [OE-core] [PATCH] os-release: do not add empty parentheses to VERSION > > Setting DISTRO_CODENAME to an empty string previously led to a VERSION > field in /etc/os-release containing empty parantheses, e.g. > DISTRO_VERSION = "5.0.12" > DISTRO_CODENAME = "" > ==> /etc/os-release: > VERSION="5.0.12 ()" > > This is probably not what a user expects, especially since it is quite > common to set variables to empty strings to disable something in OE > based builds, but using `unset VARNAME` seems pretty uncommon. > > Instead of adding the parentheses with the DISTRO_CODENAME if the > variable is in the datastore add them only if the variable is not empty. > > Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com> > --- > meta/recipes-core/os-release/os-release.bb | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meta/recipes-core/os-release/os-release.bb b/meta/recipes-core/os-release/os-release.bb > index e1906d05d..65e63a342 100644 > --- a/meta/recipes-core/os-release/os-release.bb > +++ b/meta/recipes-core/os-release/os-release.bb > @@ -22,7 +22,7 @@ OS_RELEASE_UNQUOTED_FIELDS = "ID VERSION_ID VARIANT_ID" > > ID = "${DISTRO}" > NAME = "${DISTRO_NAME}" > -VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if 'DISTRO_CODENAME' in d else ''}" > +VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if d.getVar('DISTRO_CODENAME') != '' else ''}" To cover all bases you should then use: VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if (d.getVar('DISTRO_CODENAME') or '') != '' else ''}" One thing I do not understand, and this is mostly a question for RP, is how the first DISTRO_CODENAME is actually expanded in the above definition. I would have expected it to be: VERSION = "${DISTRO_VERSION}${@' (%s)' % d.getvar('DISTRO_CODENAME') if (d.getVar('DISTRO_CODENAME') or '') != '' else ''}" Or are all variables in d somehow available as Python variables inside ${@...}? Because then a lot of such definitions could be simplified, like VERSION_CODENAME below which would become: VERSION_CODENAME = "${@DISTRO_CODENAME or ''}". (Though that also seems odd as it could just as well be: VERSION_CODENAME = "${DISTRO_CODENAME}" ) > VERSION_ID = "${DISTRO_VERSION}" > VERSION_CODENAME = "${@d.getVar('DISTRO_CODENAME') or ''}" > PRETTY_NAME = "${DISTRO_NAME} ${VERSION}" > -- > 2.50.1 //Peter
Hello Peter, thanks for the Review! On Sat, 2025-09-13 at 10:28 +0000, Peter Kjellerstedt wrote: > > VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if > (d.getVar('DISTRO_CODENAME') or '') != '' else ''}" you are correct, I'll send a v2. > One thing I do not understand, and this is mostly a question for RP, > is how the first DISTRO_CODENAME is actually expanded in the above > definition. > [...] > Or are all variables in d somehow available as Python variables > inside ${@...}? This is indeed very interesting. I tried to trace the variable expansion code in bitbake/lib/bb/data_smart.py and found this commit from 2010: https://git.openembedded.org/bitbake/commit/?id=606fa1fd97cbd47a6a7ebdc7a2e6aa93a8f65cf5 > Access metadata vars as locals in python snippets > > Example: > FOO = "bar" > BAR = "${@FOO + '/baz'}" > > ${BAR} == "bar/baz" So `${@VARNAME}` expanding the vars in inline python expressions seems to be intended but undocumented and mostly unused behaviour. Sincerely Daniel Wagenknecht
> -----Original Message----- > From: Daniel Wagenknecht <dwagenknecht@emlix.com> > Sent: den 22 september 2025 10:06 > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-core@lists.openembedded.org; Richard Purdie <richard.purdie@linuxfoundation.org> > Subject: Re: [OE-core] [PATCH] os-release: do not add empty parentheses to VERSION > > Hello Peter, > > thanks for the Review! > > On Sat, 2025-09-13 at 10:28 +0000, Peter Kjellerstedt wrote: > > > > VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if (d.getVar('DISTRO_CODENAME') or '') != '' else ''}" > > you are correct, I'll send a v2. > > > One thing I do not understand, and this is mostly a question for RP, > > is how the first DISTRO_CODENAME is actually expanded in the above > > definition. > > [...] > > Or are all variables in d somehow available as Python variables > > inside ${@...}? > > This is indeed very interesting. I tried to trace the variable > expansion code in bitbake/lib/bb/data_smart.py and found this commit > from 2010: > https://git.openembedded.org/bitbake/commit/?id=606fa1fd97cbd47a6a7ebdc7a2e6aa93a8f65cf5 > > > Access metadata vars as locals in python snippets > > > > Example: > > FOO = "bar" > > BAR = "${@FOO + '/baz'}" > > > > ${BAR} == "bar/baz" > > So `${@VARNAME}` expanding the vars in inline python expressions seems > to be intended but undocumented and mostly unused behaviour. > > Sincerely > Daniel Wagenknecht We discussed this in the tech meeting today. Basically I wondered whether this use of direct access to variables in inline Python expressions is something that is encouraged or discouraged. RP responded that this is something that was integrated 15 years ago as an experiment, which was then more or less forgotten. Someone should evaluate the performance implications of using this before its use is encouraged. Adrian F also mentioned that there are some patches for uboot(?) that rely on this as apparently the evaluation of the variables is different compared to when they are fetched using d.getVar(). //Peter
> -----Original Message----- > From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> On Behalf Of Peter Kjellerstedt vialists.openembedded.org > Sent: den 23 september 2025 17:55 > To: openembedded-core@lists.openembedded.org; Richard Purdie <richard.purdie@linuxfoundation.org> > Cc: Daniel Wagenknecht <dwagenknecht@emlix.com>; Adrian Freihofer <adrian.freihofer@gmail.com> > Subject: [OE-core] Direct expansion of vars in inline Python expressions (was: RE: [PATCH] os-release: do not add empty parentheses to VERSION) > > > -----Original Message----- > > From: Daniel Wagenknecht <dwagenknecht@emlix.com> > > Sent: den 22 september 2025 10:06 > > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-core@lists.openembedded.org; Richard Purdie <richard.purdie@linuxfoundation.org> > > Subject: Re: [OE-core] [PATCH] os-release: do not add empty parentheses to VERSION > > > > Hello Peter, > > > > thanks for the Review! > > > > On Sat, 2025-09-13 at 10:28 +0000, Peter Kjellerstedt wrote: > > > > > > VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if (d.getVar('DISTRO_CODENAME') or '') != '' else ''}" > > > > you are correct, I'll send a v2. > > > > > One thing I do not understand, and this is mostly a question for RP, > > > is how the first DISTRO_CODENAME is actually expanded in the above > > > definition. > > > [...] > > > Or are all variables in d somehow available as Python variables > > > inside ${@...}? > > > > This is indeed very interesting. I tried to trace the variable > > expansion code in bitbake/lib/bb/data_smart.py and found this commit > > from 2010: > > https://git.openembedded.org/bitbake/commit/?id=606fa1fd97cbd47a6a7ebdc7a2e6aa93a8f65cf5 > > > > > Access metadata vars as locals in python snippets > > > > > > Example: > > > FOO = "bar" > > > BAR = "${@FOO + '/baz'}" > > > > > > ${BAR} == "bar/baz" > > > > So `${@VARNAME}` expanding the vars in inline python expressions seems > > to be intended but undocumented and mostly unused behaviour. > > > > Sincerely > > Daniel Wagenknecht > > We discussed this in the tech meeting today. Basically I wondered whether > this use of direct access to variables in inline Python expressions is > something that is encouraged or discouraged. RP responded that this is > something that was integrated 15 years ago as an experiment, which was > then more or less forgotten. Someone should evaluate the performance > implications of using this before its use is encouraged. Adrian F also > mentioned that there are some patches for uboot(?) that rely on this as > apparently the evaluation of the variables is different compared to when > they are fetched using d.getVar(). > > //Peter And when I wrote "Someone" above, that does not imply RP. I would be happy to help, but I have absolutely no experience in how to evaluate Python performance... Some aspects that I think should be tested are: * What if the support is removed again, will that improve/worsen memory usage and/or build times? * If as many current d.getVar() as possible are turned into direct variable expressions, will that improve/worsen memory usage and/or build times? //Peter
diff --git a/meta/recipes-core/os-release/os-release.bb b/meta/recipes-core/os-release/os-release.bb index e1906d05d..65e63a342 100644 --- a/meta/recipes-core/os-release/os-release.bb +++ b/meta/recipes-core/os-release/os-release.bb @@ -22,7 +22,7 @@ OS_RELEASE_UNQUOTED_FIELDS = "ID VERSION_ID VARIANT_ID" ID = "${DISTRO}" NAME = "${DISTRO_NAME}" -VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if 'DISTRO_CODENAME' in d else ''}" +VERSION = "${DISTRO_VERSION}${@' (%s)' % DISTRO_CODENAME if d.getVar('DISTRO_CODENAME') != '' else ''}" VERSION_ID = "${DISTRO_VERSION}" VERSION_CODENAME = "${@d.getVar('DISTRO_CODENAME') or ''}" PRETTY_NAME = "${DISTRO_NAME} ${VERSION}"
Setting DISTRO_CODENAME to an empty string previously led to a VERSION field in /etc/os-release containing empty parantheses, e.g. DISTRO_VERSION = "5.0.12" DISTRO_CODENAME = "" ==> /etc/os-release: VERSION="5.0.12 ()" This is probably not what a user expects, especially since it is quite common to set variables to empty strings to disable something in OE based builds, but using `unset VARNAME` seems pretty uncommon. Instead of adding the parentheses with the DISTRO_CODENAME if the variable is in the datastore add them only if the variable is not empty. Signed-off-by: Daniel Wagenknecht <dwagenknecht@emlix.com> --- meta/recipes-core/os-release/os-release.bb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)