| Message ID | 20251114060211.1742728-1-Qi.Chen@windriver.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bitbake-devel,V2] lib/bb/parse/ast.py: error out for internal fragment in case of a previous value | expand |
You need to explain the difference between v1 and v2 and why v2 is needed. What was wrong with v1? The environment pass through hack is horrible. Alex On Fri 14. Nov 2025 at 7.02, <Qi.Chen@windriver.com> wrote: > From: Chen Qi <Qi.Chen@windriver.com> > > When an internal fragment is enabled, and there's already a value > for the corresponding variable, we should error out to avoid any > confusion. > > For example, when 'machine/qemux86-64' fragement is enabled, and > we get some "MACHINE = xxx" in local.conf or env, we should error > out and recomment users to use 'bitbake-config-build disable-fragment'. > > We should be tolerating weak assignments. For example, DISTRO defaults > to "nodistro", and when 'distro/poky" fragment is enabled, there should > be no confusion. > > The implementation hacks the environment variable as a way to tell > bitbake that we're using 'bitbake-config-build'. Because we recommend > users to use bitbake-config-build, then it should not error out. > > Fixes [YOCTO #16060] > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > bin/bitbake-layers | 3 +++ > lib/bb/parse/ast.py | 7 +++++++ > 2 files changed, 10 insertions(+) > > diff --git a/bin/bitbake-layers b/bin/bitbake-layers > index 341ecbcd9..c4569c6c3 100755 > --- a/bin/bitbake-layers > +++ b/bin/bitbake-layers > @@ -64,6 +64,9 @@ def main(): > if global_args.force > 1: > bbpaths = [] > else: > + if toolname == "bitbake-config-build": > + os.environ["BB_ENV_PASSTHROUGH_ADDITIONS"] = > os.getenv("BB_ENV_PASSTHROUGH_ADDITIONS") + " > _BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_" > + os.environ["_BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_"] = "1" > tinfoil.prepare(True) > bbpaths = tinfoil.config_data.getVar('BBPATH').split(':') > > diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py > index e6ff1ff76..70a2a37a6 100644 > --- a/lib/bb/parse/ast.py > +++ b/lib/bb/parse/ast.py > @@ -364,6 +364,13 @@ class AddFragmentsNode(AstNode): > def check_and_set_builtin_fragment(fragment, data, > builtin_fragments): > prefix, value = fragment.split('/', 1) > if prefix in builtin_fragments.keys(): > + if data.getVar(builtin_fragments[prefix], > noweakdefault=True) != None: > + if not > os.getenv("_BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_"): > + bb.fatal( > + ("A builtin fragment '%s' is used while %s > has already got an assignment.\n" > + "Please either disable the fragment or > remove the value assignment.\n" > + "To disable the fragment, use > 'bitbake-config-build disable-fragment %s'." > + ) % (fragment, builtin_fragments[prefix], > fragment)) > fragment_history = > data.varhistory.variable(self.fragments_variable) > loginfo={} > for fh in fragment_history[::-1]: > -- > 2.34.1 > >
On 11/14/25 14:15, Alexander Kanavin wrote: > You need to explain the difference between v1 and v2 and why v2 is > needed. What was wrong with v1? > > The environment pass through hack is horrible. > > Alex Hi Alex, V1 to fix YOCTO #16060 is to use default value for internal fragment. V2 is to error out in case of a previous value. See Richard's comments in https://bugzilla.yoctoproject.org/show_bug.cgi?id=16060 I know that the env method is a horrible hack. I looked around but could not find any other choice. Do you have any suggestion? Regards, Qi > > On Fri 14. Nov 2025 at 7.02, <Qi.Chen@windriver.com> wrote: > > From: Chen Qi <Qi.Chen@windriver.com> > > When an internal fragment is enabled, and there's already a value > for the corresponding variable, we should error out to avoid any > confusion. > > For example, when 'machine/qemux86-64' fragement is enabled, and > we get some "MACHINE = xxx" in local.conf or env, we should error > out and recomment users to use 'bitbake-config-build > disable-fragment'. > > We should be tolerating weak assignments. For example, DISTRO defaults > to "nodistro", and when 'distro/poky" fragment is enabled, there > should > be no confusion. > > The implementation hacks the environment variable as a way to tell > bitbake that we're using 'bitbake-config-build'. Because we recommend > users to use bitbake-config-build, then it should not error out. > > Fixes [YOCTO #16060] > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > bin/bitbake-layers | 3 +++ > lib/bb/parse/ast.py > <https://urldefense.com/v3/__http://ast.py__;!!AjveYdw8EvQ!ZSR5THHxswHSbWGyBGxON8N8yJ_VhCA5M36eDazemj6ot6xLm281YkTXH7rtSU6j-Jad4Pl6r20d3jZ-lxN1l8c$> > | 7 +++++++ > 2 files changed, 10 insertions(+) > > diff --git a/bin/bitbake-layers b/bin/bitbake-layers > index 341ecbcd9..c4569c6c3 100755 > --- a/bin/bitbake-layers > +++ b/bin/bitbake-layers > @@ -64,6 +64,9 @@ def main(): > if global_args.force > 1: > bbpaths = [] > else: > + if toolname == "bitbake-config-build": > + os.environ["BB_ENV_PASSTHROUGH_ADDITIONS"] = > os.getenv("BB_ENV_PASSTHROUGH_ADDITIONS") + " > _BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_" > + os.environ["_BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_"] = "1" > tinfoil.prepare(True) > bbpaths = tinfoil.config_data.getVar('BBPATH').split(':') > > diff --git a/lib/bb/parse/ast.py > <https://urldefense.com/v3/__http://ast.py__;!!AjveYdw8EvQ!ZSR5THHxswHSbWGyBGxON8N8yJ_VhCA5M36eDazemj6ot6xLm281YkTXH7rtSU6j-Jad4Pl6r20d3jZ-lxN1l8c$> > b/lib/bb/parse/ast.py > <https://urldefense.com/v3/__http://ast.py__;!!AjveYdw8EvQ!ZSR5THHxswHSbWGyBGxON8N8yJ_VhCA5M36eDazemj6ot6xLm281YkTXH7rtSU6j-Jad4Pl6r20d3jZ-lxN1l8c$> > index e6ff1ff76..70a2a37a6 100644 > --- a/lib/bb/parse/ast.py > <https://urldefense.com/v3/__http://ast.py__;!!AjveYdw8EvQ!ZSR5THHxswHSbWGyBGxON8N8yJ_VhCA5M36eDazemj6ot6xLm281YkTXH7rtSU6j-Jad4Pl6r20d3jZ-lxN1l8c$> > +++ b/lib/bb/parse/ast.py > <https://urldefense.com/v3/__http://ast.py__;!!AjveYdw8EvQ!ZSR5THHxswHSbWGyBGxON8N8yJ_VhCA5M36eDazemj6ot6xLm281YkTXH7rtSU6j-Jad4Pl6r20d3jZ-lxN1l8c$> > @@ -364,6 +364,13 @@ class AddFragmentsNode(AstNode): > def check_and_set_builtin_fragment(fragment, data, > builtin_fragments): > prefix, value = fragment.split('/', 1) > if prefix in builtin_fragments.keys(): > + if data.getVar(builtin_fragments[prefix], > noweakdefault=True) != None: > + if not > os.getenv("_BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_"): > + bb.fatal( > + ("A builtin fragment '%s' is used > while %s has already got an assignment.\n" > + "Please either disable the fragment > or remove the value assignment.\n" > + "To disable the fragment, use > 'bitbake-config-build disable-fragment %s'." > + ) % (fragment, > builtin_fragments[prefix], fragment)) > fragment_history = > data.varhistory.variable(self.fragments_variable) > loginfo={} > for fh in fragment_history[::-1]: > -- > 2.34.1 >
On Fri, 14 Nov 2025 at 07:20, ChenQi <Qi.Chen@windriver.com> wrote:
> I know that the env method is a horrible hack. I looked around but could not find any other choice. Do you have any suggestion?
I don't understand why the special exception in bitbake-config-build
is needed to begin with. What is the scenario where it is necessary?
Alex
On 11/14/25 14:28, Alexander Kanavin wrote: > On Fri, 14 Nov 2025 at 07:20, ChenQi <Qi.Chen@windriver.com> wrote: >> I know that the env method is a horrible hack. I looked around but could not find any other choice. Do you have any suggestion? > I don't understand why the special exception in bitbake-config-build > is needed to begin with. What is the scenario where it is necessary? > > Alex Let me use an example to explain this one. When machine/qemux86-64 fragment is enabled and users use 'MACHINE=xxx bitbake <pkg>', the expected behavior is to error out and recommend user to use 'bitbake-config-build disable-fragment'. Here comes the problem. bitbake-config-build makes use of tinfoil and when setting up tinfoil, the above error will appear again. We can't recommend users to use 'bitbake-config-build disable-fragment' while the recommended method still errors out. This is the reason for the special exception. Regards, Qi
On Fri, 14 Nov 2025 at 07:37, ChenQi <Qi.Chen@windriver.com> wrote: > When machine/qemux86-64 fragment is enabled and users use 'MACHINE=xxx > bitbake <pkg>', the expected behavior is to error out and recommend user > to use 'bitbake-config-build disable-fragment'. > > Here comes the problem. bitbake-config-build makes use of tinfoil and > when setting up tinfoil, the above error will appear again. We can't > recommend users to use 'bitbake-config-build disable-fragment' while the > recommended method still errors out. This is the reason for the special > exception. But why would users be running 'MACHINE=xxx bitbake-config-build disable-fragment ...' ? That prefix is intended only for bitbake invocations. Alex
On 11/14/25 14:49, Alexander Kanavin wrote: > On Fri, 14 Nov 2025 at 07:37, ChenQi <Qi.Chen@windriver.com> wrote: >> When machine/qemux86-64 fragment is enabled and users use 'MACHINE=xxx >> bitbake <pkg>', the expected behavior is to error out and recommend user >> to use 'bitbake-config-build disable-fragment'. >> >> Here comes the problem. bitbake-config-build makes use of tinfoil and >> when setting up tinfoil, the above error will appear again. We can't >> recommend users to use 'bitbake-config-build disable-fragment' while the >> recommended method still errors out. This is the reason for the special >> exception. > But why would users be running 'MACHINE=xxx bitbake-config-build > disable-fragment ...' ? That prefix is intended only for bitbake > invocations. > > Alex I mean that users run something like 'MACHINE=qemuarm64 bitbake base-files', and they get error like below: ERROR: A builtin fragment 'machine/qemux86-64' is used while MACHINE has already got an assignment. Please either disable the fragment or remove the value assignment. To disable the fragment, use 'bitbake-config-build disable-fragment machine/qemux86-64'. Now they run 'bitbake-config-build disable-fragment machine/qemux86-64' as recommended. If there's no such special exception, they will get the same error again. Even if they run a simple 'bitbake-config-build --help', they'll get the same error again. That's the reason for this special exception. Regards, Qi
On 11/14/25 15:12, Chen Qi via lists.openembedded.org wrote: > On 11/14/25 14:49, Alexander Kanavin wrote: >> On Fri, 14 Nov 2025 at 07:37, ChenQi <Qi.Chen@windriver.com> wrote: >>> When machine/qemux86-64 fragment is enabled and users use 'MACHINE=xxx >>> bitbake <pkg>', the expected behavior is to error out and recommend >>> user >>> to use 'bitbake-config-build disable-fragment'. >>> >>> Here comes the problem. bitbake-config-build makes use of tinfoil and >>> when setting up tinfoil, the above error will appear again. We can't >>> recommend users to use 'bitbake-config-build disable-fragment' while >>> the >>> recommended method still errors out. This is the reason for the special >>> exception. >> But why would users be running 'MACHINE=xxx bitbake-config-build >> disable-fragment ...' ? That prefix is intended only for bitbake >> invocations. >> >> Alex > Hi Alex, Ignore the reply below. Sorry, I just realized what's going on here. I was using 'MACHINE ="qemuarm64"' in local.conf to test things. So the error will appear again and again even with a simple 'bitbake-config-build --help' if there's no such special exception. Regards, Qi > I mean that users run something like 'MACHINE=qemuarm64 bitbake > base-files', and they get error like below: > > ERROR: A builtin fragment 'machine/qemux86-64' is used while MACHINE > has already got an assignment. > Please either disable the fragment or remove the value assignment. > To disable the fragment, use 'bitbake-config-build disable-fragment > machine/qemux86-64'. > > Now they run 'bitbake-config-build disable-fragment > machine/qemux86-64' as recommended. If there's no such special > exception, they will get the same error again. Even if they run a > simple 'bitbake-config-build --help', they'll get the same error again. > > That's the reason for this special exception. > > Regards, > Qi > > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#18387): https://lists.openembedded.org/g/bitbake-devel/message/18387 > Mute This Topic: https://lists.openembedded.org/mt/116288219/7304865 > Group Owner: bitbake-devel+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [Qi.Chen@eng.windriver.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Thanks Chen. I’m only reading emails this week, but I’ll try to look into it and come up with ideas on Monday. Alex On Fri 14. Nov 2025 at 8.17, ChenQi <Qi.Chen@windriver.com> wrote: > On 11/14/25 15:12, Chen Qi via lists.openembedded.org wrote: > > On 11/14/25 14:49, Alexander Kanavin wrote: > >> On Fri, 14 Nov 2025 at 07:37, ChenQi <Qi.Chen@windriver.com> wrote: > >>> When machine/qemux86-64 fragment is enabled and users use 'MACHINE=xxx > >>> bitbake <pkg>', the expected behavior is to error out and recommend > >>> user > >>> to use 'bitbake-config-build disable-fragment'. > >>> > >>> Here comes the problem. bitbake-config-build makes use of tinfoil and > >>> when setting up tinfoil, the above error will appear again. We can't > >>> recommend users to use 'bitbake-config-build disable-fragment' while > >>> the > >>> recommended method still errors out. This is the reason for the special > >>> exception. > >> But why would users be running 'MACHINE=xxx bitbake-config-build > >> disable-fragment ...' ? That prefix is intended only for bitbake > >> invocations. > >> > >> Alex > > > Hi Alex, > > Ignore the reply below. Sorry, I just realized what's going on here. > > I was using 'MACHINE ="qemuarm64"' in local.conf to test things. > > So the error will appear again and again even with a simple > 'bitbake-config-build --help' if there's no such special exception. > > Regards, > Qi > > > I mean that users run something like 'MACHINE=qemuarm64 bitbake > > base-files', and they get error like below: > > > > ERROR: A builtin fragment 'machine/qemux86-64' is used while MACHINE > > has already got an assignment. > > Please either disable the fragment or remove the value assignment. > > To disable the fragment, use 'bitbake-config-build disable-fragment > > machine/qemux86-64'. > > > > Now they run 'bitbake-config-build disable-fragment > > machine/qemux86-64' as recommended. If there's no such special > > exception, they will get the same error again. Even if they run a > > simple 'bitbake-config-build --help', they'll get the same error again. > > > > That's the reason for this special exception. > > > > Regards, > > Qi > > > > > > -=-=-=-=-=-=-=-=-=-=-=- > > Links: You receive all messages sent to this group. > > View/Reply Online (#18387): > https://lists.openembedded.org/g/bitbake-devel/message/18387 > > Mute This Topic: https://lists.openembedded.org/mt/116288219/7304865 > > Group Owner: bitbake-devel+owner@lists.openembedded.org > > Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [ > Qi.Chen@eng.windriver.com] > > -=-=-=-=-=-=-=-=-=-=-=- > > > >
On Fri, 2025-11-14 at 14:02 +0800, Qi.Chen@windriver.com wrote: > From: Chen Qi <Qi.Chen@windriver.com> > > When an internal fragment is enabled, and there's already a value > for the corresponding variable, we should error out to avoid any > confusion. > > For example, when 'machine/qemux86-64' fragement is enabled, and > we get some "MACHINE = xxx" in local.conf or env, we should error > out and recomment users to use 'bitbake-config-build disable-fragment'. > > We should be tolerating weak assignments. For example, DISTRO defaults > to "nodistro", and when 'distro/poky" fragment is enabled, there should > be no confusion. > > The implementation hacks the environment variable as a way to tell > bitbake that we're using 'bitbake-config-build'. Because we recommend > users to use bitbake-config-build, then it should not error out. > > Fixes [YOCTO #16060] > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > --- > bin/bitbake-layers | 3 +++ > lib/bb/parse/ast.py | 7 +++++++ > 2 files changed, 10 insertions(+) Thanks, I think this is a step in the right direction but I can see the challenge you're facing with the parsing. There are probably other bugs in this area, for example what if there is an invalid fragment set (you delete a fragment file, then try to disable it with bitbake-build- config?). What might be a slightly "nicer" approach would be to pass the toolname into the bitbake datastore as some variable (BB_TOOLNAME?) from tinfoil, then we might want to skip all of the fragment code if bitbake-config-build is in use? We'd have to check what other implications that might have, for example, does bitbake-build-config need a full datastore? What are the implications for bitbake's cache files and cache file hashes? Cheers, Richard
On Fri, 2025-11-14 at 12:14 +0000, Richard Purdie via lists.openembedded.org wrote: > On Fri, 2025-11-14 at 14:02 +0800, Qi.Chen@windriver.com wrote: > > From: Chen Qi <Qi.Chen@windriver.com> > > > > When an internal fragment is enabled, and there's already a value > > for the corresponding variable, we should error out to avoid any > > confusion. > > > > For example, when 'machine/qemux86-64' fragement is enabled, and > > we get some "MACHINE = xxx" in local.conf or env, we should error > > out and recomment users to use 'bitbake-config-build disable-fragment'. > > > > We should be tolerating weak assignments. For example, DISTRO defaults > > to "nodistro", and when 'distro/poky" fragment is enabled, there should > > be no confusion. > > > > The implementation hacks the environment variable as a way to tell > > bitbake that we're using 'bitbake-config-build'. Because we recommend > > users to use bitbake-config-build, then it should not error out. > > > > Fixes [YOCTO #16060] > > > > Signed-off-by: Chen Qi <Qi.Chen@windriver.com> > > --- > > bin/bitbake-layers | 3 +++ > > lib/bb/parse/ast.py | 7 +++++++ > > 2 files changed, 10 insertions(+) > > Thanks, I think this is a step in the right direction but I can see the > challenge you're facing with the parsing. There are probably other bugs > in this area, for example what if there is an invalid fragment set (you > delete a fragment file, then try to disable it with bitbake-build- > config?). > > What might be a slightly "nicer" approach would be to pass the toolname > into the bitbake datastore as some variable (BB_TOOLNAME?) from > tinfoil, then we might want to skip all of the fragment code if > bitbake-config-build is in use? > > We'd have to check what other implications that might have, for > example, does bitbake-build-config need a full datastore? What are the > implications for bitbake's cache files and cache file hashes? I did have a couple of other ideas: a) we could pass in options to tinfoil. We could have a fragements=True/False parameter and allow this tool to disable those. b) we could add a new CookerFeatures option to disable fragments. I'm kind of leaning to b) since this was the existing designed in mechanism to change things like this. Cheers, Richard
diff --git a/bin/bitbake-layers b/bin/bitbake-layers index 341ecbcd9..c4569c6c3 100755 --- a/bin/bitbake-layers +++ b/bin/bitbake-layers @@ -64,6 +64,9 @@ def main(): if global_args.force > 1: bbpaths = [] else: + if toolname == "bitbake-config-build": + os.environ["BB_ENV_PASSTHROUGH_ADDITIONS"] = os.getenv("BB_ENV_PASSTHROUGH_ADDITIONS") + " _BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_" + os.environ["_BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_"] = "1" tinfoil.prepare(True) bbpaths = tinfoil.config_data.getVar('BBPATH').split(':') diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py index e6ff1ff76..70a2a37a6 100644 --- a/lib/bb/parse/ast.py +++ b/lib/bb/parse/ast.py @@ -364,6 +364,13 @@ class AddFragmentsNode(AstNode): def check_and_set_builtin_fragment(fragment, data, builtin_fragments): prefix, value = fragment.split('/', 1) if prefix in builtin_fragments.keys(): + if data.getVar(builtin_fragments[prefix], noweakdefault=True) != None: + if not os.getenv("_BB_INTERNAL_RUN_BITBAKE_CONFIG_BUILD_"): + bb.fatal( + ("A builtin fragment '%s' is used while %s has already got an assignment.\n" + "Please either disable the fragment or remove the value assignment.\n" + "To disable the fragment, use 'bitbake-config-build disable-fragment %s'." + ) % (fragment, builtin_fragments[prefix], fragment)) fragment_history = data.varhistory.variable(self.fragments_variable) loginfo={} for fh in fragment_history[::-1]: