diff mbox series

bitbake-setup: replace {THISDIR} token with an explicit keyword

Message ID 20251106114714.918370-1-alex.kanavin@gmail.com
State New
Headers show
Series bitbake-setup: replace {THISDIR} token with an explicit keyword | expand

Commit Message

Alexander Kanavin Nov. 6, 2025, 11:47 a.m. UTC 
From: Alexander Kanavin <alex@linutronix.de>

{THISDIR} is a special value token that can be used in the list of enabled
layers to specify the layer location relative to the confguration file:
https://git.openembedded.org/bitbake/commit/?id=b3153be29de8b8570b0c184369bd41f4c646cf92

This replaces the token with an explicit separate keyword for such layers:
so that special processing to determine the final value can be avoided, and
the feature can be formalized in the json schema:

instead of
   "bb-layers": [
        "{THISDIR}/meta-my-project"
    ]

this allows
   "bb-layers-relative-to-this-file": [
        "meta-my-project"

Going forward I think we should strive to avoid any further special value tokens.

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 bin/bitbake-setup     | 28 ++++++++++++++++------------
 lib/bb/tests/setup.py | 20 ++++++++++----------
 2 files changed, 26 insertions(+), 22 deletions(-)

Comments

Antonin Godard Nov. 6, 2025, 12:11 p.m. UTC | #1 
Hi,

On Thu Nov 6, 2025 at 12:47 PM CET, Alexander Kanavin via lists.openembedded.org wrote:
> From: Alexander Kanavin <alex@linutronix.de>
>
> {THISDIR} is a special value token that can be used in the list of enabled
> layers to specify the layer location relative to the confguration file:
> https://git.openembedded.org/bitbake/commit/?id=b3153be29de8b8570b0c184369bd41f4c646cf92
>
> This replaces the token with an explicit separate keyword for such layers:
> so that special processing to determine the final value can be avoided, and
> the feature can be formalized in the json schema:
>
> instead of
>    "bb-layers": [
>         "{THISDIR}/meta-my-project"
>     ]
>
> this allows
>    "bb-layers-relative-to-this-file": [

Can I suggest naming it "bb-layers-local"? "bb-layers-relative-to-this-file"
feels bit overwhelming, I think with associated documentation users will be
able to figure what "local" stands for in this case.

I can't help but wonder: why is there a need to have a separate keyword? Can't
bitbake-setup do:

  for <layer> in bb-layers; do
    if <layer> is not in <sources>; then 
      if <layer> is found next to the .conf.json; then 
        use this local layer
      else
        error
      fi
    else
      use setup/layers/<layer>
    fi
  done

?

Antonin
Alexander Kanavin Nov. 6, 2025, 12:28 p.m. UTC | #2 
On Thu 6. Nov 2025 at 13.11, Antonin Godard <antonin.godard@bootlin.com>
wrote:

> Hi,
>
> On Thu Nov 6, 2025 at 12:47 PM CET, Alexander Kanavin via
> lists.openembedded.org wrote:
> > From: Alexander Kanavin <alex@linutronix.de>
> >
> > {THISDIR} is a special value token that can be used in the list of
> enabled
> > layers to specify the layer location relative to the confguration file:
> >
> https://git.openembedded.org/bitbake/commit/?id=b3153be29de8b8570b0c184369bd41f4c646cf92
> >
> > This replaces the token with an explicit separate keyword for such
> layers:
> > so that special processing to determine the final value can be avoided,
> and
> > the feature can be formalized in the json schema:
> >
> > instead of
> >    "bb-layers": [
> >         "{THISDIR}/meta-my-project"
> >     ]
> >
> > this allows
> >    "bb-layers-relative-to-this-file": [
>
> Can I suggest naming it "bb-layers-local"?
> "bb-layers-relative-to-this-file"
> feels bit overwhelming, I think with associated documentation users will be
> able to figure what "local" stands for in this case.



‘Local’ implies the path can be absolute, which is not supported, and it
doesn’t suggest what the path should be relative to. I’d rather make it
longer but clearer.



>
> I can't help but wonder: why is there a need to have a separate keyword?
> Can't
> bitbake-setup do:
>
>   for <layer> in bb-layers; do
>     if <layer> is not in <sources>; then
>       if <layer> is found next to the .conf.json; then
>         use this local layer
>       else
>         error
>       fi
>     else
>       use setup/layers/<layer>
>     fi
>   done
>
> ?



With hidden logic like this it’s not clear by reading the config what is
local and what isn’t. There’s also a possibility of paths clashing.

Alex

>
Richard Purdie Nov. 6, 2025, 3:15 p.m. UTC | #3 
On Thu, 2025-11-06 at 13:28 +0100, Alexander Kanavin via lists.openembedded.org wrote:
> On Thu 6. Nov 2025 at 13.11, Antonin Godard <antonin.godard@bootlin.com> wrote:
> > Hi,
> > 
> > On Thu Nov 6, 2025 at 12:47 PM CET, Alexander Kanavin via lists.openembedded.org wrote:
> > > From: Alexander Kanavin <alex@linutronix.de>
> > > 
> > > {THISDIR} is a special value token that can be used in the list of enabled
> > > layers to specify the layer location relative to the confguration file:
> > > https://git.openembedded.org/bitbake/commit/?id=b3153be29de8b8570b0c184369bd41f4c646cf92
> > > 
> > > This replaces the token with an explicit separate keyword for such layers:
> > > so that special processing to determine the final value can be avoided, and
> > > the feature can be formalized in the json schema:
> > > 
> > > instead of
> > >     "bb-layers": [
> > >          "{THISDIR}/meta-my-project"
> > >      ]
> > > 
> > > this allows
> > >     "bb-layers-relative-to-this-file": [
> > 
> > Can I suggest naming it "bb-layers-local"? "bb-layers-relative-to-this-file"
> > feels bit overwhelming, I think with associated documentation users will be
> > able to figure what "local" stands for in this case.
> > 
> 
> 
> 
> ‘Local’ implies the path can be absolute, which is not supported, and it doesn’t suggest what the path should be relative to. I’d rather make it longer but clearer.

bb-layers-relative maybe?

bb-layers-relative-to-this-file is precise but does feel a bit too
unwieldy. Any other options?

Cheers,

Richard
Robert P. J. Day Nov. 6, 2025, 3:26 p.m. UTC | #4 
On Thu, 6 Nov 2025, Richard Purdie via lists.openembedded.org wrote:

> On Thu, 2025-11-06 at 13:28 +0100, Alexander Kanavin via lists.openembedded.org wrote:
> > On Thu 6. Nov 2025 at 13.11, Antonin Godard <antonin.godard@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Thu Nov 6, 2025 at 12:47 PM CET, Alexander Kanavin via lists.openembedded.org wrote:
> > > > From: Alexander Kanavin <alex@linutronix.de>
> > > >
> > > > {THISDIR} is a special value token that can be used in the list of enabled
> > > > layers to specify the layer location relative to the confguration file:
> > > > https://git.openembedded.org/bitbake/commit/?id=b3153be29de8b8570b0c184369bd41f4c646cf92
> > > >
> > > > This replaces the token with an explicit separate keyword for such layers:
> > > > so that special processing to determine the final value can be avoided, and
> > > > the feature can be formalized in the json schema:
> > > >
> > > > instead of
> > > >     "bb-layers": [
> > > >          "{THISDIR}/meta-my-project"
> > > >      ]
> > > >
> > > > this allows
> > > >     "bb-layers-relative-to-this-file": [
> > >
> > > Can I suggest naming it "bb-layers-local"? "bb-layers-relative-to-this-file"
> > > feels bit overwhelming, I think with associated documentation users will be
> > > able to figure what "local" stands for in this case.
> > >
> >
> >
> >
> > ‘Local’ implies the path can be absolute, which is not supported, and it doesn’t suggest what the path should be relative to. I’d rather make it longer but clearer.
>
> bb-layers-relative maybe?
>
> bb-layers-relative-to-this-file is precise but does feel a bit too
> unwieldy. Any other options?

  the qualifier "-to-this-file" seems implied and thus superfluous.

rday
Alexander Kanavin Nov. 6, 2025, 5:09 p.m. UTC | #5 
On Thu, 6 Nov 2025 at 16:23, Robert P. J. Day via
lists.openembedded.org <rpjday=crashcourse.ca@lists.openembedded.org>
wrote:

> > bb-layers-relative maybe?
> >
> > bb-layers-relative-to-this-file is precise but does feel a bit too
> > unwieldy. Any other options?
>
>   the qualifier "-to-this-file" seems implied and thus superfluous.

Alright. I sent a respin that uses 'bb-layers-relative'.

Alex
Peter Kjellerstedt Nov. 7, 2025, 12:59 a.m. UTC | #6 
> -----Original Message-----
> From: bitbake-devel@lists.openembedded.org <bitbake-devel@lists.openembedded.org> On Behalf Of Richard Purdie via lists.openembedded.org
> Sent: den 6 november 2025 16:16
> To: alex.kanavin@gmail.com; Antonin Godard <antonin.godard@bootlin.com>
> Cc: bitbake-devel@lists.openembedded.org; Alexander Kanavin <alex@linutronix.de>
> Subject: Re: [bitbake-devel] [PATCH] bitbake-setup: replace {THISDIR} token with an explicit keyword
> 
> On Thu, 2025-11-06 at 13:28 +0100, Alexander Kanavin via lists.openembedded.org wrote:
> > On Thu 6. Nov 2025 at 13.11, Antonin Godard <antonin.godard@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Thu Nov 6, 2025 at 12:47 PM CET, Alexander Kanavin via lists.openembedded.org wrote:
> > > > From: Alexander Kanavin <alex@linutronix.de>
> > > >
> > > > {THISDIR} is a special value token that can be used in the list of enabled
> > > > layers to specify the layer location relative to the confguration file:
> > > > https://git.openembedded.org/bitbake/commit/?id=b3153be29de8b8570b0c184369bd41f4c646cf92
> > > >
> > > > This replaces the token with an explicit separate keyword for such layers:
> > > > so that special processing to determine the final value can be avoided, and
> > > > the feature can be formalized in the json schema:
> > > >
> > > > instead of
> > > >     "bb-layers": [
> > > >          "{THISDIR}/meta-my-project"
> > > >      ]
> > > >
> > > > this allows
> > > >     "bb-layers-relative-to-this-file": [
> > >
> > > Can I suggest naming it "bb-layers-local"? "bb-layers-relative-to-this-file"
> > > feels bit overwhelming, I think with associated documentation users will be
> > > able to figure what "local" stands for in this case.
> >
> > ‘Local’ implies the path can be absolute, which is not supported, and it
> > doesn’t suggest what the path should be relative to. I’d rather make it
> > longer but clearer.
> 
> bb-layers-relative maybe?
> 
> bb-layers-relative-to-this-file is precise but does feel a bit too
> unwieldy. Any other options?
> 
> Cheers,
> 
> Richard

After I saw Alex' patch for bitbake-setup.schema.json:
https://lore.kernel.org/openembedded-core/20251106170815.2439421-1-alex.kanavin@gmail.com/T/#u
it made me realize that I had not thought that paths in both 
bb-layers and bb-layers-relative are actually relative, just to 
different locations. Unfortunately, I think the newly suggested 
name is then misleading as it implies that paths in bb-layers 
are not relative. :( I now also understand why the longer 
bb-layers-relative-to-this-file was originally proposed.

I don't have a good solution for this. The best I can come up 
with is to rename them both to, e.g., bb-layers-dir-relative and 
bb-layers-file-relative.

//Peter
Alexander Kanavin Nov. 7, 2025, 3:32 a.m. UTC | #7 
On Fri 7. Nov 2025 at 1.59, Peter Kjellerstedt <peter.kjellerstedt@axis.com>
wrote:

>
> >
> > bb-layers-relative maybe?
> >
> > bb-layers-relative-to-this-file is precise but does feel a bit too
> > unwieldy. Any other options?
> >
> > Cheers,
> >
> > Richard
>
> After I saw Alex' patch for bitbake-setup.schema.json:
>
> https://lore.kernel.org/openembedded-core/20251106170815.2439421-1-alex.kanavin@gmail.com/T/#u
> it made me realize that I had not thought that paths in both
> bb-layers and bb-layers-relative are actually relative, just to
> different locations. Unfortunately, I think the newly suggested
> name is then misleading as it implies that paths in bb-layers
> are not relative. :( I now also understand why the longer
> bb-layers-relative-to-this-file was originally proposed.
>
> I don't have a good solution for this. The best I can come up
> with is to rename them both to, e.g., bb-layers-dir-relative and
> bb-layers-file-relative


I tend to agree. I’d rather call them ‘bb-layers-in-sources’ and
‘bb-layers-in-this-dir’. Patch is coming even though I’m technically on
holiday since midnight. This is important.

Alex

>
Alexander Kanavin Nov. 7, 2025, 8:07 a.m. UTC | #8 
On Fri, 7 Nov 2025 at 04:32, Alexander Kanavin via
lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
wrote:
>> After I saw Alex' patch for bitbake-setup.schema.json:
>> https://lore.kernel.org/openembedded-core/20251106170815.2439421-1-alex.kanavin@gmail.com/T/#u
>> it made me realize that I had not thought that paths in both
>> bb-layers and bb-layers-relative are actually relative, just to
>> different locations. Unfortunately, I think the newly suggested
>> name is then misleading as it implies that paths in bb-layers
>> are not relative. :( I now also understand why the longer
>> bb-layers-relative-to-this-file was originally proposed.
>>
>> I don't have a good solution for this. The best I can come up
>> with is to rename them both to, e.g., bb-layers-dir-relative and
>> bb-layers-file-relative
>
>
> I tend to agree. I’d rather call them ‘bb-layers-in-sources’ and ‘bb-layers-in-this-dir’. Patch is coming even though I’m technically on holiday since midnight. This is important.

Patches sent:
https://lists.openembedded.org/g/bitbake-devel/message/18298
https://lists.openembedded.org/g/bitbake-devel/message/18297
https://lists.openembedded.org/g/openembedded-core/topic/patch/116167407

I also renamed the sources unpack dir from 'layers' to 'sources',
which is somewhat more invasive, but is a real improvement too. This
needed adjustments in yocto-autobuilder-helper as well, hopefully I
got most (or all!) of it right.
https://lists.yoctoproject.org/g/yocto-patches/topic/yocto_autobuilder_helper_patch/116167410

Alex
Richard Purdie Nov. 7, 2025, 9:55 a.m. UTC | #9 
On Fri, 2025-11-07 at 04:32 +0100, Alexander Kanavin wrote:
> On Fri 7. Nov 2025 at 1.59, Peter Kjellerstedt <peter.kjellerstedt@axis.com> wrote:
> > 
> > > 
> > > bb-layers-relative maybe?
> > > 
> > > bb-layers-relative-to-this-file is precise but does feel a bit too
> > > unwieldy. Any other options?
> > > 
> > > Cheers,
> > > 
> > > Richard
> > 
> > After I saw Alex' patch for bitbake-setup.schema.json:
> > https://lore.kernel.org/openembedded-core/20251106170815.2439421-1-alex.kanavin@gmail.com/T/#u
> > it made me realize that I had not thought that paths in both 
> > bb-layers and bb-layers-relative are actually relative, just to 
> > different locations. Unfortunately, I think the newly suggested 
> > name is then misleading as it implies that paths in bb-layers 
> > are not relative. :( I now also understand why the longer 
> > bb-layers-relative-to-this-file was originally proposed.
> > 
> > I don't have a good solution for this. The best I can come up 
> > with is to rename them both to, e.g., bb-layers-dir-relative and 
> > bb-layers-file-relative
> 
> I tend to agree. I’d rather call them ‘bb-layers-in-sources’ and ‘bb-
> layers-in-this-dir’. Patch is coming even though I’m technically on
> holiday since midnight. This is important.

I don't agree with "sources" in here.

I think if we call them:

bb-layers

and

bb-layers-file-relative

then this should be ok? I agree we need to get this right.

Cheers,

Richard
Alexander Kanavin Nov. 7, 2025, 10:14 a.m. UTC | #10 
On Fri, 7 Nov 2025 at 10:55, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> > I tend to agree. I’d rather call them ‘bb-layers-in-sources’ and ‘bb-
> > layers-in-this-dir’. Patch is coming even though I’m technically on
> > holiday since midnight. This is important.
>
> I don't agree with "sources" in here.
>
> I think if we call them:
>
> bb-layers
>
> and
>
> bb-layers-file-relative
>
> then this should be ok? I agree we need to get this right.

That's not too bad, I'm ok with it. Sadly I'm running out out of time,
as travel is happening today, so this last tweak is on someone else
than me :-/

Alex
Richard Purdie Nov. 7, 2025, 10:17 a.m. UTC | #11 
On Fri, 2025-11-07 at 11:14 +0100, Alexander Kanavin via lists.openembedded.org wrote:
> On Fri, 7 Nov 2025 at 10:55, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > > I tend to agree. I’d rather call them ‘bb-layers-in-sources’ and ‘bb-
> > > layers-in-this-dir’. Patch is coming even though I’m technically on
> > > holiday since midnight. This is important.
> > 
> > I don't agree with "sources" in here.
> > 
> > I think if we call them:
> > 
> > bb-layers
> > 
> > and
> > 
> > bb-layers-file-relative
> > 
> > then this should be ok? I agree we need to get this right.
> 
> That's not too bad, I'm ok with it. Sadly I'm running out out of time,
> as travel is happening today, so this last tweak is on someone else
> than me :-/

No problem, I can make this one based on one of your patches.

Cheers,

Richard
Alexander Kanavin Nov. 7, 2025, 10:20 a.m. UTC | #12 
On Fri, 7 Nov 2025 at 11:17, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:

> No problem, I can make this one based on one of your patches.

Thanks! Don't forget the json schema too (I sent patches for it, so
can be reused too :)

I'm packing a suitcase right this moment, but I will keep an eye on
emails over the next days.

Alex
Richard Purdie Nov. 7, 2025, 1:18 p.m. UTC | #13 
On Fri, 2025-11-07 at 11:20 +0100, Alexander Kanavin wrote:
> On Fri, 7 Nov 2025 at 11:17, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> 
> > No problem, I can make this one based on one of your patches.
> 
> Thanks! Don't forget the json schema too (I sent patches for it, so
> can be reused too :)

I sorted the scheme too, yes. Thanks for the reminder.

Cheers,

Richard
diff mbox series

Patch

diff --git a/bin/bitbake-setup b/bin/bitbake-setup
index 47de4654e..7e227d82d 100755
--- a/bin/bitbake-setup
+++ b/bin/bitbake-setup
@@ -129,21 +129,24 @@  def checkout_layers(layers, layerdir, d):
     return layers_fixed_revisions
 
 def setup_bitbake_build(bitbake_config, layerdir, setupdir, thisdir):
-    def _setup_build_conf(layers, build_conf_dir):
+    def _setup_build_conf(layers, relative_layers, build_conf_dir):
         os.makedirs(build_conf_dir)
         layers_s = []
+
         for l in layers:
-            if l.startswith("{THISDIR}/"):
-                if thisdir:
-                    l = l.format(THISDIR=thisdir)
-                else:
-                    raise Exception("Configuration is using {THISDIR} to specify " \
-                    "a layer path relative to itself. This can be done only " \
-                    "when the configuration is specified by its path on local " \
-                    "disk, not when it's in a registry or is fetched over http.")
-            if not os.path.isabs(l):
-                l = os.path.join(layerdir, l)
+            l = os.path.join(layerdir, l)
+            layers_s.append("  {} \\".format(l))
+
+        for l in relative_layers:
+            if thisdir:
+                l = os.path.join(thisdir, l)
+            else:
+                raise Exception("Configuration is using {THISDIR} to specify " \
+                "a layer path relative to itself. This can be done only " \
+                "when the configuration is specified by its path on local " \
+                "disk, not when it's in a registry or is fetched over http.")
             layers_s.append("  {} \\".format(l))
+
         layers_s = "\n".join(layers_s)
         bblayers_conf = """BBLAYERS ?= " \\
 {}
@@ -220,7 +223,8 @@  def setup_bitbake_build(bitbake_config, layerdir, setupdir, thisdir):
         os.rename(bitbake_confdir, backup_bitbake_confdir)
 
     if layers:
-        _setup_build_conf(layers, bitbake_confdir)
+        relative_layers = bitbake_config.get("bb-layers-relative-to-this-file") or []
+        _setup_build_conf(layers, relative_layers, bitbake_confdir)
 
     if template:
         bb.process.run("{} setup -c {} -b {} --no-shell".format(oesetupbuild, template, bitbake_builddir))
diff --git a/lib/bb/tests/setup.py b/lib/bb/tests/setup.py
index 767a6298d..83b1794a3 100644
--- a/lib/bb/tests/setup.py
+++ b/lib/bb/tests/setup.py
@@ -148,9 +148,10 @@  print("BBPATH is {{}}".format(os.environ["BBPATH"]))
                 "oe-fragments": ["test-fragment-2"]
             },
             {
-                "name": "gizmo-notemplate-with-thisdir",
-                "description": "Gizmo notemplate configuration using THISDIR",
-                "bb-layers": ["layerC","layerD/meta-layer","{THISDIR}/layerE/meta-layer"],
+                "name": "gizmo-notemplate-with-relative-layers",
+                "description": "Gizmo notemplate configuration using relative layers",
+                "bb-layers": ["layerC","layerD/meta-layer"],
+                "bb-layers-relative-to-this-file": ["layerE/meta-layer"],
                 "oe-fragments": ["test-fragment-2"]
             }
         ]
@@ -204,14 +205,13 @@  print("BBPATH is {{}}".format(os.environ["BBPATH"]))
             with open(os.path.join(bb_conf_path, 'bblayers.conf')) as f:
                 bblayers = f.read()
                 for l in bitbake_config["bb-layers"]:
-                    if l.startswith('{THISDIR}/'):
-                        thisdir_layer = os.path.join(
+                    self.assertIn(os.path.join(setuppath, "layers", l), bblayers)
+                for l in bitbake_config.get("bb-layers-relative-to-this-file") or []:
+                    relative_layer = os.path.join(
                             os.path.dirname(config_upstream["path"]),
-                            l.removeprefix("{THISDIR}/"),
+                            l,
                         )
-                        self.assertIn(thisdir_layer, bblayers)
-                    else:
-                        self.assertIn(os.path.join(setuppath, "layers", l), bblayers)
+                    self.assertIn(relative_layer, bblayers)
 
         if 'oe-fragment' in bitbake_config.keys():
             for f in bitbake_config["oe-fragments"]:
@@ -298,7 +298,7 @@  print("BBPATH is {{}}".format(os.environ["BBPATH"]))
                                                                   'gizmo-env-passthrough',
                                                                   'gizmo-no-fragment',
                                                                   'gadget-notemplate','gizmo-notemplate',
-                                                                  'gizmo-notemplate-with-thisdir')}
+                                                                  'gizmo-notemplate-with-relative-layers')}
                                }
         for cf, v in test_configurations.items():
             for c in v['buildconfigs']: