diff mbox series

[bitbake-devel] lib/bb/parse/ast.py: treat internal fragment setting as default value

Message ID 20251112055736.3078776-1-Qi.Chen@windriver.com
State New
Headers show
Series [bitbake-devel] lib/bb/parse/ast.py: treat internal fragment setting as default value | expand

Commit Message

Chen, Qi Nov. 12, 2025, 5:57 a.m. UTC
From: Chen Qi <Qi.Chen@windriver.com>

In this way, users can continue to override settings such as MACHINE
via local.conf and env vars with plain '=' setting.

Fixes [YOCTO #16060]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 lib/bb/parse/ast.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Richard Purdie Nov. 12, 2025, 4:01 p.m. UTC | #1
On Wed, 2025-11-12 at 13:57 +0800, Chen Qi via lists.openembedded.org wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> In this way, users can continue to override settings such as MACHINE
> via local.conf and env vars with plain '=' setting.
> 
> Fixes [YOCTO #16060]
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  lib/bb/parse/ast.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
> index e6ff1ff76..f9d93aa0f 100644
> --- a/lib/bb/parse/ast.py
> +++ b/lib/bb/parse/ast.py
> @@ -373,7 +373,7 @@ class AddFragmentsNode(AstNode):
>                          loginfo["detail"] = f"{value} ({self.fragments_variable} contains \"{fragment}\")"
>                          break
>                  # parsing=True since we want to emulate X=Y and allow X:override=Z to continue to exist
> -                data.setVar(builtin_fragments[prefix], value, parsing=True, **loginfo)
> +                data.setVarFlag(builtin_fragments[prefix], '_defaultval', value, parsing=True, **loginfo)
>                  return True
>              return False
> 


This is a tough one. I've thought about the two sides to this and you
can argue it both ways.

If we do this, we will get questions of "I set this fragment, why
didn't it actually do that?".

I think if you specify a fragment and it should override the conf
files...

Cheers,

Richard
Chen, Qi Nov. 13, 2025, 3:17 a.m. UTC | #2
Thank you for the explanation. 

My major concern is about the behavioral change in a project setup by bitbake-setup.

When I was trying out bitbake-setup, the OE_FRAGMENTS are automatically generated in conf/toolcfg.conf. And I got an "empty" local.conf. So at that time, I felt it natural that user configuration (local.conf) should take priority over the automatically generated one (toolcfg.conf). The logic is like /etc and /usr in Linux systems.

It's a little counter-intuitive that local.conf does not override settings in toolcfg.conf. I mean, people have to  edit toolcfg.conf to change MACHINE in a project setup by bitbake-setup. And they can't use "MACHINE=xxx bitbake <pkg>".

Regards,
Qi

-----Original Message-----
From: Richard Purdie <richard.purdie@linuxfoundation.org> 
Sent: Thursday, November 13, 2025 12:01 AM
To: Chen, Qi <Qi.Chen@windriver.com>; bitbake-devel@lists.openembedded.org
Cc: alex.kanavin@gmail.com
Subject: Re: [bitbake-devel][PATCH] lib/bb/parse/ast.py: treat internal fragment setting as default value

On Wed, 2025-11-12 at 13:57 +0800, Chen Qi via lists.openembedded.org wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> In this way, users can continue to override settings such as MACHINE 
> via local.conf and env vars with plain '=' setting.
> 
> Fixes [YOCTO #16060]
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  lib/bb/parse/ast.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py index 
> e6ff1ff76..f9d93aa0f 100644
> --- a/lib/bb/parse/ast.py
> +++ b/lib/bb/parse/ast.py
> @@ -373,7 +373,7 @@ class AddFragmentsNode(AstNode):
>                          loginfo["detail"] = f"{value} ({self.fragments_variable} contains \"{fragment}\")"
>                          break
>                  # parsing=True since we want to emulate X=Y and allow 
> X:override=Z to continue to exist
> -                data.setVar(builtin_fragments[prefix], value, 
> parsing=True, **loginfo)
> +                data.setVarFlag(builtin_fragments[prefix], 
> +'_defaultval', value, parsing=True, **loginfo)
>                  return True
>              return False
> 


This is a tough one. I've thought about the two sides to this and you can argue it both ways.

If we do this, we will get questions of "I set this fragment, why didn't it actually do that?".

I think if you specify a fragment and it should override the conf files...

Cheers,

Richard
Alexander Kanavin Nov. 13, 2025, 5:13 a.m. UTC | #3
For what it’s worth I agree with Chen. Priority order should be
environment, local.conf and only then toolcfg.conf. Yes it will lead to
confusion in legacy local.conf driven setups, but overriding machine on the
command line is a common use case and it should work.

Alex

On Thu 13. Nov 2025 at 4.17, Chen, Qi <Qi.Chen@windriver.com> wrote:

> Thank you for the explanation.
>
> My major concern is about the behavioral change in a project setup by
> bitbake-setup.
>
> When I was trying out bitbake-setup, the OE_FRAGMENTS are automatically
> generated in conf/toolcfg.conf. And I got an "empty" local.conf. So at that
> time, I felt it natural that user configuration (local.conf) should take
> priority over the automatically generated one (toolcfg.conf). The logic is
> like /etc and /usr in Linux systems.
>
> It's a little counter-intuitive that local.conf does not override settings
> in toolcfg.conf. I mean, people have to  edit toolcfg.conf to change
> MACHINE in a project setup by bitbake-setup. And they can't use
> "MACHINE=xxx bitbake <pkg>".
>
> Regards,
> Qi
>
> -----Original Message-----
> From: Richard Purdie <richard.purdie@linuxfoundation.org>
> Sent: Thursday, November 13, 2025 12:01 AM
> To: Chen, Qi <Qi.Chen@windriver.com>; bitbake-devel@lists.openembedded.org
> Cc: alex.kanavin@gmail.com
> Subject: Re: [bitbake-devel][PATCH] lib/bb/parse/ast.py: treat internal
> fragment setting as default value
>
> On Wed, 2025-11-12 at 13:57 +0800, Chen Qi via lists.openembedded.org
> wrote:
> > From: Chen Qi <Qi.Chen@windriver.com>
> >
> > In this way, users can continue to override settings such as MACHINE
> > via local.conf and env vars with plain '=' setting.
> >
> > Fixes [YOCTO #16060]
> >
> > Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> > ---
> >  lib/bb/parse/ast.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py index
> > e6ff1ff76..f9d93aa0f 100644
> > --- a/lib/bb/parse/ast.py
> > +++ b/lib/bb/parse/ast.py
> > @@ -373,7 +373,7 @@ class AddFragmentsNode(AstNode):
> >                          loginfo["detail"] = f"{value}
> ({self.fragments_variable} contains \"{fragment}\")"
> >                          break
> >                  # parsing=True since we want to emulate X=Y and allow
> > X:override=Z to continue to exist
> > -                data.setVar(builtin_fragments[prefix], value,
> > parsing=True, **loginfo)
> > +                data.setVarFlag(builtin_fragments[prefix],
> > +'_defaultval', value, parsing=True, **loginfo)
> >                  return True
> >              return False
> >
>
>
> This is a tough one. I've thought about the two sides to this and you can
> argue it both ways.
>
> If we do this, we will get questions of "I set this fragment, why didn't
> it actually do that?".
>
> I think if you specify a fragment and it should override the conf files...
>
> Cheers,
>
> Richard
>
>
>
>
>
diff mbox series

Patch

diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
index e6ff1ff76..f9d93aa0f 100644
--- a/lib/bb/parse/ast.py
+++ b/lib/bb/parse/ast.py
@@ -373,7 +373,7 @@  class AddFragmentsNode(AstNode):
                         loginfo["detail"] = f"{value} ({self.fragments_variable} contains \"{fragment}\")"
                         break
                 # parsing=True since we want to emulate X=Y and allow X:override=Z to continue to exist
-                data.setVar(builtin_fragments[prefix], value, parsing=True, **loginfo)
+                data.setVarFlag(builtin_fragments[prefix], '_defaultval', value, parsing=True, **loginfo)
                 return True
             return False