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

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

Hi Antonin,

On 10/17/24 10:44 AM, Antonin Godard wrote:
> 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:
[...]
>> 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::
> """
> 

Looks fine to me. A small nitpick here is that it's not necessarily ONLY 
for ${PN}, e.g. if we have more than one package containing 
binaries/shlibs. But at the same time, if a recipe has multiple of such 
packages, it could make sense to have their counterpart in -dev, -src, 
-dbg as well. Anyway, just a random thought but I think this is good 
enough and not really misleading :)

>>> +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).
> """
> 

Note sure the last paragraph makes a lot of sense in that context, I 
would simply remove it. I realize that I may have mislead you in my 
suggestion, it was intended to be a justification as to why I think this 
mistakes happen often, because we often have an additional variable in 
front of the path as one would see it on the target.

I would add a note to make it extra clear that ${D} should NOT be added 
to it. It's implied here by "omission" but I've seen this happen too 
many times that I would really suggest adding a note for that common 
mistake :)

[...]

>>> +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

s/pattern/patterns/

> package. At the end of package splitting, all the remaining files are stored in
> the base ``${PN}`` package.
> 

Is that true? I'm pretty sure you get a warning/build failure if a file 
is not in any package. It just happens that most common paths are in 
FILES:${PN} but I don't think there's anything making bitbake have PN a 
catch-all package?

This would also mean it'd make no sense to have a package listed on the 
right side of PN in PACKAGES, but that's a valid use-case for anything 
not matching any prior FILES:. But maybe I'm misremembering :)

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

Not sure if that isn't too advanced already for something in the 
overview-manual.

I guess we could tell the user how to find out in which package a file is?

oe-pkgdata-util find-path '/etc/foo.conf'

should do that if I remember correctly.

> """
> 
>>> +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).
> 

Fair enough.

Cheers,
Quentin