Re: [docs] [PATCH 4/5] ref-manual: variables: add SIGGEN_LOCKEDSIGS* variables

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

Hi Julien,

On top of Ulrich's comments, here are my suggestions/corrections:

On Thu Oct 31, 2024 at 5:56 PM CET, Julien Stephan wrote:
> Variables SIGGEN_LOCKEDSIGS, SIGGEN_LOCKEDSIGS_TASKSIG_CHECK and
> SIGGEN_LOCKEDSIGS_TYPES are used to lock specific tasks to specific
> signatures. They are used by bitbake -S <lockedsigs> and bblock, so add
> documentation for them.
>
> Signed-off-by: Julien Stephan <jstephan@baylibre.com>
> ---
>  documentation/ref-manual/variables.rst | 33 +++++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index 07ed143ac5606617c720301ac6621355a341f90a..8cfb978e795e43e3ef25f3e18e11baf7c2a28d54 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -7880,6 +7880,39 @@ system and gives an overview of their function and contents.
>           might break at runtime if the interface of the recipe was changed
>           after the other had been built.
>  
> +   :term:`SIGGEN_LOCKEDSIGS`
> +     The list of locked taks, with the form::
> +
> +       SIGGEN_LOCKEDSIGS += "<package>:<task>:<signature>"
> +
> +     If ``<signature>`` exists for the specified ``<task>`` and ``<package>``

How would I find such a signature by hand? Maybe worth explaining it here?

> +     in sstate, BitBake will use the cached output instead of rebuilding the
> +     ``<task>``. If it does not exist, BitBake will build the ``<task>`` and
> +     the sstate will be used next time.

Could you give a real-life example of that at the end here?

> +   :term:`SIGGEN_LOCKEDSIGS_TASKSIG_CHECK`
> +     Specifies the debug level of task signature check. 3 levels are supported:
> +
> +     * info: displays a "Note" message to remind user that a task is locked
> +       and current signature matches the locked one.
> +     * warn: displays a "Warning" message if a task is locked and current
> +       signature does not match the locked one.
> +     * error: same as warn but displays an “Error” message and abort.

For "info", "warn" and "error" could you enclose that in double ticks? ``info``
and so on.

> +   :term:`SIGGEN_LOCKEDSIGS_TYPES`
> +     Allowed overrides for :term:`SIGGEN_LOCKEDSIGS`. This is mainly used
> +     for achitecture specific lock. A common value for :term:`SIGGEN_LOCKEDSIGS_TYPES`

s/achitecture/architecture/

By "mainly", do you mean "only" or could there by anything other than
architectures covered by the variable?

> +     is ``${PACKAGE_ARCHS}``::
> +
> +       SIGGEN_LOCKEDSIGS_TYPES += "${PACKAGE_ARCHS}"
> +
> +       SIGGEN_LOCKEDSIGS_core2-64 += "bc:do_compile:abcd"
> +       SIGGEN_LOCKEDSIGS_cortexa57 += "bc:do_compile:efgh"

Maybe real hashes would be better here

> +
> +     Here, the ``do_compile`` task from ``bc`` will be locked only for
> +     ``x86-64`` and ``arm64`` but not for other architectures such as

Wouldn't it be locked for ``core2-64`` and ``cortexa57`` more specifically? I
might misunderstand something here. :)

> +     ``qemumips``.
> +
>     :term:`SITEINFO_BITS`
>        Specifies the number of bits for the target system CPU. The value
>        should be either "32" or "64".

Also I could suggest splitting the series in patches for sphinx-lint/vale and
patches for documentation of variables/bblock.

Thanks!
Antonin

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