diff mbox series

kernel-yocto: use oe.types.boolean on KMETA_AUDIT

Message ID 20241216-kmeta-audit-vartrue-v1-1-51bbb0b2085a@bootlin.com
State New
Headers show
Series kernel-yocto: use oe.types.boolean on KMETA_AUDIT | expand

Commit Message

Antonin Godard Dec. 16, 2024, 11:35 a.m. UTC
Following commit a39a1f7cf78ad1ca07438bce634a47e970f25047
("kernel-yocto: allow early exit to configuration audit"), we can
disable auditing of the kernel configuration by setting KMETA_AUDIT
empty.

Since the default value of this variables is "yes", use
oe.types.boolean() so that it will also be disabled when setting it to
"no", and avoid potential confusion.

Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
---
 meta/classes-recipe/kernel-yocto.bbclass | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


---
base-commit: e345b34703d4fa5e0bc9a82ac33b7c1fd84f99fe
change-id: 20241216-kmeta-audit-vartrue-6650cb20f762

Best regards,

Comments

Bruce Ashfield Dec. 16, 2024, 1:21 p.m. UTC | #1
On Mon, Dec 16, 2024 at 6:35 AM Antonin Godard <antonin.godard@bootlin.com>
wrote:

> Following commit a39a1f7cf78ad1ca07438bce634a47e970f25047
> ("kernel-yocto: allow early exit to configuration audit"), we can
> disable auditing of the kernel configuration by setting KMETA_AUDIT
> empty.
>
> Since the default value of this variables is "yes", use
> oe.types.boolean() so that it will also be disabled when setting it to
> "no", and avoid potential confusion.
>

That isn't the intent of the variable. It is being used as a flag here,
but it can also be used to encode other information.

Quite literally, if it is set to anything, the audit should happen. Only
when it is unset should it not.

Bruce


