Re: [docs] [PATCH] overview-manual: concepts: add details on package splitting

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

Hi Quentin,

Thanks for the great comments :)

On Wed Oct 16, 2024 at 4:10 PM CEST, Quentin Schulz wrote:
> Hi Antonin,
>
> On 10/16/24 3:19 PM, Antonin Godard via lists.yoctoproject.org wrote:
> > The package splitting section of the overview manual currently lacks any
> > explanation of how package splitting is implemented and redirects to
> > the package class, which is not really understandable for newcomers to
> > the project.
> >
> > This patch adds a short explanation of what is done:
> >
> > * How the PACKAGES variable is defined.
> > * How the FILES variable is defined.
> > * How the two work together.
> > * How to add a custom package.
> >
> > This should give enough details to a new user on what package splitting
> > achieves and how to add a custom package.
> >
> > Adresses [YOCTO #13225]
> >
> > Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
> > ---
> >   documentation/overview-manual/concepts.rst | 49 +++++++++++++++++++++++++++---
> >   1 file changed, 44 insertions(+), 5 deletions(-)
> >
> > diff --git a/documentation/overview-manual/concepts.rst b/documentation/overview-manual/concepts.rst
> > index 62f2327a7e27a777f738523e7fafe62422186abb..d3e90b80d4fe1954c7aacba8f0fe243cba04c1b5 100644
> > --- a/documentation/overview-manual/concepts.rst
> > +++ b/documentation/overview-manual/concepts.rst
> > @@ -912,11 +912,50 @@ the analysis and package splitting process use several areas:
> >      execute on a system and it generates code for yet another machine
> >      (e.g. :ref:`ref-classes-cross-canadian` recipes).
> >
> > -The :term:`FILES` variable defines the
> > -files that go into each package in
> > -:term:`PACKAGES`. If you want
> > -details on how this is accomplished, you can look at
> > -:yocto_git:`package.bbclass </poky/tree/meta/classes-global/package.bbclass>`.
> > +Packages for a recipe are listed in the :term:`PACKAGES` variable. The
> > +:yocto_git:`meta/conf/bitbake.conf </poky/tree/meta/conf/bitbake.conf>`
> > +configuration file defines the following default list of packages::
> > +
> > +  PACKAGES = "${PN}-src ${PN}-dbg ${PN}-staticdev ${PN}-dev ${PN}-doc ${PN}-locale ${PACKAGE_BEFORE_PN} ${PN}"
> > +
> > +Each of these packages contain a default list of files defined with the
> > +:term:`FILES` variable. For example, the package ``${PN}-dev`` represents the
> > +development variant of the main package ``${PN}`` and its default list of file
>
> I misread the first time I read that because I thought "and its default
> list of files contains development-oriented files" applied to "${PN}"
> and not "${PN}-dev". I could suggest simply adding a coma:
>
> """
> the package ``${PN}-dev`` represents the development variant of the main
> package ``${PN}``, and its default list
> """
>
> Just a suggestion though, could be that it's just me reading it this way :)

Right, I feel like the comma makes sense too.

> s/file/files/ in any case.

+1

> Additionally, I think PN-dev does not represent the development variant
> of the main package, it represents the files that are useful for
> developing an application depending on PN main package. Using "variant"
> seems to indicate that PN-dev could be self-sufficient and potentially
> have the same binaries/shlibs than in PN but compiled differently.

That's correct, 'variant' is misleading here. In the end, what about:

"""
For example, the package ``${PN}-dev`` represents files useful to the
development of applications depending on ``${PN}``. The default list of files
for ``${PN}-dev``, also defined in :oe_git:`meta/conf/bitbake.conf
</openembedded-core/tree/meta/conf/bitbake.conf>`, is defined as follows::
"""

> > +contains development-oriented files only::
> > +
> > +  FILES:${PN}-dev = "${includedir} ${FILES_SOLIBSDEV} ${libdir}/*.la \
> > +                  ${libdir}/*.o ${libdir}/pkgconfig ${datadir}/pkgconfig \
> > +                  ${datadir}/aclocal ${base_libdir}/*.o \
> > +                  ${libdir}/${BPN}/*.la ${base_libdir}/*.la \
> > +                  ${libdir}/cmake ${datadir}/cmake"
> > +
> > +The paths in this list must be *absolute*, and must use the path prefixes
>
> There is a very important piece of information that needs to be added
> here and a source of many newcomers' mistake: these are absolute paths
> from the PoV of the root of the filesystem on the target, NOT from bitbake.
>
> Indeed, in do_install (and do_package, etc.) we need to prefix our paths
> with $D (and $PKGD/$PKGDEST) when installing/packaging files, however,
> FILES doesn't not need those, even more it should NOT have those.

You're right, will change to:

"""
The paths must use the path prefixes defined by the
:oe_git:`meta/conf/bitbake.conf </openembedded-core/tree/meta/conf/bitbake.conf>`
configuration file, such as ``${sysconfdir}``, ``${bindir}``, etc.

The paths in this list must be *absolute* paths from the point of view of the
root filesystem on the target. For example, assuming the following path is
added to the :term:`FILES` variable::

  ${sysconfdir}/foo.conf

In a later phase of the build system, the created package will be used to
install the file ``/etc/foo.conf`` on the target, since ``${sysconfdir}``
equals to ``/etc`` (unless explicitely overwritten).
"""

> > +defined by the :yocto_git:`bitbake.conf </poky/tree/meta/conf/bitbake.conf>`
>
> use oe_git here instead since bitbake is maintained by OE folks.

+1

> > +configuration file, such as ``${sysconfdir}``, ``${bindir}``, etc. They can
> > +optionally use wildcards (``*`` ) to match multiple files installed in a
> > +directory.
> > +
> > +.. note::
> > +
> > +   The list of files for a package is defined using the override syntax by
> > +   separating :term:`FILES` and the package name by a semi-colon (``:``).
> > +
> > +The order of the packages in the :term:`PACKAGES` is important: during package
> > +splitting, going from the first element in :term:`PACKAGES` to the last, each
>
> I would say leftmost to rightmost as we have some other variables that
> go rightmost to leftmost (hello OVERRIDES :) ).

