diff mbox series

[RFC] parse/ast: add support for additive built-in fragments

Message ID 20260901-appending-fragments-v1-1-2e359cf751ce@bootlin.com
State New
Headers show
Series [RFC] parse/ast: add support for additive built-in fragments | expand

Commit Message

Antonin Godard Sept. 1, 2026, 7:52 a.m. UTC 
Add support for the following syntax specified in a fragment list
definition:

  fragmentname:VARIABLE:add

Setting this fragment appends to VARIABLE instead of setting its value.
This does not break pre-existing fragments which still set a variable's
value entirely.

For example, in OE-Core consider the following definition:

  OE_FRAGMENTS_BUILTIN = "class:INHERIT:add"

Then one could specify the following:

  OE_FRAGMENTS = "class/buildstats class/rm_work"

Which would result in appending " buildstats rm_work" to the INHERIT
variable.

This would also allow bitbake-setup configurations to come with a list
of pre-enabled classes, for example.

Note: The "add" suffix was chosen in the definition to avoid confusion
with the existing "append" usage in Bitbake.

Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
---
Note: The bitbake-config-build OE-Core utility would require adaptations
as it currently removes any pre-existing built-in fragment with the
same suffix. I can also send the associated OE-Core patches.

Note 2: We could also have these fragments specified as:

  OE_FRAGMENTS = "class/add/buildstats class/add/retain"

To avoid confusing them with original built-in fragments. While I
agree it disambiguates them from original built-ins, I also think from a
user point of view, being able to set:

  OE_FRAGMENTS = "machine/qemuarm distro/poky class/buildstats class/retain"

feels a bit more natural. Discussion is open :)
---
 lib/bb/parse/ast.py | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)


---
base-commit: 18cca50ba3da5ce27bc674478957bb08e7536b9a
change-id: 20260827-appending-fragments-8a308ebdb633

Comments

Richard Purdie Sept. 1, 2026, 12:15 p.m. UTC | #1 
On Tue, 2026-09-01 at 09:52 +0200, Antonin Godard via lists.openembedded.org wrote:
> Add support for the following syntax specified in a fragment list
> definition:
> 
>   fragmentname:VARIABLE:add
> 
> Setting this fragment appends to VARIABLE instead of setting its value.
> This does not break pre-existing fragments which still set a variable's
> value entirely.
> 
> For example, in OE-Core consider the following definition:
> 
>   OE_FRAGMENTS_BUILTIN = "class:INHERIT:add"
> 
> Then one could specify the following:
> 
>   OE_FRAGMENTS = "class/buildstats class/rm_work"
> 
> Which would result in appending " buildstats rm_work" to the INHERIT
> variable.
> 
> This would also allow bitbake-setup configurations to come with a list
> of pre-enabled classes, for example.
> 
> Note: The "add" suffix was chosen in the definition to avoid confusion
> with the existing "append" usage in Bitbake.
> 
> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
> ---
> Note: The bitbake-config-build OE-Core utility would require adaptations
> as it currently removes any pre-existing built-in fragment with the
> same suffix. I can also send the associated OE-Core patches.
> 
> Note 2: We could also have these fragments specified as:
> 
>   OE_FRAGMENTS = "class/add/buildstats class/add/retain"
> 
> To avoid confusing them with original built-in fragments. While I
> agree it disambiguates them from original built-ins, I also think from a
> user point of view, being able to set:
> 
>   OE_FRAGMENTS = "machine/qemuarm distro/poky class/buildstats class/retain"
> 
> feels a bit more natural. Discussion is open :)


It is definitely an interesting one and the syntax isn't too bad.. Are
there uses beyond class inherit though?

machine/distro were added as they are clear variables in common use
which avoided tons of boilerplate in layer definitions. Do we have
similar needs here or are these inherits better captured in config
fragments?

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/parse/ast.py b/lib/bb/parse/ast.py
index a372b3534e4..a828c49c937 100644
--- a/lib/bb/parse/ast.py
+++ b/lib/bb/parse/ast.py
@@ -367,12 +367,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:
+                fragment_var, fragment_type = builtin_fragments[prefix][0], builtin_fragments[prefix][1]
+                if fragment_type == "set" and data.getVar(fragment_var, noweakdefault=True) is not None:
                     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, fragment_var, fragment))
                 fragment_history = data.varhistory.variable(self.fragments_variable)
                 loginfo={}
                 for fh in fragment_history[::-1]:
@@ -381,15 +382,27 @@  class AddFragmentsNode(AstNode):
                         loginfo["line"]   = fh["line"]
                         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)
+                if fragment_type == "set":
+                    # parsing=True since we want to emulate X=Y and allow X:override=Z to continue to exist
+                    data.setVar(fragment_var, value, parsing=True, **loginfo)
+                elif fragment_type == "add":
+                    data.appendVar(fragment_var, f" {value.strip()}", **loginfo)
+                else:
+                    bb.fatal(f"Unknown fragment type '{fragment_type}' "
+                             f"(from '{prefix}:{fragment_var}:{fragment_type}' in {self.builtin_fragments_variable})")
                 return True
             return False
 
         fragments = data.getVar(self.fragments_variable)
         layers = data.getVar('BBLAYERS')
         flagged_variables = data.getVar(self.flagged_variables_list_variable).split()
-        builtin_fragments = {f[0]:f[1] for f in [f.split(':') for f in data.getVar(self.builtin_fragments_variable).split()] }
+        builtin_fragments = {}
+        for fragment in data.getVar(self.builtin_fragments_variable).split():
+            fragment = fragment.split(':')
+            builtin_fragments[fragment[0]] = (
+                fragment[1],
+                "set" if len(fragment) < 3 else fragment[2],
+            )
 
         if not fragments:
             return
@@ -402,7 +415,7 @@  class AddFragmentsNode(AstNode):
                     fragments.split(),
                 )
             )
-            if len(builtin_fragments_list) > 1:
+            if len(builtin_fragments_list) > 1 and not builtin_fragments[builtin_fragment_key][1] == "add":
                 bb.warn(
                     ("Multiple builtin fragments are enabled for %s via variable %s: %s. "
                      "This likely points to a mis-configuration in the metadata, as only "