All of lore.kernel.org
 help / color / mirror / Atom feed
* Layer.conf require of meta-virt-default-versions.inc
@ 2020-02-07 15:42 Bertrand Marquis
  2020-02-10  8:05 ` [meta-virtualization] " Paul Barker
  0 siblings, 1 reply; 13+ messages in thread
From: Bertrand Marquis @ 2020-02-07 15:42 UTC (permalink / raw)
  To: meta-virtualization; +Cc: nd

Hi Everyone,

Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.

After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.

Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
- using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
- even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf

Could anyone confirm or explain me where I am wrong here ?

Here are the tests I did to end up with those conclusions:
- "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
  So the file is not found here.
- “require conf/distro/include/meta-virt-default-versions.inc” build correctly
  So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
  So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
  So the file is not included here

Is there a right way to achieve what is want in layer.conf ?

Thanks
Bertrand

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-07 15:42 Layer.conf require of meta-virt-default-versions.inc Bertrand Marquis
@ 2020-02-10  8:05 ` Paul Barker
  2020-02-10 11:12   ` Bertrand Marquis
                     ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Paul Barker @ 2020-02-10  8:05 UTC (permalink / raw)
  To: Bertrand Marquis; +Cc: meta-virtualization, nd, bruce.ashfield

On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
>
> Hi Everyone,
>
> Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
>
> After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.

This is correct - layer.conf in each layer is parsed before local.conf.

>
> Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
>
> Could anyone confirm or explain me where I am wrong here ?

Looking at layer.conf in meta-virtualization on the master branch, it
does worry me a bit that these lines might not work as expected:

    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'virtualization', 'meta-virt-cfg', '', d)}"
    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'k8s', 'meta-virt-k8s-cfg', '', d)}"

Perhaps these should become unconditional appends to USER_CLASSES and
the DISTRO_FEATURES checks should be done inside those classes.

Bruce, any thoughts on this one for meta-virtualization?

>
> Here are the tests I did to end up with those conclusions:
> - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
>   So the file is not found here.
> - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
>   So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
>   So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
>   So the file is not included here
>
> Is there a right way to achieve what is want in layer.conf ?

You need to delay parsing of your variable until after bitbake.conf is
parsed (which includes local.conf and other related files). The
simplest way to do this is to place the logic in a bbclass and add
this to the INHERIT variable in your layer.conf. The classes in
INHERIT are picked up as if they were at the start of each recipe that
is parsed.

Thanks,
Paul

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10  8:05 ` [meta-virtualization] " Paul Barker
@ 2020-02-10 11:12   ` Bertrand Marquis
  2020-02-10 14:19     ` Bruce Ashfield
  2020-02-10 14:17   ` Bruce Ashfield
  2020-02-10 14:41   ` Bruce Ashfield
  2 siblings, 1 reply; 13+ messages in thread
From: Bertrand Marquis @ 2020-02-10 11:12 UTC (permalink / raw)
  To: meta-virtualization; +Cc: nd, bruce.ashfield, Paul Barker



> On 10 Feb 2020, at 08:05, Paul Barker <pbarker@konsulko.com> wrote:
> 
> On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
>> 
>> Hi Everyone,
>> 
>> Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
>> 
>> After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
> 
> This is correct - layer.conf in each layer is parsed before local.conf.
> 
>> 
>> Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
>> - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
>> - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
>> 
>> Could anyone confirm or explain me where I am wrong here ?
> 
> Looking at layer.conf in meta-virtualization on the master branch, it
> does worry me a bit that these lines might not work as expected:
> 
>    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'virtualization', 'meta-virt-cfg', '', d)}"
>    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'k8s', 'meta-virt-k8s-cfg', '', d)}"
> 
> Perhaps these should become unconditional appends to USER_CLASSES and
> the DISTRO_FEATURES checks should be done inside those classes.
> 
> Bruce, any thoughts on this one for meta-virtualization?
> 
>> 
>> Here are the tests I did to end up with those conclusions:
>> - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
>>  So the file is not found here.
>> - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
>>  So the file is properly included here.
>> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
>>  So the file is properly included here.
>> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
>>  So the file is not included here
>> 
>> Is there a right way to achieve what is want in layer.conf ?
> 
> You need to delay parsing of your variable until after bitbake.conf is
> parsed (which includes local.conf and other related files). The
> simplest way to do this is to place the logic in a bbclass and add
> this to the INHERIT variable in your layer.conf. The classes in
> INHERIT are picked up as if they were at the start of each recipe that
> is parsed.
> 

Thanks a lot, I will give this a look but enforcing PREFERRED_VERSION in a bbclass does not really seem to be something standard.

Does someone knows meta-virtualization tries to enforce versions on python ? 
It seems that meta-virt-default-versions.inc is in fact never included (in Zeus or in master at least) so I am wondering if the content should not be removed all together ?
The file was introduced in case an other layer is providing a different version of some packages to enforce the ones provided by meta-virtualization but in fact in master the file is empty.

A the end the complete system with a class to include meta-virt-default-versions.inc if virtualization is in DISTRO_FEATURES could be removed.

Thanks,
Bertrand


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10  8:05 ` [meta-virtualization] " Paul Barker
  2020-02-10 11:12   ` Bertrand Marquis
@ 2020-02-10 14:17   ` Bruce Ashfield
  2020-02-10 14:23     ` Bertrand Marquis
  2020-02-10 14:41   ` Bruce Ashfield
  2 siblings, 1 reply; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 14:17 UTC (permalink / raw)
  To: Paul Barker; +Cc: Bertrand Marquis, meta-virtualization, nd

On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
> >
> > Hi Everyone,
> >
> > Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
> >
> > After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
>
> This is correct - layer.conf in each layer is parsed before local.conf.
>
> >
> > Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> > - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> > - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
> >
> > Could anyone confirm or explain me where I am wrong here ?
>
> Looking at layer.conf in meta-virtualization on the master branch, it
> does worry me a bit that these lines might not work as expected:
>
>     USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'virtualization', 'meta-virt-cfg', '', d)}"
>     USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'k8s', 'meta-virt-k8s-cfg', '', d)}"

They do work as expected. See the post from Mark Hatle (to the mailing
list) explaining the details of why they are setup as they are.

This is one of the only ways to ensure that something which is
parsed/expanded early enough can then be used in the layer conf to do
version pinning like this.

That being said, what part of those lines do you think might not be working ?

Bruce

>
> Perhaps these should become unconditional appends to USER_CLASSES and
> the DISTRO_FEATURES checks should be done inside those classes.
>
> Bruce, any thoughts on this one for meta-virtualization?
>
> >
> > Here are the tests I did to end up with those conclusions:
> > - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
> >   So the file is not found here.
> > - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
> >   So the file is properly included here.
> > - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
> >   So the file is properly included here.
> > - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
> >   So the file is not included here
> >
> > Is there a right way to achieve what is want in layer.conf ?
>
> You need to delay parsing of your variable until after bitbake.conf is
> parsed (which includes local.conf and other related files). The
> simplest way to do this is to place the logic in a bbclass and add
> this to the INHERIT variable in your layer.conf. The classes in
> INHERIT are picked up as if they were at the start of each recipe that
> is parsed.
>
> Thanks,
> Paul



--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 11:12   ` Bertrand Marquis
@ 2020-02-10 14:19     ` Bruce Ashfield
  0 siblings, 0 replies; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 14:19 UTC (permalink / raw)
  To: Bertrand Marquis; +Cc: meta-virtualization, nd, Paul Barker

On Mon, Feb 10, 2020 at 6:12 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
>
>
> > On 10 Feb 2020, at 08:05, Paul Barker <pbarker@konsulko.com> wrote:
> >
> > On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
> >>
> >> Hi Everyone,
> >>
> >> Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
> >>
> >> After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
> >
> > This is correct - layer.conf in each layer is parsed before local.conf.
> >
> >>
> >> Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> >> - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> >> - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
> >>
> >> Could anyone confirm or explain me where I am wrong here ?
> >
> > Looking at layer.conf in meta-virtualization on the master branch, it
> > does worry me a bit that these lines might not work as expected:
> >
> >    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> > 'virtualization', 'meta-virt-cfg', '', d)}"
> >    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> > 'k8s', 'meta-virt-k8s-cfg', '', d)}"
> >
> > Perhaps these should become unconditional appends to USER_CLASSES and
> > the DISTRO_FEATURES checks should be done inside those classes.
> >
> > Bruce, any thoughts on this one for meta-virtualization?
> >
> >>
> >> Here are the tests I did to end up with those conclusions:
> >> - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
> >>  So the file is not found here.
> >> - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
> >>  So the file is properly included here.
> >> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
> >>  So the file is properly included here.
> >> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
> >>  So the file is not included here
> >>
> >> Is there a right way to achieve what is want in layer.conf ?
> >
> > You need to delay parsing of your variable until after bitbake.conf is
> > parsed (which includes local.conf and other related files). The
> > simplest way to do this is to place the logic in a bbclass and add
> > this to the INHERIT variable in your layer.conf. The classes in
> > INHERIT are picked up as if they were at the start of each recipe that
> > is parsed.
> >
>
> Thanks a lot, I will give this a look but enforcing PREFERRED_VERSION in a bbclass does not really seem to be something standard.
>
> Does someone knows meta-virtualization tries to enforce versions on python ?

git history and the mailing list archives have all the details, but
the summary is that some of the major packages have very specific
version dependencies (which change over time), so we have to pin those
versions.

> It seems that meta-virt-default-versions.inc is in fact never included (in Zeus or in master at least) so I am wondering if the content should not be removed all together ?

No, it has been used for quite some time (on and off), it stays as a
placeholder for when it will be used again. See the comments in both
git history and in the file itself.

> The file was introduced in case an other layer is providing a different version of some packages to enforce the ones provided by meta-virtualization but in fact in master the file is empty.
>
> A the end the complete system with a class to include meta-virt-default-versions.inc if virtualization is in DISTRO_FEATURES could be removed.
>

definitely not.

Bruce

> Thanks,
> Bertrand
>


-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 14:17   ` Bruce Ashfield
@ 2020-02-10 14:23     ` Bertrand Marquis
  2020-02-10 14:35       ` Bruce Ashfield
  0 siblings, 1 reply; 13+ messages in thread
From: Bertrand Marquis @ 2020-02-10 14:23 UTC (permalink / raw)
  To: Bruce Ashfield, meta-virtualization; +Cc: Paul Barker, nd

[-- Attachment #1: Type: text/plain, Size: 3788 bytes --]



On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>> wrote:

On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com<mailto:pbarker@konsulko.com>> wrote:

On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com<mailto:bertrand.marquis@arm.com>> wrote:

Hi Everyone,

Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.

After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.

This is correct - layer.conf in each layer is parsed before local.conf.


Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
- using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
- even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf

Could anyone confirm or explain me where I am wrong here ?

Looking at layer.conf in meta-virtualization on the master branch, it
does worry me a bit that these lines might not work as expected:

   USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'virtualization', 'meta-virt-cfg', '', d)}"
   USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'k8s', 'meta-virt-k8s-cfg', '', d)}"

