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

Submitted by Paul Eggleton on Sept. 11, 2012, 4:52 p.m.

Details

Message ID 2b9acb9c7fca686d4df885152458cae2e22f2cc5.1347382193.git.paul.eggleton@linux.intel.com
State Accepted, archived
Headers show

Commit Message

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(-)

Patch hide | download patch | download mbox

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):

Comments

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