>
> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
> ---
>  meta/classes-recipe/kernel-yocto.bbclass | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/meta/classes-recipe/kernel-yocto.bbclass
> b/meta/classes-recipe/kernel-yocto.bbclass
> index
> a5d89dc2c8bc912539f7c9ffc25a2576920f580a..1493ba20a0c25df556df41fa4137ab17f67c91e7
> 100644
> --- a/meta/classes-recipe/kernel-yocto.bbclass
> +++ b/meta/classes-recipe/kernel-yocto.bbclass
> @@ -568,7 +568,9 @@ python do_config_analysis() {
>  python do_kernel_configcheck() {
>      import re, string, sys, subprocess
>
> -    audit_flag = d.getVar( "KMETA_AUDIT" )
> +    audit_flag = False
> +    if d.getVar("KMETA_AUDIT"):
> +        audit_flag = oe.types.boolean(d.getVar("KMETA_AUDIT"))
>      if not audit_flag:
>         bb.note( "kernel config audit disabled, skipping .." )
>         return
>
> ---
> base-commit: e345b34703d4fa5e0bc9a82ac33b7c1fd84f99fe
> change-id: 20241216-kmeta-audit-vartrue-6650cb20f762
>
> Best regards,
> --
> Antonin Godard <antonin.godard@bootlin.com>
>
>
Bruce Ashfield Dec. 16, 2024, 1:29 p.m. UTC | #2
On Mon, Dec 16, 2024 at 8:21 AM Bruce Ashfield <bruce.ashfield@gmail.com>
wrote:

>
>
> On Mon, Dec 16, 2024 at 6:35 AM Antonin Godard <antonin.godard@bootlin.com>
> wrote:
>
>> Following commit a39a1f7cf78ad1ca07438bce634a47e970f25047
>> ("kernel-yocto: allow early exit to configuration audit"), we can
>> disable auditing of the kernel configuration by setting KMETA_AUDIT
>> empty.
>>
>> Since the default value of this variables is "yes", use
>> oe.types.boolean() so that it will also be disabled when setting it to
>> "no", and avoid potential confusion.
>>
>
> That isn't the intent of the variable. It is being used as a flag here,
> but it can also be used to encode other information.
>
> Quite literally, if it is set to anything, the audit should happen. Only
> when it is unset should it not.
>

I hit send too soon. It's Monday.

What I wanted to add was this question: Is that still the behaviour with
the boolean call ? If so, no concerns from me.

Bruce



>
> Bruce
>
>
>>
>> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
>> ---
>>  meta/classes-recipe/kernel-yocto.bbclass | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes-recipe/kernel-yocto.bbclass
>> b/meta/classes-recipe/kernel-yocto.bbclass
>> index
>> a5d89dc2c8bc912539f7c9ffc25a2576920f580a..1493ba20a0c25df556df41fa4137ab17f67c91e7
>> 100644
>> --- a/meta/classes-recipe/kernel-yocto.bbclass
>> +++ b/meta/classes-recipe/kernel-yocto.bbclass
>> @@ -568,7 +568,9 @@ python do_config_analysis() {
>>  python do_kernel_configcheck() {
>>      import re, string, sys, subprocess
>>
>> -    audit_flag = d.getVar( "KMETA_AUDIT" )
>> +    audit_flag = False
>> +    if d.getVar("KMETA_AUDIT"):
>> +        audit_flag = oe.types.boolean(d.getVar("KMETA_AUDIT"))
>>      if not audit_flag:
>>         bb.note( "kernel config audit disabled, skipping .." )
>>         return
>>
>> ---
>> base-commit: e345b34703d4fa5e0bc9a82ac33b7c1fd84f99fe
>> change-id: 20241216-kmeta-audit-vartrue-6650cb20f762
>>
>> Best regards,
>> --
>> Antonin Godard <antonin.godard@bootlin.com>
>>
>>
>
> --
> - Thou shalt not follow the NULL pointer, for chaos and madness await thee
> at its end
> - "Use the force Harry" - Gandalf, Star Trek II
>
>
Antonin Godard Dec. 16, 2024, 1:50 p.m. UTC | #3
Hi Bruce,

On Mon Dec 16, 2024 at 2:29 PM CET, Bruce Ashfield via lists.openembedded.org wrote:
> On Mon, Dec 16, 2024 at 8:21 AM Bruce Ashfield <bruce.ashfield@gmail.com>
> wrote:
>
>>
>>
>> On Mon, Dec 16, 2024 at 6:35 AM Antonin Godard <antonin.godard@bootlin.com>
>> wrote:
>>
>>> Following commit a39a1f7cf78ad1ca07438bce634a47e970f25047
>>> ("kernel-yocto: allow early exit to configuration audit"), we can
>>> disable auditing of the kernel configuration by setting KMETA_AUDIT
>>> empty.
>>>
>>> Since the default value of this variables is "yes", use
>>> oe.types.boolean() so that it will also be disabled when setting it to
>>> "no", and avoid potential confusion.
>>>
>>
>> That isn't the intent of the variable. It is being used as a flag here,
>> but it can also be used to encode other information.
>>
>> Quite literally, if it is set to anything, the audit should happen. Only
>> when it is unset should it not.
>>
>
> I hit send too soon. It's Monday.
>
> What I wanted to add was this question: Is that still the behaviour with
> the boolean call ? If so, no concerns from me.

It seems that this is the only place where the KMETA_AUDIT variable is used so I
figured its role was to enable/disable the audit and only that. Maybe the
"audit_flag" name is a bit misleading then?

The behavior of the boolean call is to return False if KMETA_AUDIT == "no", so
it _is_ different from "if set to anything, the audit happens".


Antonin
Bruce Ashfield Dec. 16, 2024, 1:56 p.m. UTC | #4
On Mon, Dec 16, 2024 at 8:50 AM Antonin Godard <antonin.godard@bootlin.com>
wrote:

> Hi Bruce,
>
> On Mon Dec 16, 2024 at 2:29 PM CET, Bruce Ashfield via
> lists.openembedded.org wrote:
> > On Mon, Dec 16, 2024 at 8:21 AM Bruce Ashfield <bruce.ashfield@gmail.com
> >
> > wrote:
> >
> >>
> >>
> >> On Mon, Dec 16, 2024 at 6:35 AM Antonin Godard <
> antonin.godard@bootlin.com>
> >> wrote:
> >>
> >>> Following commit a39a1f7cf78ad1ca07438bce634a47e970f25047
> >>> ("kernel-yocto: allow early exit to configuration audit"), we can
> >>> disable auditing of the kernel configuration by setting KMETA_AUDIT
> >>> empty.
> >>>
> >>> Since the default value of this variables is "yes", use
> >>> oe.types.boolean() so that it will also be disabled when setting it to
> >>> "no", and avoid potential confusion.
> >>>
> >>
> >> That isn't the intent of the variable. It is being used as a flag here,
> >> but it can also be used to encode other information.
> >>
> >> Quite literally, if it is set to anything, the audit should happen. Only
> >> when it is unset should it not.
> >>
> >
> > I hit send too soon. It's Monday.
> >
> > What I wanted to add was this question: Is that still the behaviour with
> > the boolean call ? If so, no concerns from me.
>
> It seems that this is the only place where the KMETA_AUDIT variable is
> used so I
> figured its role was to enable/disable the audit and only that. Maybe the
> "audit_flag" name is a bit misleading then?
>

Maybe, but I'd rather not change the name if nothing is broken.

I normally just put a comment above the variable to indicate
that sort of behaviour (i.e. # if this variable is set to anything non-zero
an audit will be performed), but I for some reason didn't do that
here!

There are some audit modes for in-tree configs that I'm working
on that will use that variable to change modes (different from the
allnoconfig/alldefconfig flags that are already used).

Bruce



>
> The behavior of the boolean call is to return False if KMETA_AUDIT ==
> "no", so
> it _is_ different from "if set to anything, the audit happens".
>
>
> Antonin
>
> --
> Antonin Godard, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
Antonin Godard Dec. 16, 2024, 2:09 p.m. UTC | #5
On Mon Dec 16, 2024 at 2:56 PM CET, Bruce Ashfield via lists.openembedded.org wrote:
> On Mon, Dec 16, 2024 at 8:50 AM Antonin Godard <antonin.godard@bootlin.com>
> wrote:
>
>> Hi Bruce,
>>
>> On Mon Dec 16, 2024 at 2:29 PM CET, Bruce Ashfield via
>> lists.openembedded.org wrote:
>> > On Mon, Dec 16, 2024 at 8:21 AM Bruce Ashfield <bruce.ashfield@gmail.com
>> >
>> > wrote:
>> >
>> >>
>> >>
>> >> On Mon, Dec 16, 2024 at 6:35 AM Antonin Godard <
>> antonin.godard@bootlin.com>
>> >> wrote:
>> >>
>> >>> Following commit a39a1f7cf78ad1ca07438bce634a47e970f25047
>> >>> ("kernel-yocto: allow early exit to configuration audit"), we can
>> >>> disable auditing of the kernel configuration by setting KMETA_AUDIT
>> >>> empty.
>> >>>
>> >>> Since the default value of this variables is "yes", use
>> >>> oe.types.boolean() so that it will also be disabled when setting it to
>> >>> "no", and avoid potential confusion.
>> >>>
>> >>
>> >> That isn't the intent of the variable. It is being used as a flag here,
>> >> but it can also be used to encode other information.
>> >>
>> >> Quite literally, if it is set to anything, the audit should happen. Only
>> >> when it is unset should it not.
>> >>
>> >
>> > I hit send too soon. It's Monday.
>> >
>> > What I wanted to add was this question: Is that still the behaviour with
>> > the boolean call ? If so, no concerns from me.
>>
>> It seems that this is the only place where the KMETA_AUDIT variable is
>> used so I
>> figured its role was to enable/disable the audit and only that. Maybe the
>> "audit_flag" name is a bit misleading then?
>>
>
> Maybe, but I'd rather not change the name if nothing is broken.
>
> I normally just put a comment above the variable to indicate
> that sort of behaviour (i.e. # if this variable is set to anything non-zero
> an audit will be performed), but I for some reason didn't do that
> here!
>
> There are some audit modes for in-tree configs that I'm working
> on that will use that variable to change modes (different from the
> allnoconfig/alldefconfig flags that are already used).

Ok. boolean() will error if it doesn't find 'yes', 'no', '1', '0' or any such
value, so we need to NACK this patch then. Thanks for the added context.

I will work on documenting this in yocto-docs when I have the time (unless
you've got some free time to document these variables :) ). There's an open bug
on this here https://bugzilla.yoctoproject.org/show_bug.cgi?id=13835, which is
the initial reason why I was looking into these.


Antonin
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
index a5d89dc2c8bc912539f7c9ffc25a2576920f580a..1493ba20a0c25df556df41fa4137ab17f67c91e7 100644
--- a/meta/classes-recipe/kernel-yocto.bbclass
+++ b/meta/classes-recipe/kernel-yocto.bbclass
@@ -568,7 +568,9 @@  python do_config_analysis() {
 python do_kernel_configcheck() {
     import re, string, sys, subprocess
 
-    audit_flag = d.getVar( "KMETA_AUDIT" )
+    audit_flag = False
+    if d.getVar("KMETA_AUDIT"):
+        audit_flag = oe.types.boolean(d.getVar("KMETA_AUDIT"))
     if not audit_flag:
        bb.note( "kernel config audit disabled, skipping .." )
        return