Re: [docs] [PATCH 2/2] dev-manual: document how to provide confs from layer.conf

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Antonin,

On 10/30/24 10:23 AM, Antonin Godard via lists.yoctoproject.org wrote:
> Add a section on providing global level configuration from the
> layer.conf file. Since this file is parsed at an earlier stage in the
> parsing process, it's not possible to combine bb.utils.contains and
> {DISTRO,MACHINE}_FEATURES to conditionally some configurations.
> 
> This patch documents:
> 
> - First that this file can be used for providing such configuration.
> - Then demonstrates how to conditionally provide them, using a technique
>    that is currently used in meta-virtualization
>    (https://git.yoctoproject.org/meta-virtualization/tree/conf/layer.conf#n50).
> 
> Fixes [YOCTO #12688].
> 
> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
> ---
>   documentation/dev-manual/layers.rst | 78 ++++++++++++++++++++++++++++++++++++-
>   1 file changed, 77 insertions(+), 1 deletion(-)
> 
> diff --git a/documentation/dev-manual/layers.rst b/documentation/dev-manual/layers.rst
> index 91889bd0ae391c7b6e62068778ae5c180c840762..94d959fde67e911435d99ce8dd85b3d591573c82 100644
> --- a/documentation/dev-manual/layers.rst
> +++ b/documentation/dev-manual/layers.rst
> @@ -644,6 +644,83 @@ variable and append the layer's root name::
>      order of ``.conf`` or ``.bbclass`` files. Future versions of BitBake
>      might address this.
>   
> +Providing Global-level Configurations With Your Layer
> +-----------------------------------------------------
> +
> +When creating a layer, you may need to define configurations that should take
> +effect globally in your build environment when it is part of the
> +:term:`BBLAYERS` variable. The ``layer.conf`` file is a :term:`configuration

"when it is part of the BBLAYERS variable" is a complicated way of 
saying "when it is used"/"when it is part of the build setup". Maybe we 
can find something more explicit?

I would also suggest adding "in the new layer" after ``layer.conf`` to 
explicit the location of layer.conf (maybe we should even add it's in 
conf/?). We have many different possible locations for configuration 
files so I feel like we should tell people where exactly to expect 
files. (e.g. local.conf, distro conf file, machine conf file, 
(multiconfig?), bblayers.conf, are all expected in different locations).

> +file` that affects the build system globally, so it is a candidate for this
> +use-case.
> +
> +For example, if your layer provides an alternative to the linux kernel named

s/provides an alternative to the Linux kernel/provides a Linux kernel 
recipe/

s/linux/Linux/

> +``linux-custom``, you may want to define the :term:`PREFERRED_PROVIDER` for the
> +``linux-custom`` recipe::
> +
> +  PREFERRED_PROVIDER_virtual/kernel = "linux-custom"
> +
> +This can be defined in the ``layer.conf`` file. If your layer has a higher
> +:term:`BBFILE_PRIORITY` value, it will take precedence over previous definitions
> +of the same ``PREFERRED_PROVIDER_virtual/kernel`` assignments.
> +

I seem to recall this is the perfect example of what we do NOT want in a 
layer.conf though (might be bad recollection though :) ). I actually do 
not even know what'd be a valid reason to have variables directly 
defined in layer.conf (I see you're providing an example in the next 
section, but do we really need this section to provide an example of 
something the user shouldn't be doing?).

I would add a very big note/warning at the end of this section saying 
that this is almost always wrong as it is mostly expected that adding a 
layer should only minimally change the build output. And that for this 
very example, it should be in a machine or distro configuration file 
instead.

I am not sure that BBFILE_PRIORITY is applicable for this as what 
matters is the parsing order of all layer.conf files from entries in 
BBLAYERS. I believe it's just parsed in the order it appears in 
BBLAYERS? Last one has priority (because we use = and not ?=). If that 
is actually working, we should then expand 
https://docs.yoctoproject.org/ref-manual/variables.html#term-BBFILE_PRIORITY 
as well because it clearly only defines priority for recipes.

> +Conditionally Provide Global-level Configurations With Your Layer
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +In some cases, your layer may provide global configurations only if some
> +features it provides are enabled. Since the ``layer.conf`` file is parsed at an
> +earlier stage in the parsing process, the :term:`DISTRO_FEATURES` and
> +:term:`MACHINE_FEATURES` variables are not yet available to ``layer.conf``, and
> +declaring conditional assignments based on these variables is not possible. The
> +following technique shows a way to leverage this limitation by using the
> +:term:`USER_CLASSES` variable and a conditional ``include`` command.
> +
> +In the following steps, let's assume our layer is named ``meta-mylayer`` and
> +that this layer defines a custom :ref:`distro feature <ref-features-distro>`
> +named ``mylayer-kernel``. We will set the :term:`PREFERRED_PROVIDER` variable
> +for the kernel only if our feature ``mylayer-kernel`` is part of the
> +:term:`DISTRO_FEATURES`:
> +
> +#. Create an include file in the directory
> +   ``meta-mylayer/conf/distro/include/``, for example a file named
> +   ``mylayer-kernel-provider.inc`` that sets the kernel provider to
> +   ``linux-custom``::
> +
> +     PREFERRED_PROVIDER_virtual/kernel = "linux-custom"

Random thought but maybe we should have align this with three 
whitespaces starting from the first character in the list. Here it is 
only one and difficult to notice at first sight. Actually, it's two 
whitespaces, I think that proves my point :D

> +
> +#. Provide a path to this include file in your ``layer.conf``::
> +
> +     META_MYLAYER_KERNEL_PROVIDER_PATH = "${LAYERDIR}/conf/distro/include/mylayer-kernel-provider.inc"
> +
> +#. Create a new class in ``meta-mylayer/classes-global/``, for example a class
> +   ``meta-mylayer-cfg.bbclass``. It Conditionally includes the file

s/Conditionally/conditionally/

Maybe say "Make it conditionally include the file" instead, as the way 
it's phrased could mean that because it's named this way or/and in that 
special directory that it magically includes the file, instead of 
telling the user what to put in that file.

> +   ``mylayer-kernel-provider.inc`` defined above, using the variable
> +   ``META_MYLAYER_KERNEL_PROVIDER_PATH`` defined in ``layer.conf``::
> +
> +     include ${@bb.utils.contains('DISTRO_FEATURES', 'mylayer-kernel', '${META_MYLAYER_KERNEL_PROVIDER_PATH}', '', d)}
> +
> +   For details on the ``bb.utils.contains`` function, see its definition in
> +   :bitbake_git:`lib/bb/utils.py </tree/lib/bb/utils.py>`.
> +
> +   .. note::
> +
> +      The ``include`` command is designed to not fail if the function
> +      ``bb.utils.contains`` returns an empty string.

I believe `require` too and we very likely want to use require here to 
make sure the path doesn't have a typo :)

Also, I'm wondering if this isn't actually "breaking" the sstate cache 
as this META_MYLAYER_KERNEL_PROVIDER_PATH would be different on each 
machine so people wouldn't be able to reuse the sstate cache.

Building with the exact same setup - and a shared SSTATE_DIR - in two 
different directories on the same machine should answer that question :)

> +
> +#. Back to your ``layer.conf`` file, add the class ``meta-mylayer-cfg`` class to
> +   the :term:`USER_CLASSES` variable::
> +
> +     USER_CLASSES:append = " meta-mylayer-cfg"
> +
> +   This will add the class ``meta-mylayer-cfg`` to the list of classes to
> +   globally inherit. Since the ``include`` command is conditional in
> +   ``meta-mylayer-cfg.bbclass``, even though inherited the class will have no
> +   effect until the feature ``mylayer-kernel`` is enabled through
> +   :term:`DISTRO_FEATURES`.
> +
> +This technique can also be used for :ref:`Machine features
> +<ref-features-machine>` by following the same steps, the only difference being
> +to place the include file in ``meta-mylayer/conf/machine/include/`` instead.
> +

I don't think the path needs to be specific?

>   Managing Layers
>   ===============
>   
> @@ -916,4 +993,3 @@ above:
>   .. note::
>      Both the ``create-layers-setup`` and the ``setup-layers`` provided several additional options
>      that customize their behavior - you are welcome to study them via ``--help`` command line parameter.
> -

Not sure this line deletion makes sense in this patch?

Cheers,
Quentin