[PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Tom Rini]

Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
that we evaluate it fully before performing the live and vmdk logic
checks.

Signed-off-by: Tom Rini <tom.rini@gmail.com>
---
 meta/classes/image.bbclass | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 79de5a2..ccfa1b1 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -78,10 +78,11 @@ do_rootfs[vardeps] += "BAD_RECOMMENDATIONS NO_RECOMMENDATIONS"
 
 do_build[depends] += "virtual/kernel:do_deploy"
 
+EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
 def build_live(d):
-    if base_contains("IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
-        d.setVar('NOISO', base_contains('IMAGE_FSTYPES', "iso", "0", "1", d))
-        d.setVar('NOHDD', base_contains('IMAGE_FSTYPES', "hddimg", "0", "1", d))
+    if base_contains("EVALED_IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
+        d.setVar('NOISO', base_contains('EVALED_IMAGE_FSTYPES', "iso", "0", "1", d))
+        d.setVar('NOHDD', base_contains('EVALED_IMAGE_FSTYPES', "hddimg", "0", "1", d))
         if d.getVar('NOISO', True) == "0" or d.getVar('NOHDD', True) == "0":
             return "image-live"
         return ""
@@ -90,7 +91,7 @@ def build_live(d):
 IMAGE_TYPE_live = "${@build_live(d)}"
 
 inherit ${IMAGE_TYPE_live}
-IMAGE_TYPE_vmdk = '${@base_contains("IMAGE_FSTYPES", "vmdk", "image-vmdk", "", d)}'
+IMAGE_TYPE_vmdk = '${@base_contains("EVALED_IMAGE_FSTYPES", "vmdk", "image-vmdk", "", d)}'
 inherit ${IMAGE_TYPE_vmdk}
 
 python () {
-- 
1.9.3

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Otavio Salvador]

On Thu, Jun 26, 2014 at 9:27 AM, Tom Rini <tom.rini@gmail.com> wrote:
> Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
> that we evaluate it fully before performing the live and vmdk logic
> checks.
>
> Signed-off-by: Tom Rini <tom.rini@gmail.com>

Do you have an example how it used to fail?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Tom Rini]

On Thu, Jun 26, 2014 at 02:25:34PM -0300, Otavio Salvador wrote:
> On Thu, Jun 26, 2014 at 9:27 AM, Tom Rini <tom.rini@gmail.com> wrote:
> > Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
> > that we evaluate it fully before performing the live and vmdk logic
> > checks.
> >
> > Signed-off-by: Tom Rini <tom.rini@gmail.com>
> 
> Do you have an example how it used to fail?

Sure:
diff --git a/meta/recipes-core/images/build-appliance-image_8.0.bb b/meta/recipes-core/images/build-appliance-image_8.0.bb
index 7b70025..23bb02e 100644
--- a/meta/recipes-core/images/build-appliance-image_8.0.bb
+++ b/meta/recipes-core/images/build-appliance-image_8.0.bb
@@ -17,7 +17,7 @@ IMAGE_ROOTFS_EXTRA_SPACE = "41943040"
 APPEND += "quiet"
 
 DEPENDS = "zip-native"
-IMAGE_FSTYPES = "vmdk"
+IMAGE_FSTYPES_x86-64 = "vmdk"
 
 inherit core-image
 
And now you won't get vmdk images on qemux86-64 (same for live, did the
same proof-of-concept there when working up the patch).

-- 
Tom

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Otavio Salvador]

On Fri, Jun 27, 2014 at 9:42 AM, Tom Rini <tom.rini@gmail.com> wrote:
> On Thu, Jun 26, 2014 at 02:25:34PM -0300, Otavio Salvador wrote:
>> On Thu, Jun 26, 2014 at 9:27 AM, Tom Rini <tom.rini@gmail.com> wrote:
>> > Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
>> > that we evaluate it fully before performing the live and vmdk logic
>> > checks.
>> >
>> > Signed-off-by: Tom Rini <tom.rini@gmail.com>
>>
>> Do you have an example how it used to fail?
>
> Sure:
> diff --git a/meta/recipes-core/images/build-appliance-image_8.0.bb b/meta/recipes-core/images/build-appliance-image_8.0.bb
> index 7b70025..23bb02e 100644
> --- a/meta/recipes-core/images/build-appliance-image_8.0.bb
> +++ b/meta/recipes-core/images/build-appliance-image_8.0.bb
> @@ -17,7 +17,7 @@ IMAGE_ROOTFS_EXTRA_SPACE = "41943040"
>  APPEND += "quiet"
>
>  DEPENDS = "zip-native"
> -IMAGE_FSTYPES = "vmdk"
> +IMAGE_FSTYPES_x86-64 = "vmdk"
>
>  inherit core-image
>
> And now you won't get vmdk images on qemux86-64 (same for live, did the
> same proof-of-concept there when working up the patch).