They do work as expected. See the post from Mark Hatle (to the mailing
list) explaining the details of why they are setup as they are.

This is one of the only ways to ensure that something which is
parsed/expanded early enough can then be used in the layer conf to do
version pinning like this.

That being said, what part of those lines do you think might not be working ?

If virtualization is set in local.conf (which is the standard I would say), it will not be set when the layer.conf is parsed.

This could be solve by doing the test in the class directly (using a python function instead).

Bertrand



Bruce


Perhaps these should become unconditional appends to USER_CLASSES and
the DISTRO_FEATURES checks should be done inside those classes.

Bruce, any thoughts on this one for meta-virtualization?


Here are the tests I did to end up with those conclusions:
- "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
 So the file is not found here.
- “require conf/distro/include/meta-virt-default-versions.inc” build correctly
 So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
 So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
 So the file is not included here

Is there a right way to achieve what is want in layer.conf ?

You need to delay parsing of your variable until after bitbake.conf is
parsed (which includes local.conf and other related files). The
simplest way to do this is to place the logic in a bbclass and add
this to the INHERIT variable in your layer.conf. The classes in
INHERIT are picked up as if they were at the start of each recipe that
is parsed.

Thanks,
Paul



--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II


[-- Attachment #2: Type: text/html, Size: 17966 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 14:23     ` Bertrand Marquis
@ 2020-02-10 14:35       ` Bruce Ashfield
  2020-02-10 14:45         ` Bertrand Marquis
  0 siblings, 1 reply; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 14:35 UTC (permalink / raw)
  To: Bertrand Marquis; +Cc: meta-virtualization, Paul Barker, nd

On Mon, Feb 10, 2020 at 9:23 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
>
>
> On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com> wrote:
>
>
> On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
>
>
> Hi Everyone,
>
> Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
>
> After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
>
>
> This is correct - layer.conf in each layer is parsed before local.conf.
>
>
> Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
>
> Could anyone confirm or explain me where I am wrong here ?
>
>
> Looking at layer.conf in meta-virtualization on the master branch, it
> does worry me a bit that these lines might not work as expected:
>
>    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'virtualization', 'meta-virt-cfg', '', d)}"
>    USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'k8s', 'meta-virt-k8s-cfg', '', d)}"
>
>
> They do work as expected. See the post from Mark Hatle (to the mailing
> list) explaining the details of why they are setup as they are.
>
> This is one of the only ways to ensure that something which is
> parsed/expanded early enough can then be used in the layer conf to do
> version pinning like this.
>
> That being said, what part of those lines do you think might not be working ?
>
>
> If virtualization is set in local.conf (which is the standard  would say), it will not be set when the layer.conf is parsed.

No, a distro conf is actually what is expected. With sometimes
temporary testing with local.conf

>

That being said, I've run many tests with it in local.conf, and I'm
seeing exactly what I expect. The version files are either included,
or not, based on the distro feature.  I can easily show that by
putting a syntax error in the file, and making it either break the
build, or not, based on the distro feature.

Are you not seeing the file included at all, or just that the
preferred versions aren't kicking in ?

 ... again, check the mailing list archives and you'll see the
technical explanation of how and why it is put together as it is.

Cheers,

Bruce

> This could be solve by doing the test in the class directly (using a python function instead).
>
> Bertrand
>
>
>
> Bruce
>
>
> Perhaps these should become unconditional appends to USER_CLASSES and
> the DISTRO_FEATURES checks should be done inside those classes.
>
> Bruce, any thoughts on this one for meta-virtualization?
>
>
> Here are the tests I did to end up with those conclusions:
> - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
>  So the file is not found here.
> - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
>  So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
>  So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
>  So the file is not included here
>
> Is there a right way to achieve what is want in layer.conf ?
>
>
> You need to delay parsing of your variable until after bitbake.conf is
> parsed (which includes local.conf and other related files). The
> simplest way to do this is to place the logic in a bbclass and add
> this to the INHERIT variable in your layer.conf. The classes in
> INHERIT are picked up as if they were at the start of each recipe that
> is parsed.
>
> Thanks,
> Paul
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>


-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10  8:05 ` [meta-virtualization] " Paul Barker
  2020-02-10 11:12   ` Bertrand Marquis
  2020-02-10 14:17   ` Bruce Ashfield
@ 2020-02-10 14:41   ` Bruce Ashfield
  2 siblings, 0 replies; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 14:41 UTC (permalink / raw)
  To: Paul Barker; +Cc: Bertrand Marquis, meta-virtualization, nd

On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com> wrote:
>
> On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
> >
> > Hi Everyone,
> >
> > Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
> >
> > After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
>
> This is correct - layer.conf in each layer is parsed before local.conf.
>
> >
> > Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> > - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> > - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
> >
> > Could anyone confirm or explain me where I am wrong here ?
>
> Looking at layer.conf in meta-virtualization on the master branch, it
> does worry me a bit that these lines might not work as expected:
>
>     USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'virtualization', 'meta-virt-cfg', '', d)}"
>     USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'k8s', 'meta-virt-k8s-cfg', '', d)}"
>
> Perhaps these should become unconditional appends to USER_CLASSES and
> the DISTRO_FEATURES checks should be done inside those classes.

FWIW: that does work as well, just when we were testing this about a
year ago, it wasn't required to hide the include in the class. That is
the design, use the USER_CLASS loading, to defer the distro feature
check to a point where it is resolved.

If I can reproduce the bad behaviour here, I can definitely shuffle
things to the included file themselves.