+1

> > +file matching a pattern specified in the corresponding :term:`FILES` definition
> > +will be included in the package and *excluded* for the remaining packages listed
> > +in :term:`PACKAGES`. As a consequence, custom packages should be added using the
>
> I could suggest:
> """
> A given file can only ever be in one package. Therefore, the first (from
> leftmost to rightmost in :term:`PACKAGES`) package for which the given
> file matches a path from its :term:`FILES` will contain the given file.
> """
>
> Still a bit too complex of a sentence but couldn't come up with anything
> better for now.

How about:

"""
A given file can only ever be in one package. By iterating from the
leftmost to rightmost package in :term:`PACKAGES`, each file matching one of the
pattern defined in the corresponding :term:`FILES` definition is included in the
package. At the end of package splitting, all the remaining files are stored in
the base ``${PN}`` package.

The result of package splitting is stored in the :term:`PKGDEST` directory, and
contains one directory per package.
"""

> > +prepend operator (``=+``) to be prioritized.
> > +
>
> I am not sure it is wise to say "prepend" operator as we have three ways
> of prepending:
> - =+
> - =.
> - :prepend
>
> But since you explicit which one you're talking about, I guess it's
> fine? The bitbake docs only says "=+" operator apparently:
> https://docs.yoctoproject.org/bitbake/bitbake-user-manual/bitbake-user-manual-metadata.html#appending-and-prepending-with-spaces

I think using the "prepend operator" + showing it in parenthesis makes sense
here, as newcomers may not be aware that it is used for prepending (there is no
explanation on operators in this document).

> Also nitpicking, but it's not necessary that a package be added with =+,
> e.g. if it should contain files that aren't matched by any other
> path/pattern in other FILES. But doesn't hurt to say to always do it.

Although you're correct, I would say let's keep it simple here since it's
already confusing enough for newcomers :)

> > +For example, to add a custom package variant of the ``${PN}`` recipe named
> > +``${PN}-extra`` (name is arbitrary), one should write::
> > +
> > +  PACKAGES =+ "${PN}-extra"
> > +
> > +Alternatively, a custom package can be added to the
> > +:term:`PACKAGE_BEFORE_PN` variable::
> > +
> > +  PACKAGE_BEFORE_PN += "${PN}-extra"
> >
>
> I would more recommend people to use PACKAGE_BEFORE_PN than the PACKAGES
> =+ scheme. The reason is quite simple, most people are used to += and
> using that instead of =+ could easily be missed by reviewers.
>
> The former is only required if you need to have files typically in any
> of ${PN}-src ${PN}-dbg ${PN}-staticdev ${PN}-dev ${PN}-doc ${PN}-locale,
> in another package.
>
> That's my opinion though and both are correct (and both should be
> mentioned!), so that would be fine to keep as is.

Absolutely, I will swap PACKAGE_BEFORE_PN and the =+ technique, so the most
important is introduced first.

Cheers,
Antonin

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