[yocto-docs,v3] ref-manual: classes: fix bin_package description

The previous bin_package description was confusing: it would instruct to
use the git fetcher to extract the content of an RPM package using the
`subpath` option - but that's not possible as the git fetcher can be
used to clone a repository but not to do the extraction.

Update the description by telling what it really does and what it
doesn't do, and by giving an HTTPS+RPM example.

Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
---
Changes in v3:
- Actually explain what bin_package does and what it doesn't do.
- Link to v2: https://lore.kernel.org/r/20241120-fix-bin-package-v2-1-917a5c2745d2@bootlin.com

Changes in v2:
- Instead of updating the example, update the description of the class
  with a more common (and working) example usage of the class.
- Link to v1: https://lore.kernel.org/r/20241118-fix-bin-package-v1-1-906f0148fdaa@bootlin.com
---
 documentation/ref-manual/classes.rst | 42 +++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)


---
base-commit: 4175839e718db49bf6971e900c1cf176d03458d7
change-id: 20241115-fix-bin-package-eed7633fbecd

Best regards,

Hi Antonin,

On 11/27/24 12:36 PM, Antonin Godard wrote:
> The previous bin_package description was confusing: it would instruct to
> use the git fetcher to extract the content of an RPM package using the
> `subpath` option - but that's not possible as the git fetcher can be
> used to clone a repository but not to do the extraction.
> 
> Update the description by telling what it really does and what it
> doesn't do, and by giving an HTTPS+RPM example.
> 
> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
> ---
> Changes in v3:
> - Actually explain what bin_package does and what it doesn't do.
> - Link to v2: https://lore.kernel.org/r/20241120-fix-bin-package-v2-1-917a5c2745d2@bootlin.com
> 
> Changes in v2:
> - Instead of updating the example, update the description of the class
>    with a more common (and working) example usage of the class.
> - Link to v1: https://lore.kernel.org/r/20241118-fix-bin-package-v1-1-906f0148fdaa@bootlin.com
> ---
>   documentation/ref-manual/classes.rst | 42 +++++++++++++++++++++---------------
>   1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
> index b92f4e4f20ea8f702c90f4e3d29251b2461d07d0..ff9979d1f1ac0afb0ee59fcd193dec2c1323e1fe 100644
> --- a/documentation/ref-manual/classes.rst
> +++ b/documentation/ref-manual/classes.rst
> @@ -159,27 +159,35 @@ software that includes bash-completion data.
>   ``bin_package``
>   ===============
>   
> -The :ref:`ref-classes-bin-package` class is a helper class for recipes that extract the
> -contents of a binary package (e.g. an RPM) and install those contents
> -rather than building the binary from source. The binary package is
> -extracted and new packages in the configured output package format are
> -created. Extraction and installation of proprietary binaries is a good
> -example use for this class.
> +The :ref:`ref-classes-bin-package` class is a helper class for recipes that

Would put a comma after recipes so people don't think it's for recipes 
that extract the contents[...] but rather that the class does it for the 
recipes inheriting it.

> +disables the :ref:`ref-tasks-configure` and :ref:`ref-tasks-compile` tasks and
> +copies the content of the :term:`S` directory into the :term:`D` directory. This
> +is useful for installing binary packages (e.g. RPM packages) by passing the
> +package in the :term:`SRC_URI` variable and inheriting this class.
>   

I would also provide tarballs as another example of "binary packages", 
not sure how to word this though.