Bruce

>
> Bruce, any thoughts on this one for meta-virtualization?
>
> >
> > Here are the tests I did to end up with those conclusions:
> > - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
> >   So the file is not found here.
> > - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
> >   So the file is properly included here.
> > - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
> >   So the file is properly included here.
> > - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
> >   So the file is not included here
> >
> > Is there a right way to achieve what is want in layer.conf ?
>
> You need to delay parsing of your variable until after bitbake.conf is
> parsed (which includes local.conf and other related files). The
> simplest way to do this is to place the logic in a bbclass and add
> this to the INHERIT variable in your layer.conf. The classes in
> INHERIT are picked up as if they were at the start of each recipe that
> is parsed.
>
> Thanks,
> Paul



-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 14:35       ` Bruce Ashfield
@ 2020-02-10 14:45         ` Bertrand Marquis
  2020-02-10 14:48           ` Bruce Ashfield
       [not found]           ` <15F211E71CA8CDBB.3625@lists.yoctoproject.org>
  0 siblings, 2 replies; 13+ messages in thread
From: Bertrand Marquis @ 2020-02-10 14:45 UTC (permalink / raw)
  To: Bruce Ashfield, meta-virtualization; +Cc: Paul Barker, nd

[-- Attachment #1: Type: text/plain, Size: 5220 bytes --]



On 10 Feb 2020, at 14:35, Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>> wrote:

On Mon, Feb 10, 2020 at 9:23 AM Bertrand Marquis
<Bertrand.Marquis@arm.com<mailto:Bertrand.Marquis@arm.com>> wrote:



On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>> wrote:

On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com<mailto:pbarker@konsulko.com>> wrote:


On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com<mailto:bertrand.marquis@arm.com>> wrote:


Hi Everyone,

Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.

After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.


This is correct - layer.conf in each layer is parsed before local.conf.


Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
- using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
- even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf

Could anyone confirm or explain me where I am wrong here ?


Looking at layer.conf in meta-virtualization on the master branch, it
does worry me a bit that these lines might not work as expected:

  USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'virtualization', 'meta-virt-cfg', '', d)}"
  USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'k8s', 'meta-virt-k8s-cfg', '', d)}"


They do work as expected. See the post from Mark Hatle (to the mailing
list) explaining the details of why they are setup as they are.

This is one of the only ways to ensure that something which is
parsed/expanded early enough can then be used in the layer conf to do
version pinning like this.

That being said, what part of those lines do you think might not be working ?


If virtualization is set in local.conf (which is the standard  would say), it will not be set when the layer.conf is parsed.

No, a distro conf is actually what is expected. With sometimes
temporary testing with local.conf



That being said, I've run many tests with it in local.conf, and I'm
seeing exactly what I expect. The version files are either included,
or not, based on the distro feature.  I can easily show that by
putting a syntax error in the file, and making it either break the
build, or not, based on the distro feature.

Are you not seeing the file included at all, or just that the
preferred versions aren't kicking in ?

In Zeus definitely not, whatever I put as error in the .inc file everything goes well.

On master that works (adding an error in one inc file creates an error if virtualization is activated in local.conf)



... again, check the mailing list archives and you'll see the
technical explanation of how and why it is put together as it is.

I did and I agree that current behaviour in master is what is wanted. Was only not in my current development in Zeus.

Cheers,
Bertrand


Cheers,

Bruce

This could be solve by doing the test in the class directly (using a python function instead).

Bertrand



Bruce


Perhaps these should become unconditional appends to USER_CLASSES and
the DISTRO_FEATURES checks should be done inside those classes.

Bruce, any thoughts on this one for meta-virtualization?


Here are the tests I did to end up with those conclusions:
- "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
So the file is not found here.
- “require conf/distro/include/meta-virt-default-versions.inc” build correctly
So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
So the file is not included here

Is there a right way to achieve what is want in layer.conf ?


You need to delay parsing of your variable until after bitbake.conf is
parsed (which includes local.conf and other related files). The
simplest way to do this is to place the logic in a bbclass and add
this to the INHERIT variable in your layer.conf. The classes in
INHERIT are picked up as if they were at the start of each recipe that
is parsed.

Thanks,
Paul




--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II




--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II


[-- Attachment #2: Type: text/html, Size: 26167 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 14:45         ` Bertrand Marquis
@ 2020-02-10 14:48           ` Bruce Ashfield
       [not found]           ` <15F211E71CA8CDBB.3625@lists.yoctoproject.org>
  1 sibling, 0 replies; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 14:48 UTC (permalink / raw)
  To: Bertrand Marquis; +Cc: meta-virtualization, Paul Barker, nd

On Mon, Feb 10, 2020 at 9:45 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
>
>
> On 10 Feb 2020, at 14:35, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 9:23 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>
>
>
>
> On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com> wrote:
>
>
> On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
>
>
> Hi Everyone,
>
> Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
>
> After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
>
>
> This is correct - layer.conf in each layer is parsed before local.conf.
>
>
> Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
>
> Could anyone confirm or explain me where I am wrong here ?
>
>
> Looking at layer.conf in meta-virtualization on the master branch, it
> does worry me a bit that these lines might not work as expected:
>
>   USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'virtualization', 'meta-virt-cfg', '', d)}"
>   USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'k8s', 'meta-virt-k8s-cfg', '', d)}"
>
>
> They do work as expected. See the post from Mark Hatle (to the mailing
> list) explaining the details of why they are setup as they are.
>
> This is one of the only ways to ensure that something which is
> parsed/expanded early enough can then be used in the layer conf to do
> version pinning like this.
>
> That being said, what part of those lines do you think might not be working ?
>
>
> If virtualization is set in local.conf (which is the standard  would say), it will not be set when the layer.conf is parsed.
>
>
> No, a distro conf is actually what is expected. With sometimes
> temporary testing with local.conf
>
>
>
> That being said, I've run many tests with it in local.conf, and I'm
> seeing exactly what I expect. The version files are either included,
> or not, based on the distro feature.  I can easily show that by
> putting a syntax error in the file, and making it either break the
> build, or not, based on the distro feature.
>
> Are you not seeing the file included at all, or just that the
> preferred versions aren't kicking in ?
>
>
> In Zeus definitely not, whatever I put as error in the .inc file everything goes well.
>
> On master that works (adding an error in one inc file creates an error if virtualization is activated in local.conf)
>
>
>
> ... again, check the mailing list archives and you'll see the
> technical explanation of how and why it is put together as it is.
>
>
> I did and I agree that current behaviour in master is what is wanted. Was only not in my current development in Zeus.

