Patchwork [bitbake-devel,2/3] lib/bb/command.py: ensure setVariable only sets values as strings

login
register
mail settings
Submitter Paul Eggleton
Date Sept. 11, 2012, 4:52 p.m.
Message ID <2b9acb9c7fca686d4df885152458cae2e22f2cc5.1347382193.git.paul.eggleton@linux.intel.com>
Download mbox | patch
Permalink /patch/36335/
State New
Headers show

Comments

Paul Eggleton - Sept. 11, 2012, 4:52 p.m.
This is the interface Hob uses to set variable values in many instances,
and at the moment it is possible that some of the values it passes are
not strings. If a non-string value gets into the datastore it can
trigger exceptions during parsing when we attempt to expand the variable
and substitute in the non-string value.

This fixes using the meta-ti layer within Hob - it currently has a
reference to BB_NUMBER_THREADS within a shell function and since this
is a variable that Hob was setting from its configuration as an integer,
due to the above this was triggering an ExpansionError.

Fixes [YOCTO #2875].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 bitbake/lib/bb/command.py |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Chris Larson - Sept. 11, 2012, 10:57 p.m.
On Tue, Sep 11, 2012 at 9:52 AM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> This is the interface Hob uses to set variable values in many instances,
> and at the moment it is possible that some of the values it passes are
> not strings. If a non-string value gets into the datastore it can
> trigger exceptions during parsing when we attempt to expand the variable
> and substitute in the non-string value.
>
> This fixes using the meta-ti layer within Hob - it currently has a
> reference to BB_NUMBER_THREADS within a shell function and since this
> is a variable that Hob was setting from its configuration as an integer,
> due to the above this was triggering an ExpansionError.

This is probably a good change, but I think that failing expansion due
to a non-string value is a bad behavior in bitbake. I think it should
call str() on it as appropriate, as that's the most idiomatic
solution: Don't check if something is a string, don't require only
strings, but call str() to get your string
Paul Eggleton - Sept. 12, 2012, 12:29 p.m.
On Tuesday 11 September 2012 15:57:24 Chris Larson wrote:
> On Tue, Sep 11, 2012 at 9:52 AM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
> > This is the interface Hob uses to set variable values in many instances,
> > and at the moment it is possible that some of the values it passes are
> > not strings. If a non-string value gets into the datastore it can
> > trigger exceptions during parsing when we attempt to expand the variable
> > and substitute in the non-string value.
> > 
> > This fixes using the meta-ti layer within Hob - it currently has a
> > reference to BB_NUMBER_THREADS within a shell function and since this
> > is a variable that Hob was setting from its configuration as an integer,
> > due to the above this was triggering an ExpansionError.
> 
> This is probably a good change, but I think that failing expansion due
> to a non-string value is a bad behavior in bitbake. I think it should
> call str() on it as appropriate, as that's the most idiomatic
> solution: Don't check if something is a string, don't require only
> strings, but call str() to get your string

Hmm, I suppose so; I do still worry about existing python code assuming the 
result of getVar() is either None or a string though. Richard tells me that we 
have some non-string values mostly internal to BitBake that are considered 
legal, so that obviously isn't always 100% true, but it's a fair assumption to 
have made for most situations.

Cheers,
Paul
Richard Purdie - Sept. 12, 2012, 2:55 p.m.
On Wed, 2012-09-12 at 13:29 +0100, Paul Eggleton wrote:
> On Tuesday 11 September 2012 15:57:24 Chris Larson wrote:
> > On Tue, Sep 11, 2012 at 9:52 AM, Paul Eggleton
> > <paul.eggleton@linux.intel.com> wrote:
> > > This is the interface Hob uses to set variable values in many instances,
> > > and at the moment it is possible that some of the values it passes are
> > > not strings. If a non-string value gets into the datastore it can
> > > trigger exceptions during parsing when we attempt to expand the variable
> > > and substitute in the non-string value.
> > > 
> > > This fixes using the meta-ti layer within Hob - it currently has a
> > > reference to BB_NUMBER_THREADS within a shell function and since this
> > > is a variable that Hob was setting from its configuration as an integer,
> > > due to the above this was triggering an ExpansionError.
> > 
> > This is probably a good change, but I think that failing expansion due
> > to a non-string value is a bad behavior in bitbake. I think it should
> > call str() on it as appropriate, as that's the most idiomatic
> > solution: Don't check if something is a string, don't require only
> > strings, but call str() to get your string
> 
> Hmm, I suppose so; I do still worry about existing python code assuming the 
> result of getVar() is either None or a string though. Richard tells me that we 
> have some non-string values mostly internal to BitBake that are considered 
> legal, so that obviously isn't always 100% true, but it's a fair assumption to 
> have made for most situations.

If its being used in variable expansion, it should be a string or
convertible to a string so I have to admit I was thinking we probably
should have a str() conversion in there as well. I'm hoping that doesn't
have much overhead for something that is already a string. I'd also be
concerned about turning None into "None" which is probably the biggest
worry.

Cheers,

Richard

Patch

diff --git a/bitbake/lib/bb/command.py b/bitbake/lib/bb/command.py
index fd8912a..27b5171 100644
--- a/bitbake/lib/bb/command.py
+++ b/bitbake/lib/bb/command.py
@@ -157,7 +157,7 @@  class CommandsSync:
         Set the value of variable in configuration.data
         """
         varname = params[0]
-        value = params[1]
+        value = str(params[1])
         command.cooker.configuration.data.setVar(varname, value)
 
     def initCooker(self, command, params):