Isn't it a bug in BitBake?

I'd expect it to work fine. Richard, can you provide some feedback?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Richard Purdie]

On Thu, 2014-06-26 at 08:27 -0400, Tom Rini wrote:
> Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
> that we evaluate it fully before performing the live and vmdk logic
> checks.
> 
> Signed-off-by: Tom Rini <tom.rini@gmail.com>
> ---
>  meta/classes/image.bbclass | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 79de5a2..ccfa1b1 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -78,10 +78,11 @@ do_rootfs[vardeps] += "BAD_RECOMMENDATIONS NO_RECOMMENDATIONS"
>  
>  do_build[depends] += "virtual/kernel:do_deploy"
>  
> +EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
>  def build_live(d):
> -    if base_contains("IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> -        d.setVar('NOISO', base_contains('IMAGE_FSTYPES', "iso", "0", "1", d))
> -        d.setVar('NOHDD', base_contains('IMAGE_FSTYPES', "hddimg", "0", "1", d))
> +    if base_contains("EVALED_IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> +        d.setVar('NOISO', base_contains('EVALED_IMAGE_FSTYPES', "iso", "0", "1", d))
> +        d.setVar('NOHDD', base_contains('EVALED_IMAGE_FSTYPES', "hddimg", "0", "1", d))
>          if d.getVar('NOISO', True) == "0" or d.getVar('NOHDD', True) == "0":
>              return "image-live"
>          return ""

The trouble is this doesn't add up. 

EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"

basically sets EVALED_... to the expanded value of IMAGE_FSTYPES

base_contains() also expands the variable its passed.

I'm a little concerned as to why those two expansions of the same thing
should result in something different.

From your later explaination, it would appear the use of the override is
causing problems but I don't understand how immediate expansion would
work around that :/.

So before this merges, more investigation is needed as there is
something not adding up...

Cheers,

Richard


> @@ -90,7 +91,7 @@ def build_live(d):
>  IMAGE_TYPE_live = "${@build_live(d)}"
>  
>  inherit ${IMAGE_TYPE_live}
> -IMAGE_TYPE_vmdk = '${@base_contains("IMAGE_FSTYPES", "vmdk", "image-vmdk", "", d)}'
> +IMAGE_TYPE_vmdk = '${@base_contains("EVALED_IMAGE_FSTYPES", "vmdk", "image-vmdk", "", d)}'
>  inherit ${IMAGE_TYPE_vmdk}
>  
>  python () {
> -- 
> 1.9.3
> 

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Tom Rini]

On Fri, Jun 27, 2014 at 02:40:44PM +0100, Richard Purdie wrote:
> On Thu, 2014-06-26 at 08:27 -0400, Tom Rini wrote:
> > Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
> > that we evaluate it fully before performing the live and vmdk logic
> > checks.
> > 
> > Signed-off-by: Tom Rini <tom.rini@gmail.com>
> > ---
> >  meta/classes/image.bbclass | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index 79de5a2..ccfa1b1 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -78,10 +78,11 @@ do_rootfs[vardeps] += "BAD_RECOMMENDATIONS NO_RECOMMENDATIONS"
> >  
> >  do_build[depends] += "virtual/kernel:do_deploy"
> >  
> > +EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
> >  def build_live(d):
> > -    if base_contains("IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> > -        d.setVar('NOISO', base_contains('IMAGE_FSTYPES', "iso", "0", "1", d))
> > -        d.setVar('NOHDD', base_contains('IMAGE_FSTYPES', "hddimg", "0", "1", d))
> > +    if base_contains("EVALED_IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> > +        d.setVar('NOISO', base_contains('EVALED_IMAGE_FSTYPES', "iso", "0", "1", d))
> > +        d.setVar('NOHDD', base_contains('EVALED_IMAGE_FSTYPES', "hddimg", "0", "1", d))
> >          if d.getVar('NOISO', True) == "0" or d.getVar('NOHDD', True) == "0":
> >              return "image-live"
> >          return ""
> 
> The trouble is this doesn't add up. 
> 
> EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
> 
> basically sets EVALED_... to the expanded value of IMAGE_FSTYPES
> 
> base_contains() also expands the variable its passed.
> 
> I'm a little concerned as to why those two expansions of the same thing
> should result in something different.

Well, they're happening at different parts in the parse.  The vmdk
example is clearer I think since it's not another "big" python function.

> From your later explaination, it would appear the use of the override is
> causing problems but I don't understand how immediate expansion would
> work around that :/.
> 
> So before this merges, more investigation is needed as there is
> something not adding up...

Well, I think the high level answer is that no, we aren't applying
OVERRIDES at the point when we're parsing classes still rather than a
specific task.  Adding an immediate eval at this level does force things
and then we see what we're supposed to.  My gut feeling here is that if
we poke deeply enough at base_contains we'll see that we aren't passing
along the expand flag when we thought we were.

-- 
Tom

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Richard Purdie]

On Fri, 2014-06-27 at 10:16 -0400, Tom Rini wrote:
> On Fri, Jun 27, 2014 at 02:40:44PM +0100, Richard Purdie wrote:
> > On Thu, 2014-06-26 at 08:27 -0400, Tom Rini wrote:
> > > Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
> > > that we evaluate it fully before performing the live and vmdk logic
> > > checks.
> > > 
> > > Signed-off-by: Tom Rini <tom.rini@gmail.com>
> > > ---
> > >  meta/classes/image.bbclass | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > > index 79de5a2..ccfa1b1 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -78,10 +78,11 @@ do_rootfs[vardeps] += "BAD_RECOMMENDATIONS NO_RECOMMENDATIONS"
> > >  
> > >  do_build[depends] += "virtual/kernel:do_deploy"
> > >  
> > > +EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
> > >  def build_live(d):
> > > -    if base_contains("IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> > > -        d.setVar('NOISO', base_contains('IMAGE_FSTYPES', "iso", "0", "1", d))
> > > -        d.setVar('NOHDD', base_contains('IMAGE_FSTYPES', "hddimg", "0", "1", d))
> > > +    if base_contains("EVALED_IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> > > +        d.setVar('NOISO', base_contains('EVALED_IMAGE_FSTYPES', "iso", "0", "1", d))
> > > +        d.setVar('NOHDD', base_contains('EVALED_IMAGE_FSTYPES', "hddimg", "0", "1", d))
> > >          if d.getVar('NOISO', True) == "0" or d.getVar('NOHDD', True) == "0":
> > >              return "image-live"
> > >          return ""
> > 
> > The trouble is this doesn't add up. 
> > 
> > EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
> > 
> > basically sets EVALED_... to the expanded value of IMAGE_FSTYPES
> > 
> > base_contains() also expands the variable its passed.
> > 
> > I'm a little concerned as to why those two expansions of the same thing
> > should result in something different.
> 
> Well, they're happening at different parts in the parse.  The vmdk
> example is clearer I think since it's not another "big" python function.
> 
> > From your later explaination, it would appear the use of the override is
> > causing problems but I don't understand how immediate expansion would
> > work around that :/.
> > 
> > So before this merges, more investigation is needed as there is
> > something not adding up...
> 
> Well, I think the high level answer is that no, we aren't applying
> OVERRIDES at the point when we're parsing classes still rather than a
> specific task.  Adding an immediate eval at this level does force things
> and then we see what we're supposed to.  My gut feeling here is that if
> we poke deeply enough at base_contains we'll see that we aren't passing
> along the expand flag when we thought we were.

I wondered about that but I did already check and:

http://git.yoctoproject.org/cgit.cgi/poky/tree/bitbake/lib/bb/utils.py

def contains(variable, checkvalues, truevalue, falsevalue, d):
    val = d.getVar(variable, True)

so it is being expanded afaict...

Cheers,

Richard

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Tom Rini]

On Fri, Jun 27, 2014 at 03:54:30PM +0100, Richard Purdie wrote:
> On Fri, 2014-06-27 at 10:16 -0400, Tom Rini wrote:
> > On Fri, Jun 27, 2014 at 02:40:44PM +0100, Richard Purdie wrote:
> > > On Thu, 2014-06-26 at 08:27 -0400, Tom Rini wrote:
> > > > Incase we have overrides applied to IMAGE_FSTYPES we need to make sure
> > > > that we evaluate it fully before performing the live and vmdk logic
> > > > checks.
> > > > 
> > > > Signed-off-by: Tom Rini <tom.rini@gmail.com>
> > > > ---
> > > >  meta/classes/image.bbclass | 9 +++++----
> > > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > > > index 79de5a2..ccfa1b1 100644
> > > > --- a/meta/classes/image.bbclass
> > > > +++ b/meta/classes/image.bbclass
> > > > @@ -78,10 +78,11 @@ do_rootfs[vardeps] += "BAD_RECOMMENDATIONS NO_RECOMMENDATIONS"
> > > >  
> > > >  do_build[depends] += "virtual/kernel:do_deploy"
> > > >  
> > > > +EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
> > > >  def build_live(d):
> > > > -    if base_contains("IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> > > > -        d.setVar('NOISO', base_contains('IMAGE_FSTYPES', "iso", "0", "1", d))
> > > > -        d.setVar('NOHDD', base_contains('IMAGE_FSTYPES', "hddimg", "0", "1", d))
> > > > +    if base_contains("EVALED_IMAGE_FSTYPES", "live", "live", "0", d) == "0": # live is not set but hob might set iso or hddimg
> > > > +        d.setVar('NOISO', base_contains('EVALED_IMAGE_FSTYPES', "iso", "0", "1", d))
> > > > +        d.setVar('NOHDD', base_contains('EVALED_IMAGE_FSTYPES', "hddimg", "0", "1", d))
> > > >          if d.getVar('NOISO', True) == "0" or d.getVar('NOHDD', True) == "0":
> > > >              return "image-live"
> > > >          return ""
> > > 
> > > The trouble is this doesn't add up. 
> > > 
> > > EVALED_IMAGE_FSTYPES := "${IMAGE_FSTYPES}"
> > > 
> > > basically sets EVALED_... to the expanded value of IMAGE_FSTYPES
> > > 
> > > base_contains() also expands the variable its passed.
> > > 
> > > I'm a little concerned as to why those two expansions of the same thing
> > > should result in something different.
> > 
> > Well, they're happening at different parts in the parse.  The vmdk
> > example is clearer I think since it's not another "big" python function.
> > 
> > > From your later explaination, it would appear the use of the override is
> > > causing problems but I don't understand how immediate expansion would
> > > work around that :/.
> > > 
> > > So before this merges, more investigation is needed as there is
> > > something not adding up...
> > 
> > Well, I think the high level answer is that no, we aren't applying
> > OVERRIDES at the point when we're parsing classes still rather than a
> > specific task.  Adding an immediate eval at this level does force things
> > and then we see what we're supposed to.  My gut feeling here is that if
> > we poke deeply enough at base_contains we'll see that we aren't passing
> > along the expand flag when we thought we were.
> 
> I wondered about that but I did already check and:
> 
> http://git.yoctoproject.org/cgit.cgi/poky/tree/bitbake/lib/bb/utils.py
> 
> def contains(variable, checkvalues, truevalue, falsevalue, d):
>     val = d.getVar(variable, True)
> 
> so it is being expanded afaict...

Yeah, but what does that expand back to?  My bitbake internals-fu is
weak, but:
$ git grep def\ getVar
lib/bb/command.py:    def getVariable(self, command, params):
lib/bb/data.py:def getVar(var, d, exp = 0):
lib/bb/data.py:def getVarFlag(var, flag, d):
lib/bb/data.py:def getVarFlags(var, d):
lib/bb/data_smart.py:    def getVar(self, var, expand=False, noweakdefault=False
lib/bb/data_smart.py:    def getVarFlag(self, var, flag, expand=False, noweakdef
lib/bb/data_smart.py:    def getVarFlags(self, var, expand = False, internalflag

So where do we map back to a 'getVar' that takes expand as arg 2 not arg
3?

-- 
Tom

Re: [PATCH] image.bbclass: Ensure IMAGE_FSTYPES is fully evaluated before live/live logic

[Richard Purdie]

On Fri, 2014-06-27 at 11:17 -0400, Tom Rini wrote:
> On Fri, Jun 27, 2014 at 03:54:30PM +0100, Richard Purdie wrote:
> > I wondered about that but I did already check and:
> > 
> > http://git.yoctoproject.org/cgit.cgi/poky/tree/bitbake/lib/bb/utils.py
> > 
> > def contains(variable, checkvalues, truevalue, falsevalue, d):
> >     val = d.getVar(variable, True)
> > 
> > so it is being expanded afaict...
> 
> Yeah, but what does that expand back to?  My bitbake internals-fu is
> weak, but:
> $ git grep def\ getVar
> lib/bb/command.py:    def getVariable(self, command, params):
> lib/bb/data.py:def getVar(var, d, exp = 0):
> lib/bb/data.py:def getVarFlag(var, flag, d):
> lib/bb/data.py:def getVarFlags(var, d):
> lib/bb/data_smart.py:    def getVar(self, var, expand=False, noweakdefault=False
> lib/bb/data_smart.py:    def getVarFlag(self, var, flag, expand=False, noweakdef
> lib/bb/data_smart.py:    def getVarFlags(self, var, expand = False, internalflag
> 
> So where do we map back to a 'getVar' that takes expand as arg 2 not arg
> 3?

Its calling:

lib/bb/data_smart.py:    def getVar(self, var, expand=False, noweakdefault=False

since d is a data store object which becomes "self", the first argument
and the other two are then passed in with expand = True.

The bb.data.getVar(d, var, expand) syntax is just clumsy and deprecated
when you can write the same thing as d.getVar(var, expand).

Cheers,

Richard