Aha. So that's the difference,

I'll spin a patch for Zeus that moves the distro check to the included
file, versus the .conf.

Bruce

>
> Cheers,
> Bertrand
>
>
> Cheers,
>
> Bruce
>
> This could be solve by doing the test in the class directly (using a python function instead).
>
> Bertrand
>
>
>
> Bruce
>
>
> Perhaps these should become unconditional appends to USER_CLASSES and
> the DISTRO_FEATURES checks should be done inside those classes.
>
> Bruce, any thoughts on this one for meta-virtualization?
>
>
> Here are the tests I did to end up with those conclusions:
> - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
> So the file is not found here.
> - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
> So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
> So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
> So the file is not included here
>
> Is there a right way to achieve what is want in layer.conf ?
>
>
> You need to delay parsing of your variable until after bitbake.conf is
> parsed (which includes local.conf and other related files). The
> simplest way to do this is to place the logic in a bbclass and add
> this to the INHERIT variable in your layer.conf. The classes in
> INHERIT are picked up as if they were at the start of each recipe that
> is parsed.
>
> Thanks,
> Paul
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>


-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
       [not found]           ` <15F211E71CA8CDBB.3625@lists.yoctoproject.org>
@ 2020-02-10 15:33             ` Bruce Ashfield
  2020-02-10 15:59               ` Bertrand Marquis
  0 siblings, 1 reply; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 15:33 UTC (permalink / raw)
  To: Bruce Ashfield; +Cc: Bertrand Marquis, meta-virtualization

[-- Attachment #1: Type: text/plain, Size: 6540 bytes --]

Attached is a (lightly) tested patch, which defers the distro feature
check to the
bbclasses, versus the .conf file.

This worked on master, and hopefully it works on Zeus as well (I'll
test Zeus later today, but can't switch at the moment).

Bruce

On Mon, Feb 10, 2020 at 9:48 AM Bruce Ashfield via
Lists.Yoctoproject.Org
<bruce.ashfield=gmail.com@lists.yoctoproject.org> wrote:
>
> On Mon, Feb 10, 2020 at 9:45 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
> >
> >
> >
> > On 10 Feb 2020, at 14:35, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
> >
> > On Mon, Feb 10, 2020 at 9:23 AM Bertrand Marquis
> > <Bertrand.Marquis@arm.com> wrote:
> >
> >
> >
> >
> > On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
> >
> > On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com> wrote:
> >
> >
> > On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
> >
> >
> > Hi Everyone,
> >
> > Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
> >
> > After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
> >
> >
> > This is correct - layer.conf in each layer is parsed before local.conf.
> >
> >
> > Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> > - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> > - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
> >
> > Could anyone confirm or explain me where I am wrong here ?
> >
> >
> > Looking at layer.conf in meta-virtualization on the master branch, it
> > does worry me a bit that these lines might not work as expected:
> >
> >   USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> > 'virtualization', 'meta-virt-cfg', '', d)}"
> >   USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> > 'k8s', 'meta-virt-k8s-cfg', '', d)}"
> >
> >
> > They do work as expected. See the post from Mark Hatle (to the mailing
> > list) explaining the details of why they are setup as they are.
> >
> > This is one of the only ways to ensure that something which is
> > parsed/expanded early enough can then be used in the layer conf to do
> > version pinning like this.
> >
> > That being said, what part of those lines do you think might not be working ?
> >
> >
> > If virtualization is set in local.conf (which is the standard  would say), it will not be set when the layer.conf is parsed.
> >
> >
> > No, a distro conf is actually what is expected. With sometimes
> > temporary testing with local.conf
> >
> >
> >
> > That being said, I've run many tests with it in local.conf, and I'm
> > seeing exactly what I expect. The version files are either included,
> > or not, based on the distro feature.  I can easily show that by
> > putting a syntax error in the file, and making it either break the
> > build, or not, based on the distro feature.
> >
> > Are you not seeing the file included at all, or just that the
> > preferred versions aren't kicking in ?
> >
> >
> > In Zeus definitely not, whatever I put as error in the .inc file everything goes well.
> >
> > On master that works (adding an error in one inc file creates an error if virtualization is activated in local.conf)
> >
> >
> >
> > ... again, check the mailing list archives and you'll see the
> > technical explanation of how and why it is put together as it is.
> >
> >
> > I did and I agree that current behaviour in master is what is wanted. Was only not in my current development in Zeus.
>
> Aha. So that's the difference,
>
> I'll spin a patch for Zeus that moves the distro check to the included
> file, versus the .conf.
>
> Bruce
>
> >
> > Cheers,
> > Bertrand
> >
> >
> > Cheers,
> >
> > Bruce
> >
> > This could be solve by doing the test in the class directly (using a python function instead).
> >
> > Bertrand
> >
> >
> >
> > Bruce
> >
> >
> > Perhaps these should become unconditional appends to USER_CLASSES and
> > the DISTRO_FEATURES checks should be done inside those classes.
> >
> > Bruce, any thoughts on this one for meta-virtualization?
> >
> >
> > Here are the tests I did to end up with those conclusions:
> > - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
> > So the file is not found here.
> > - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
> > So the file is properly included here.
> > - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
> > So the file is properly included here.
> > - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
> > So the file is not included here
> >
> > Is there a right way to achieve what is want in layer.conf ?
> >
> >
> > You need to delay parsing of your variable until after bitbake.conf is
> > parsed (which includes local.conf and other related files). The
> > simplest way to do this is to place the logic in a bbclass and add
> > this to the INHERIT variable in your layer.conf. The classes in
> > INHERIT are picked up as if they were at the start of each recipe that
> > is parsed.
> >
> > Thanks,
> > Paul
> >
> >
> >
> >
> > --
> > - Thou shalt not follow the NULL pointer, for chaos and madness await
> > thee at its end
> > - "Use the force Harry" - Gandalf, Star Trek II
> >
> >
> >
> >
> > --
> > - Thou shalt not follow the NULL pointer, for chaos and madness await
> > thee at its end
> > - "Use the force Harry" - Gandalf, Star Trek II
> >
> >
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
> 



-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

[-- Attachment #2: 0001-conf-defer-DISTRO_FEATURE-check-to-bbclass-processin.patch --]
[-- Type: application/octet-stream, Size: 2598 bytes --]

From 3a7d446fa94787eda0bcc2f3759e07db5bda6b2f Mon Sep 17 00:00:00 2001
From: Bruce Ashfield <bruce.ashfield@gmail.com>
Date: Mon, 10 Feb 2020 10:27:56 -0500
Subject: [PATCH] conf: defer DISTRO_FEATURE check to bbclass processing

We were using USER_CLASS loading to allow conditional checking
on DISTRO_FEATURES, which triggered distro feature specific version
pinning.

It was found that DISTRO_FEATURES set in local.conf is not
consistently available at layer.conf parse time, hence our checks
were not always working as expected (i.e. the version files are
not included).

If we move the DISTRO_FEATURE check to the bbclasses, and use it
to trigger the include, we should have a consistent set of variable
resolution and consistent behaviour.

Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>
---
 classes/meta-virt-cfg.bbclass     | 2 +-
 classes/meta-virt-k8s-cfg.bbclass | 2 +-
 conf/layer.conf                   | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/classes/meta-virt-cfg.bbclass b/classes/meta-virt-cfg.bbclass
index bf63a06..a20f9fe 100644
--- a/classes/meta-virt-cfg.bbclass
+++ b/classes/meta-virt-cfg.bbclass
@@ -3,4 +3,4 @@
 # layer.conf load time, we delay using a special bbclass that simply includes
 # the META_VIRT_CONFIG_PATH file.
 
-include ${META_VIRT_CONFIG_PATH}
\ No newline at end of file
+include ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', '${META_VIRT_CONFIG_PATH}', '', d)}
diff --git a/classes/meta-virt-k8s-cfg.bbclass b/classes/meta-virt-k8s-cfg.bbclass
index 1be0a5c..f1d7f81 100644
--- a/classes/meta-virt-k8s-cfg.bbclass
+++ b/classes/meta-virt-k8s-cfg.bbclass
@@ -3,4 +3,4 @@
 # layer.conf load time, we delay using a special bbclass that simply includes
 # the K8S_CONFIG_PATH file.
 
-include ${K8S_CONFIG_PATH}
+include ${@bb.utils.contains('DISTRO_FEATURES', 'k8s', '${K8S_CONFIG_PATH}', '', d)}
diff --git a/conf/layer.conf b/conf/layer.conf
index d0a7ead..922cb0d 100644
--- a/conf/layer.conf
+++ b/conf/layer.conf
@@ -46,5 +46,5 @@ INHERIT += "sanity-meta-virt"
 # the META_VIRT_CONFIG_PATH file, and likewise for the k8s configs
 META_VIRT_CONFIG_PATH = "${LAYERDIR}/conf/distro/include/meta-virt-default-versions.inc"
 K8S_CONFIG_PATH = "${LAYERDIR}/conf/distro/include/k8s-versions.inc"
-USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-cfg', '', d)}"
-USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES', 'k8s', 'meta-virt-k8s-cfg', '', d)}"
+USER_CLASSES_append = " meta-virt-cfg"
+USER_CLASSES_append = " meta-virt-k8s-cfg"
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 15:33             ` Bruce Ashfield
@ 2020-02-10 15:59               ` Bertrand Marquis
  2020-02-10 18:51                 ` Bruce Ashfield
  0 siblings, 1 reply; 13+ messages in thread
