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

["Antonin Godard" <antonin.godard@bootlin.com>]

Hi Quentin,

On Wed Nov 13, 2024 at 10:58 AM CET, Quentin Schulz wrote:
> Hi Antonin,
>
> On 11/12/24 12:32 PM, Antonin Godard 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 set some configurations.
>> 
>> This patch documents:
>> 
>> - First that this file can be used for providing such configuration.
>> - Then demonstrate 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 | 86 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 86 insertions(+)
>> 
>> diff --git a/documentation/dev-manual/layers.rst b/documentation/dev-manual/layers.rst
>> index 91889bd0ae391c7b6e62068778ae5c180c840762..e449fd43ad86c9a027e317e49b6e2f2f340a08d4 100644
>> --- a/documentation/dev-manual/layers.rst
>> +++ b/documentation/dev-manual/layers.rst
>> @@ -644,6 +644,92 @@ 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 the layer is part of the build.
>> +The ``layer.conf`` file is a :term:`configuration file` that affects the build
>> +system globally, so it is a candidate for this use-case.
>> +
>> +.. attention::
>
> I would recommend using .. note or .. warning admonitions over other ones.
>
> Right now it seems we have mainly .. note, but also a handful of .. 
> admonition and a couple of .. tip so maybe that's not the best advice.
>
> Anyway, why this suggestion?
> https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#directives
> """
> Admonitions: attention, caution, danger, error, hint, important, note, 
> tip, warning and the generic admonition. (Most themes style only “note” 
> and “warning” specially.)
> """

Thanks for the tip to the doc. I'll replace that with a warning instead.
Also there was a usage of `.. attention` for the bblock patches, I'll send a
patch to replace that.

>> +
>> +   Providing unconditional global level configuration from the ``layer.conf``
>> +   file is *not* a good practice, and should be avoided. For this reason, the
>> +   section :ref:`Conditionally Provide Global-level Configurations With Your
>> +   Layer` below shows how the ``layer.conf`` file can be used to provide
>> +   configurations only if a certain condition is met.
>> +
>> +For example, if your layer provides a Linux kernel recipe named
>> +``linux-custom``, you may want to define the :term:`PREFERRED_PROVIDER` for the
>> +``linux-custom`` recipe::
>
> """
> you may want to make :term:`PREFERRED_PROVIDER` for the Linux kernel 
> virtual provider point to the ``linux-custom`` recipe.
> """
>
> Because we are not setting PREFERRED_PROVIDER **for** the linux-custom 
> recipe, we want to have virtual/kernel point at linux-custom (and it's 
> set outside of that recipe). Moreover, the content of PREFERRED_PROVIDER 
> for the linux-custom recipe isn't really of importance, it's more for 
> other recipes depending on *a* kernel (e.g. kernel-modules, image 
> recipes, etc...).

That's correct, I get exactly what you mean and re-reading my sentence with that
in mind does bother me too. :)

> Not happy about the way I phrased it so maybe you can suggest something 
> better here?

How about simply:

"""
you may want to make :term:`PREFERRED_PROVIDER_virtual/kernel
<PREFERRED_PROVIDER>` point to ``linux-custom``:
"""

>> +
>> +  PREFERRED_PROVIDER_virtual/kernel = "linux-custom"
>> +
>> +This can be defined in the ``layer.conf`` file. If your layer is at the last
>> +position in the :term:`BBLAYERS` list, it will take precedence over previous
>> +``PREFERRED_PROVIDER_virtual/kernel`` assignments.
>> +
>
> Mmmmm, except if the machine configuration file sets it too with "="?

Also from local.conf, etc. I guess so what about:

"""
it will take precedence over previous ``PREFERRED_PROVIDER_virtual/kernel``
assignments (unless it is set from another :term:`configuration file` such as
machine configuration files).
"""

>> +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 bypass 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"
>> +
>> +#. 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``. Make it conditionally include the file
>> +   ``mylayer-kernel-provider.inc`` defined above, using the variable
>> +   ``META_MYLAYER_KERNEL_PROVIDER_PATH`` defined in ``layer.conf``::
>> +
>> +      require ${@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 ``require`` command is designed to not fail if the function
>> +      ``bb.utils.contains`` returns an empty string.
>> +
>> +#. 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`.
>> +
>
> Since you go to such great lengths to explain how to create the bbclass, 
> require it conditionally, etc... Maybe provide how to "enable through 
> DISTRO_FEATURES" so we have the complete picture?

Good idea!

>> +This technique can also be used for :ref:`Machine features
>
> s/for/with/ ?

+1, thanks :)

> Looks good otherwise :)
>
> Cheers,
> Quentin

Thank you!
Antonin

-- 
Antonin Godard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com