> -.. note::
> +For RPMs and other packages that do not contain a subdirectory, you should set
> +the :term:`SRC_URI` option ``subdir`` to :term:`BP` so that the contents are
> +extracted to the directory expected by the default value of :term:`S`. For
> +example::
> +
> +   SRC_URI = "https://example.com/downloads/somepackage.rpm;subdir=${BP}"
>   
> -   For RPMs and other packages that do not contain a subdirectory, you
> -   should specify an appropriate fetcher parameter to point to the
> -   subdirectory. For example, if BitBake is using the Git fetcher (``git://``),
> -   the "subpath" parameter limits the checkout to a specific subpath
> -   of the tree. Here is an example where ``${BP}`` is used so that the files
> -   are extracted into the subdirectory expected by the default value of
> -   :term:`S`::
> +This class assumes that the content of :term:`S` already contains the

"""
This class assumes that the content of the package as installed in 
:term:`S` mirrors the expected layout once installed on the target, 
which is generally[...]
"""

?

> +installation paths on the target , which is generally the case for binary

Spurious whitespace before comma.

> +packages. For example, a RPM package containing a library should already contain

s/a RPM/an RPM/

s/containing a library/for a library/ # to avoid redundant "contain" verb

s/should already contain/would usually contain/

> +the ``usr/lib`` directories, and should be extracted as

s/as/to/?

> +``${S}/usr/lib/<library>.so`` to be installed in :term:`D` correctly.
> +

It'd be nice to encourage people to use versioned shared libraries even 
for binary packages but not sure how to word this here.

> +.. note::
>   
> -      SRC_URI = "git://example.com/downloads/somepackage.rpm;branch=main;subpath=${BP}"
> +   The extraction of the package passed in :term:`SRC_URI` is not handled by the
> +   :ref:`ref-classes-bin-package` class, and is done by the appropriate
> +   :ref:`fetcher <bitbake-user-manual/bitbake-user-manual-fetching:fetchers>`
> +   depending on the nature of the package.

What do you mean by "depending on the nature of the package"?

>   
> -   See the ":ref:`bitbake-user-manual/bitbake-user-manual-fetching:fetchers`" section in the BitBake User Manual for
> -   more information on supported BitBake Fetchers.
> +   See the ":ref:`bitbake-user-manual/bitbake-user-manual-fetching:fetchers`"
> +   section in the BitBake User Manual for more information about supported
> +   BitBake Fetchers.
>   

I'm not entirely sure this is relevant here? Or maybe we should update 
the BitBake docs to reflect which fetcher is extracting 
packages/tarballs with which option?

Cheers,
Quentin

Hi Quentin,

On Wed Nov 27, 2024 at 1:36 PM CET, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Antonin,
>
> On 11/27/24 12:36 PM, Antonin Godard wrote:
>> The previous bin_package description was confusing: it would instruct to
>> use the git fetcher to extract the content of an RPM package using the
>> `subpath` option - but that's not possible as the git fetcher can be
>> used to clone a repository but not to do the extraction.
>> 
>> Update the description by telling what it really does and what it
>> doesn't do, and by giving an HTTPS+RPM example.
>> 
>> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
>> ---
>> Changes in v3:
>> - Actually explain what bin_package does and what it doesn't do.
>> - Link to v2: https://lore.kernel.org/r/20241120-fix-bin-package-v2-1-917a5c2745d2@bootlin.com
>> 
>> Changes in v2:
>> - Instead of updating the example, update the description of the class
>>    with a more common (and working) example usage of the class.
>> - Link to v1: https://lore.kernel.org/r/20241118-fix-bin-package-v1-1-906f0148fdaa@bootlin.com
>> ---
>>   documentation/ref-manual/classes.rst | 42 +++++++++++++++++++++---------------
>>   1 file changed, 25 insertions(+), 17 deletions(-)
>> 
>> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
>> index b92f4e4f20ea8f702c90f4e3d29251b2461d07d0..ff9979d1f1ac0afb0ee59fcd193dec2c1323e1fe 100644
>> --- a/documentation/ref-manual/classes.rst
>> +++ b/documentation/ref-manual/classes.rst
>> @@ -159,27 +159,35 @@ software that includes bash-completion data.
>>   ``bin_package``
>>   ===============
>>   
>> -The :ref:`ref-classes-bin-package` class is a helper class for recipes that extract the
>> -contents of a binary package (e.g. an RPM) and install those contents
>> -rather than building the binary from source. The binary package is
>> -extracted and new packages in the configured output package format are
>> -created. Extraction and installation of proprietary binaries is a good
>> -example use for this class.
>> +The :ref:`ref-classes-bin-package` class is a helper class for recipes that
>
> Would put a comma after recipes so people don't think it's for recipes 
> that extract the contents[...] but rather that the class does it for the 
> recipes inheriting it.

+1

>> +disables the :ref:`ref-tasks-configure` and :ref:`ref-tasks-compile` tasks and
>> +copies the content of the :term:`S` directory into the :term:`D` directory. This
>> +is useful for installing binary packages (e.g. RPM packages) by passing the
>> +package in the :term:`SRC_URI` variable and inheriting this class.
>>   
>
> I would also provide tarballs as another example of "binary packages", 
> not sure how to word this though.

I'll add another example afterwards.

>> -.. note::
>> +For RPMs and other packages that do not contain a subdirectory, you should set
>> +the :term:`SRC_URI` option ``subdir`` to :term:`BP` so that the contents are
>> +extracted to the directory expected by the default value of :term:`S`. For
>> +example::
>> +
>> +   SRC_URI = "https://example.com/downloads/somepackage.rpm;subdir=${BP}"
>>   
>> -   For RPMs and other packages that do not contain a subdirectory, you
>> -   should specify an appropriate fetcher parameter to point to the
>> -   subdirectory. For example, if BitBake is using the Git fetcher (``git://``),
>> -   the "subpath" parameter limits the checkout to a specific subpath
>> -   of the tree. Here is an example where ``${BP}`` is used so that the files
>> -   are extracted into the subdirectory expected by the default value of
>> -   :term:`S`::
>> +This class assumes that the content of :term:`S` already contains the
>
> """
> This class assumes that the content of the package as installed in 
> :term:`S` mirrors the expected layout once installed on the target, 
> which is generally[...]
> """
>
> ?

I agree, better wording

>> +installation paths on the target , which is generally the case for binary
>
> Spurious whitespace before comma.

+1

>> +packages. For example, a RPM package containing a library should already contain
>
> s/a RPM/an RPM/
>
> s/containing a library/for a library/ # to avoid redundant "contain" verb
>
> s/should already contain/would usually contain/

+1

>> +the ``usr/lib`` directories, and should be extracted as
>
> s/as/to/?

+1

>> +``${S}/usr/lib/<library>.so`` to be installed in :term:`D` correctly.
>> +
>
> It'd be nice to encourage people to use versioned shared libraries even 
> for binary packages but not sure how to word this here.

Replace by ``${S}/usr/lib/<library>.<version>.so``? and implicitly encourage it.
Not sure this is the place to give written advice on that.

>> +.. note::
>>   
>> -      SRC_URI = "git://example.com/downloads/somepackage.rpm;branch=main;subpath=${BP}"
>> +   The extraction of the package passed in :term:`SRC_URI` is not handled by the
>> +   :ref:`ref-classes-bin-package` class, and is done by the appropriate
>> +   :ref:`fetcher <bitbake-user-manual/bitbake-user-manual-fetching:fetchers>`
>> +   depending on the nature of the package.
>
> What do you mean by "depending on the nature of the package"?

I meant the type, like RPM, tarball, deb… I can replace by "type", and add a
"(RPM, Deb, tarball, …)" in parenthesis afterwards.

>>   
>> -   See the ":ref:`bitbake-user-manual/bitbake-user-manual-fetching:fetchers`" section in the BitBake User Manual for
>> -   more information on supported BitBake Fetchers.
>> +   See the ":ref:`bitbake-user-manual/bitbake-user-manual-fetching:fetchers`"
>> +   section in the BitBake User Manual for more information about supported
>> +   BitBake Fetchers.
>>   
>
> I'm not entirely sure this is relevant here? Or maybe we should update 
> the BitBake docs to reflect which fetcher is extracting 
> packages/tarballs with which option?

Since I mentioned the fetchers above I thought it would be nice to keep, but the
link is already present in the sentence above so I'll remove this paragraph.

Thank you!
Antonin

Hi Antonin,

On 11/27/24 2:37 PM, Antonin Godard wrote:
> Hi Quentin,
> 
> On Wed Nov 27, 2024 at 1:36 PM CET, Quentin Schulz via lists.yoctoproject.org wrote:
>> Hi Antonin,
>>
>> On 11/27/24 12:36 PM, Antonin Godard wrote:
>>> The previous bin_package description was confusing: it would instruct to
>>> use the git fetcher to extract the content of an RPM package using the
>>> `subpath` option - but that's not possible as the git fetcher can be
>>> used to clone a repository but not to do the extraction.
>>>
>>> Update the description by telling what it really does and what it
>>> doesn't do, and by giving an HTTPS+RPM example.
>>>
>>> Signed-off-by: Antonin Godard <antonin.godard@bootlin.com>
>>> ---
>>> Changes in v3:
>>> - Actually explain what bin_package does and what it doesn't do.
>>> - Link to v2: https://lore.kernel.org/r/20241120-fix-bin-package-v2-1-917a5c2745d2@bootlin.com
>>>
>>> Changes in v2:
>>> - Instead of updating the example, update the description of the class
>>>     with a more common (and working) example usage of the class.
>>> - Link to v1: https://lore.kernel.org/r/20241118-fix-bin-package-v1-1-906f0148fdaa@bootlin.com
>>> ---
>>>    documentation/ref-manual/classes.rst | 42 +++++++++++++++++++++---------------
>>>    1 file changed, 25 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/documentation/ref-manual/classes.rst b/documentation/ref-manual/classes.rst
>>> index b92f4e4f20ea8f702c90f4e3d29251b2461d07d0..ff9979d1f1ac0afb0ee59fcd193dec2c1323e1fe 100644
>>> --- a/documentation/ref-manual/classes.rst
>>> +++ b/documentation/ref-manual/classes.rst
>>> @@ -159,27 +159,35 @@ software that includes bash-completion data.
>>>    ``bin_package``
>>>    ===============
>>>    
>>> -The :ref:`ref-classes-bin-package` class is a helper class for recipes that extract the
>>> -contents of a binary package (e.g. an RPM) and install those contents
>>> -rather than building the binary from source. The binary package is
>>> -extracted and new packages in the configured output package format are
>>> -created. Extraction and installation of proprietary binaries is a good
>>> -example use for this class.
>>> +The :ref:`ref-classes-bin-package` class is a helper class for recipes that
>>
>> Would put a comma after recipes so people don't think it's for recipes
>> that extract the contents[...] but rather that the class does it for the
>> recipes inheriting it.
> 
> +1
> 
>>> +disables the :ref:`ref-tasks-configure` and :ref:`ref-tasks-compile` tasks and
>>> +copies the content of the :term:`S` directory into the :term:`D` directory. This
>>> +is useful for installing binary packages (e.g. RPM packages) by passing the
>>> +package in the :term:`SRC_URI` variable and inheriting this class.
>>>    
>>
>> I would also provide tarballs as another example of "binary packages",
>> not sure how to word this though.
> 
> I'll add another example afterwards.
> 
>>> -.. note::
>>> +For RPMs and other packages that do not contain a subdirectory, you should set
>>> +the :term:`SRC_URI` option ``subdir`` to :term:`BP` so that the contents are
>>> +extracted to the directory expected by the default value of :term:`S`. For
>>> +example::
>>> +
>>> +   SRC_URI = "https://example.com/downloads/somepackage.rpm;subdir=${BP}"
>>>    
>>> -   For RPMs and other packages that do not contain a subdirectory, you
>>> -   should specify an appropriate fetcher parameter to point to the
>>> -   subdirectory. For example, if BitBake is using the Git fetcher (``git://``),
>>> -   the "subpath" parameter limits the checkout to a specific subpath
>>> -   of the tree. Here is an example where ``${BP}`` is used so that the files
>>> -   are extracted into the subdirectory expected by the default value of
>>> -   :term:`S`::
>>> +This class assumes that the content of :term:`S` already contains the
>>
>> """
>> This class assumes that the content of the package as installed in
>> :term:`S` mirrors the expected layout once installed on the target,
>> which is generally[...]
>> """
>>
>> ?
> 
> I agree, better wording
> 
>>> +installation paths on the target , which is generally the case for binary
>>
>> Spurious whitespace before comma.
> 
> +1
> 
>>> +packages. For example, a RPM package containing a library should already contain
>>
>> s/a RPM/an RPM/
>>
>> s/containing a library/for a library/ # to avoid redundant "contain" verb
>>
>> s/should already contain/would usually contain/
> 
> +1
> 
>>> +the ``usr/lib`` directories, and should be extracted as
>>
>> s/as/to/?
> 
> +1
> 
>>> +``${S}/usr/lib/<library>.so`` to be installed in :term:`D` correctly.
>>> +
>>
>> It'd be nice to encourage people to use versioned shared libraries even
>> for binary packages but not sure how to word this here.
> 
> Replace by ``${S}/usr/lib/<library>.<version>.so``? and implicitly encourage it.
> Not sure this is the place to give written advice on that.
> 

It's actually ``${S}/usr/lib/<library>.so.<version>`` :) But would work 
for me. Though considering FILES:${PN} = "/" once inheriting this class, 
whether versioned or not doesn't matter to the packaging step (unlike 
the default values, where .so goes to -dev package).

>>> +.. note::
>>>    
>>> -      SRC_URI = "git://example.com/downloads/somepackage.rpm;branch=main;subpath=${BP}"
>>> +   The extraction of the package passed in :term:`SRC_URI` is not handled by the
>>> +   :ref:`ref-classes-bin-package` class, and is done by the appropriate

s/, and is done/but rather/ ?

>>> +   :ref:`fetcher <bitbake-user-manual/bitbake-user-manual-fetching:fetchers>`
>>> +   depending on the nature of the package.
>>
>> What do you mean by "depending on the nature of the package"?
> 
> I meant the type, like RPM, tarball, deb… I can replace by "type", and add a
> "(RPM, Deb, tarball, …)" in parenthesis afterwards.
> 

I would simply remove "depending on the nature of the package", that's 
already implied? Ah, or we can say depending on the file extension which 
I believe is how the fetcher is actually figuring out how to extract them?

Cheers,
Quentin

Hi Quentin,

On Wed Nov 27, 2024 at 3:15 PM CET, Quentin Schulz wrote:
[...]
>>>> +``${S}/usr/lib/<library>.so`` to be installed in :term:`D` correctly.
>>>> +
>>>
>>> It'd be nice to encourage people to use versioned shared libraries even
>>> for binary packages but not sure how to word this here.
>> 
>> Replace by ``${S}/usr/lib/<library>.<version>.so``? and implicitly encourage it.
>> Not sure this is the place to give written advice on that.
>> 
>
> It's actually ``${S}/usr/lib/<library>.so.<version>`` :) But would work 

Oops, right thanks :)

> for me. Though considering FILES:${PN} = "/" once inheriting this class, 
> whether versioned or not doesn't matter to the packaging step (unlike 
> the default values, where .so goes to -dev package).
>
>>>> +.. note::
>>>>    
>>>> -      SRC_URI = "git://example.com/downloads/somepackage.rpm;branch=main;subpath=${BP}"
>>>> +   The extraction of the package passed in :term:`SRC_URI` is not handled by the
>>>> +   :ref:`ref-classes-bin-package` class, and is done by the appropriate
>
> s/, and is done/but rather/ ?

+1

>>>> +   :ref:`fetcher <bitbake-user-manual/bitbake-user-manual-fetching:fetchers>`
>>>> +   depending on the nature of the package.
>>>
>>> What do you mean by "depending on the nature of the package"?
>> 
>> I meant the type, like RPM, tarball, deb… I can replace by "type", and add a
>> "(RPM, Deb, tarball, …)" in parenthesis afterwards.
>> 
>
> I would simply remove "depending on the nature of the package", that's 
> already implied? Ah, or we can say depending on the file extension which 
> I believe is how the fetcher is actually figuring out how to extract them?

Sure! File extension's fine to me.

Thanks,
Antonin