From: Bertrand Marquis @ 2020-02-10 15:59 UTC (permalink / raw)
  To: Bruce Ashfield; +Cc: meta-virtualization

[-- Attachment #1: Type: text/plain, Size: 6990 bytes --]



On 10 Feb 2020, at 15:33, Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>> wrote:

Attached is a (lightly) tested patch, which defers the distro feature
check to the
bbclasses, versus the .conf file.

This worked on master, and hopefully it works on Zeus as well (I'll
test Zeus later today, but can't switch at the moment).

Your patch does not apply on Zeus.
For internal reasons I cannot push one myself right now but if you wait the end of this week I can handle this an push a patch to fix this on the mailing list.

Bertrand


Bruce

On Mon, Feb 10, 2020 at 9:48 AM Bruce Ashfield via
Lists.Yoctoproject.Org<http://lists.yoctoproject.org/>
<bruce.ashfield=gmail.com@lists.yoctoproject.org<mailto:bruce.ashfield=gmail.com@lists.yoctoproject.org>> wrote:

On Mon, Feb 10, 2020 at 9:45 AM Bertrand Marquis
<Bertrand.Marquis@arm.com<mailto:Bertrand.Marquis@arm.com>> wrote:



On 10 Feb 2020, at 14:35, Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>> wrote:

On Mon, Feb 10, 2020 at 9:23 AM Bertrand Marquis
<Bertrand.Marquis@arm.com<mailto:Bertrand.Marquis@arm.com>> wrote:




On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com<mailto:bruce.ashfield@gmail.com>> wrote:

On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com<mailto:pbarker@konsulko.com>> wrote:


On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com<mailto:bertrand.marquis@arm.com>> wrote:


Hi Everyone,

Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.

After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.


This is correct - layer.conf in each layer is parsed before local.conf.


Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
- using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
- even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf

Could anyone confirm or explain me where I am wrong here ?


Looking at layer.conf in meta-virtualization on the master branch, it
does worry me a bit that these lines might not work as expected:

 USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'virtualization', 'meta-virt-cfg', '', d)}"
 USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
'k8s', 'meta-virt-k8s-cfg', '', d)}"


They do work as expected. See the post from Mark Hatle (to the mailing
list) explaining the details of why they are setup as they are.

This is one of the only ways to ensure that something which is
parsed/expanded early enough can then be used in the layer conf to do
version pinning like this.

That being said, what part of those lines do you think might not be working ?


If virtualization is set in local.conf (which is the standard  would say), it will not be set when the layer.conf is parsed.


No, a distro conf is actually what is expected. With sometimes
temporary testing with local.conf



That being said, I've run many tests with it in local.conf, and I'm
seeing exactly what I expect. The version files are either included,
or not, based on the distro feature.  I can easily show that by
putting a syntax error in the file, and making it either break the
build, or not, based on the distro feature.

Are you not seeing the file included at all, or just that the
preferred versions aren't kicking in ?


In Zeus definitely not, whatever I put as error in the .inc file everything goes well.

On master that works (adding an error in one inc file creates an error if virtualization is activated in local.conf)



... again, check the mailing list archives and you'll see the
technical explanation of how and why it is put together as it is.


I did and I agree that current behaviour in master is what is wanted. Was only not in my current development in Zeus.

Aha. So that's the difference,

I'll spin a patch for Zeus that moves the distro check to the included
file, versus the .conf.

Bruce


Cheers,
Bertrand


Cheers,

Bruce

This could be solve by doing the test in the class directly (using a python function instead).

Bertrand



Bruce


Perhaps these should become unconditional appends to USER_CLASSES and
the DISTRO_FEATURES checks should be done inside those classes.

Bruce, any thoughts on this one for meta-virtualization?


Here are the tests I did to end up with those conclusions:
- "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
So the file is not found here.
- “require conf/distro/include/meta-virt-default-versions.inc” build correctly
So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
So the file is properly included here.
- adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
So the file is not included here

Is there a right way to achieve what is want in layer.conf ?


You need to delay parsing of your variable until after bitbake.conf is
parsed (which includes local.conf and other related files). The
simplest way to do this is to place the logic in a bbclass and add
this to the INHERIT variable in your layer.conf. The classes in
INHERIT are picked up as if they were at the start of each recipe that
is parsed.

Thanks,
Paul




--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II




--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II




--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II




--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II
<0001-conf-defer-DISTRO_FEATURE-check-to-bbclass-processin.patch>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

[-- Attachment #2: Type: text/html, Size: 22715 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [meta-virtualization] Layer.conf require of meta-virt-default-versions.inc
  2020-02-10 15:59               ` Bertrand Marquis
@ 2020-02-10 18:51                 ` Bruce Ashfield
  0 siblings, 0 replies; 13+ messages in thread
From: Bruce Ashfield @ 2020-02-10 18:51 UTC (permalink / raw)
  To: Bertrand Marquis; +Cc: meta-virtualization

On Mon, Feb 10, 2020 at 10:59 AM Bertrand Marquis
<Bertrand.Marquis@arm.com> wrote:
>
>
>
> On 10 Feb 2020, at 15:33, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> Attached is a (lightly) tested patch, which defers the distro feature
> check to the
> bbclasses, versus the .conf file.
>
> This worked on master, and hopefully it works on Zeus as well (I'll
> test Zeus later today, but can't switch at the moment).
>
>
> Your patch does not apply on Zeus.
> For internal reasons I cannot push one myself right now but if you wait the end of this week I can handle this an push a patch to fix this on the mailing list.
>

I'll do the port to Zeus eventually, everything just lands in master
first, hence why I sent that variant.

Bruce

> Bertrand
>
>
> Bruce
>
> On Mon, Feb 10, 2020 at 9:48 AM Bruce Ashfield via
> Lists.Yoctoproject.Org
> <bruce.ashfield=gmail.com@lists.yoctoproject.org> wrote:
>
>
> On Mon, Feb 10, 2020 at 9:45 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>
>
>
>
> On 10 Feb 2020, at 14:35, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 9:23 AM Bertrand Marquis
> <Bertrand.Marquis@arm.com> wrote:
>
>
>
>
> On 10 Feb 2020, at 14:17, Bruce Ashfield <bruce.ashfield@gmail.com> wrote:
>
> On Mon, Feb 10, 2020 at 3:06 AM Paul Barker <pbarker@konsulko.com> wrote:
>
>
> On Fri, 7 Feb 2020 at 15:42, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
>
>
> Hi Everyone,
>
> Trying to design my own layer I used the example of meta-virtualization layer configuration to try to include a file depending on a DISTRO_FEATURES value.
>
> After several tests, I discovered that all the values that I did defined in the included file have no effect, in fact I get the feeling that DISTRO_FEATURES does not contain the entries I add in local.conf when layer.conf is parsed.
>
>
> This is correct - layer.conf in each layer is parsed before local.conf.
>
>
> Meta-virtualization is using this system to set PREFERRED_VERSION for some packages but it seems that:
> - using require without a full path does not work (maybe the require should be changed to conf/distro/include/meta-virt-default-versions.inc)
> - even with the right path using bb.utils.contains does not work if DISTRO_FEATURES is set in local.conf
>
> Could anyone confirm or explain me where I am wrong here ?
>
>
> Looking at layer.conf in meta-virtualization on the master branch, it
> does worry me a bit that these lines might not work as expected:
>
>  USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'virtualization', 'meta-virt-cfg', '', d)}"
>  USER_CLASSES_append = " ${@bb.utils.contains('DISTRO_FEATURES',
> 'k8s', 'meta-virt-k8s-cfg', '', d)}"
>
>
> They do work as expected. See the post from Mark Hatle (to the mailing
> list) explaining the details of why they are setup as they are.
>
> This is one of the only ways to ensure that something which is
> parsed/expanded early enough can then be used in the layer conf to do
> version pinning like this.
>
> That being said, what part of those lines do you think might not be working ?
>
>
> If virtualization is set in local.conf (which is the standard  would say), it will not be set when the layer.conf is parsed.
>
>
> No, a distro conf is actually what is expected. With sometimes
> temporary testing with local.conf
>
>
>
> That being said, I've run many tests with it in local.conf, and I'm
> seeing exactly what I expect. The version files are either included,
> or not, based on the distro feature.  I can easily show that by
> putting a syntax error in the file, and making it either break the
> build, or not, based on the distro feature.
>
> Are you not seeing the file included at all, or just that the
> preferred versions aren't kicking in ?
>
>
> In Zeus definitely not, whatever I put as error in the .inc file everything goes well.
>
> On master that works (adding an error in one inc file creates an error if virtualization is activated in local.conf)
>
>
>
> ... again, check the mailing list archives and you'll see the
> technical explanation of how and why it is put together as it is.
>
>
> I did and I agree that current behaviour in master is what is wanted. Was only not in my current development in Zeus.
>
>
> Aha. So that's the difference,
>
> I'll spin a patch for Zeus that moves the distro check to the included
> file, versus the .conf.
>
> Bruce
>
>
> Cheers,
> Bertrand
>
>
> Cheers,
>
> Bruce
>
> This could be solve by doing the test in the class directly (using a python function instead).
>
> Bertrand
>
>
>
> Bruce
>
>
> Perhaps these should become unconditional appends to USER_CLASSES and
> the DISTRO_FEATURES checks should be done inside those classes.
>
> Bruce, any thoughts on this one for meta-virtualization?
>
>
> Here are the tests I did to end up with those conclusions:
> - "require meta-virt-default-versions.inc” directly in layer.conf is ending with a bitbake error
> So the file is not found here.
> - “require conf/distro/include/meta-virt-default-versions.inc” build correctly
> So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require conf/distro/include/meta-virt-default-versions.inc” in layer.conf end up in an error
> So the file is properly included here.
> - adding “TEST=" “ in  meta-virt-default-versions.inc and “require ${@bb.utils.contains('DISTRO_FEATURES', 'virtualization', 'meta-virt-default-versions.inc', '', d)}” in layer.conf build correctly
> So the file is not included here
>
> Is there a right way to achieve what is want in layer.conf ?
>
>
> You need to delay parsing of your variable until after bitbake.conf is
> parsed (which includes local.conf and other related files). The
> simplest way to do this is to place the logic in a bbclass and add
> this to the INHERIT variable in your layer.conf. The classes in
> INHERIT are picked up as if they were at the start of each recipe that
> is parsed.
>
> Thanks,
> Paul
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
> 
>
>
>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await
> thee at its end
> - "Use the force Harry" - Gandalf, Star Trek II
> <0001-conf-defer-DISTRO_FEATURE-check-to-bbclass-processin.patch>
>
>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.



-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-02-10 18:51 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07 15:42 Layer.conf require of meta-virt-default-versions.inc Bertrand Marquis
2020-02-10  8:05 ` [meta-virtualization] " Paul Barker
2020-02-10 11:12   ` Bertrand Marquis
2020-02-10 14:19     ` Bruce Ashfield
2020-02-10 14:17   ` Bruce Ashfield
2020-02-10 14:23     ` Bertrand Marquis
2020-02-10 14:35       ` Bruce Ashfield
2020-02-10 14:45         ` Bertrand Marquis
2020-02-10 14:48           ` Bruce Ashfield
     [not found]           ` <15F211E71CA8CDBB.3625@lists.yoctoproject.org>
2020-02-10 15:33             ` Bruce Ashfield
2020-02-10 15:59               ` Bertrand Marquis
2020-02-10 18:51                 ` Bruce Ashfield
2020-02-10 14:41   ` Bruce Ashfield

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.