license.bbclass does not add recommends to dynamic packages

license.bbclass does not add recommends to dynamic packages

[Mike Crowe]

We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
relying on the automatically generated recommends to cause them to be
installed in the image. This works well for most packages, but fails for
packages where we only install package created using PACKAGES_DYNAMIC.

For example, liborc is being installed in our image but that package lacks
a recommends for orc-lic, so the licences that apply to it are not being
installed. This is because license.bbclass:add_package_and_files iterates
only over the packages listed in PACKAGES.

Steps to reproduce on current master:

$ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
$ bitbake orc
$ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
 Recommends: orc-lic
$ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
$

(I would have expected the last command to produce the same output as the
penultimate one.)

Even if I could fathom out how to fix orc and any other recipes so that they
did add the ${PN}-lic dependency, I'd be worried about not noticing that
the problem affected other recipes in the future.

Is there a way to teach license.bbclass:add_package_and_files to add the
${PN}-lic recommends for dynamic packages, or would it be necessary to
teach package.bbclass to do so?

Thanks.

Mike.

Re: [OE-core] license.bbclass does not add recommends to dynamic packages

[Richard Purdie]

On Wed, 2021-07-07 at 12:53 +0100, Mike Crowe via lists.openembedded.org wrote:
> We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
> relying on the automatically generated recommends to cause them to be
> installed in the image. This works well for most packages, but fails for
> packages where we only install package created using PACKAGES_DYNAMIC.
> 
> For example, liborc is being installed in our image but that package lacks
> a recommends for orc-lic, so the licences that apply to it are not being
> installed. This is because license.bbclass:add_package_and_files iterates
> only over the packages listed in PACKAGES.
> 
> Steps to reproduce on current master:
> 
> $ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
> $ bitbake orc
> $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
>  Recommends: orc-lic
> $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> $
> 
> (I would have expected the last command to produce the same output as the
> penultimate one.)
> 
> Even if I could fathom out how to fix orc and any other recipes so that they
> did add the ${PN}-lic dependency, I'd be worried about not noticing that
> the problem affected other recipes in the future.
> 
> Is there a way to teach license.bbclass:add_package_and_files to add the
> ${PN}-lic recommends for dynamic packages, or would it be necessary to
> teach package.bbclass to do so?

That all sounds rather horrible :/.

Would IMAGE_INSTALL_COMPLEMENTARY += "*-lic" work instead?

Cheers,

Richard

Re: [OE-core] license.bbclass does not add recommends to dynamic packages

[Mike Crowe]

On Wednesday 07 July 2021 at 13:25:17 +0100, Richard Purdie wrote:
> On Wed, 2021-07-07 at 12:53 +0100, Mike Crowe via lists.openembedded.org wrote:
> > We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
> > relying on the automatically generated recommends to cause them to be
> > installed in the image. This works well for most packages, but fails for
> > packages where we only install package created using PACKAGES_DYNAMIC.
> > 
> > For example, liborc is being installed in our image but that package lacks
> > a recommends for orc-lic, so the licences that apply to it are not being
> > installed. This is because license.bbclass:add_package_and_files iterates
> > only over the packages listed in PACKAGES.
> > 
> > Steps to reproduce on current master:
> > 
> > $ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
> > $ bitbake orc
> > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> >  Recommends: orc-lic
> > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > $
> > 
> > (I would have expected the last command to produce the same output as the
> > penultimate one.)
> > 
> > Even if I could fathom out how to fix orc and any other recipes so that they
> > did add the ${PN}-lic dependency, I'd be worried about not noticing that
> > the problem affected other recipes in the future.
> > 
> > Is there a way to teach license.bbclass:add_package_and_files to add the
> > ${PN}-lic recommends for dynamic packages, or would it be necessary to
> > teach package.bbclass to do so?
> 
> That all sounds rather horrible :/.
> 
> Would IMAGE_INSTALL_COMPLEMENTARY += "*-lic" work instead?

That seems to have worked well.

I wonder whether this means that it would be better to stop adding the
recommends automatically and tell users that need this to use
IMAGE_INSTALL_COMPLEMENTARY instead (either directly, or by teaching
license_image.bbclass to modify it based on another variable.)

Losing the recommends would also meaan I wouldn't need to add
--no-recommends to the image recipes that don't need the licence files.

Thanks for the speedy response!

Mike.

Re: [OE-core] license.bbclass does not add recommends to dynamic packages

[Richard Purdie]

On Wed, 2021-07-07 at 14:05 +0100, Mike Crowe wrote:
> On Wednesday 07 July 2021 at 13:25:17 +0100, Richard Purdie wrote:
> > On Wed, 2021-07-07 at 12:53 +0100, Mike Crowe via lists.openembedded.org wrote:
> > > We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
> > > relying on the automatically generated recommends to cause them to be
> > > installed in the image. This works well for most packages, but fails for
> > > packages where we only install package created using PACKAGES_DYNAMIC.
> > > 
> > > For example, liborc is being installed in our image but that package lacks
> > > a recommends for orc-lic, so the licences that apply to it are not being
> > > installed. This is because license.bbclass:add_package_and_files iterates
> > > only over the packages listed in PACKAGES.
> > > 
> > > Steps to reproduce on current master:
> > > 
> > > $ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
> > > $ bitbake orc
> > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > >  Recommends: orc-lic
> > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > $
> > > 
> > > (I would have expected the last command to produce the same output as the
> > > penultimate one.)
> > > 
> > > Even if I could fathom out how to fix orc and any other recipes so that they
> > > did add the ${PN}-lic dependency, I'd be worried about not noticing that
> > > the problem affected other recipes in the future.
> > > 
> > > Is there a way to teach license.bbclass:add_package_and_files to add the
> > > ${PN}-lic recommends for dynamic packages, or would it be necessary to
> > > teach package.bbclass to do so?
> > 
> > That all sounds rather horrible :/.
> > 
> > Would IMAGE_INSTALL_COMPLEMENTARY += "*-lic" work instead?
> 
> That seems to have worked well.
> 
> I wonder whether this means that it would be better to stop adding the
> recommends automatically and tell users that need this to use
> IMAGE_INSTALL_COMPLEMENTARY instead (either directly, or by teaching
> license_image.bbclass to modify it based on another variable.)

That would seem the better option to me at least...

> Losing the recommends would also meaan I wouldn't need to add
> --no-recommends to the image recipes that don't need the licence files.
> 
> Thanks for the speedy response!

No problem, glad it works/helps!

Cheers,

Richard

Re: [OE-core] license.bbclass does not add recommends to dynamic packages

[Richard Purdie]

On Wed, 2021-07-07 at 17:43 +0100, Richard Purdie via lists.openembedded.org wrote:
> On Wed, 2021-07-07 at 14:05 +0100, Mike Crowe wrote:
> > On Wednesday 07 July 2021 at 13:25:17 +0100, Richard Purdie wrote:
> > > On Wed, 2021-07-07 at 12:53 +0100, Mike Crowe via lists.openembedded.org wrote:
> > > > We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
> > > > relying on the automatically generated recommends to cause them to be
> > > > installed in the image. This works well for most packages, but fails for
> > > > packages where we only install package created using PACKAGES_DYNAMIC.
> > > > 
> > > > For example, liborc is being installed in our image but that package lacks
> > > > a recommends for orc-lic, so the licences that apply to it are not being
> > > > installed. This is because license.bbclass:add_package_and_files iterates
> > > > only over the packages listed in PACKAGES.
> > > > 
> > > > Steps to reproduce on current master:
> > > > 
> > > > $ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
> > > > $ bitbake orc
> > > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > >  Recommends: orc-lic
> > > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > > $
> > > > 
> > > > (I would have expected the last command to produce the same output as the
> > > > penultimate one.)
> > > > 
> > > > Even if I could fathom out how to fix orc and any other recipes so that they
> > > > did add the ${PN}-lic dependency, I'd be worried about not noticing that
> > > > the problem affected other recipes in the future.
> > > > 
> > > > Is there a way to teach license.bbclass:add_package_and_files to add the
> > > > ${PN}-lic recommends for dynamic packages, or would it be necessary to
> > > > teach package.bbclass to do so?
> > > 
> > > That all sounds rather horrible :/.
> > > 
> > > Would IMAGE_INSTALL_COMPLEMENTARY += "*-lic" work instead?
> > 
> > That seems to have worked well.
> > 
> > I wonder whether this means that it would be better to stop adding the
> > recommends automatically and tell users that need this to use
> > IMAGE_INSTALL_COMPLEMENTARY instead (either directly, or by teaching
> > license_image.bbclass to modify it based on another variable.)
> 
> That would seem the better option to me at least...

To be clear, I'd definitely support dropping that existing RRECOMMENDS code,
I think there are better ways to handle this now. I may even write that patch :)

Cheers,

Richard

Re: [OE-core] license.bbclass does not add recommends to dynamic packages

[Mike Crowe]

On Wednesday 07 July 2021 at 17:48:20 +0100, Richard Purdie wrote:
> On Wed, 2021-07-07 at 17:43 +0100, Richard Purdie via lists.openembedded.org wrote:
> > On Wed, 2021-07-07 at 14:05 +0100, Mike Crowe wrote:
> > > On Wednesday 07 July 2021 at 13:25:17 +0100, Richard Purdie wrote:
> > > > On Wed, 2021-07-07 at 12:53 +0100, Mike Crowe via lists.openembedded.org wrote:
> > > > > We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
> > > > > relying on the automatically generated recommends to cause them to be
> > > > > installed in the image. This works well for most packages, but fails for
> > > > > packages where we only install package created using PACKAGES_DYNAMIC.
> > > > > 
> > > > > For example, liborc is being installed in our image but that package lacks
> > > > > a recommends for orc-lic, so the licences that apply to it are not being
> > > > > installed. This is because license.bbclass:add_package_and_files iterates
> > > > > only over the packages listed in PACKAGES.
> > > > > 
> > > > > Steps to reproduce on current master:
> > > > > 
> > > > > $ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
> > > > > $ bitbake orc
> > > > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > > >  Recommends: orc-lic
> > > > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > > > $
> > > > > 
> > > > > (I would have expected the last command to produce the same output as the
> > > > > penultimate one.)
> > > > > 
> > > > > Even if I could fathom out how to fix orc and any other recipes so that they
> > > > > did add the ${PN}-lic dependency, I'd be worried about not noticing that
> > > > > the problem affected other recipes in the future.
> > > > > 
> > > > > Is there a way to teach license.bbclass:add_package_and_files to add the
> > > > > ${PN}-lic recommends for dynamic packages, or would it be necessary to
> > > > > teach package.bbclass to do so?
> > > > 
> > > > That all sounds rather horrible :/.
> > > > 
> > > > Would IMAGE_INSTALL_COMPLEMENTARY += "*-lic" work instead?
> > > 
> > > That seems to have worked well.
> > > 
> > > I wonder whether this means that it would be better to stop adding the
> > > recommends automatically and tell users that need this to use
> > > IMAGE_INSTALL_COMPLEMENTARY instead (either directly, or by teaching
> > > license_image.bbclass to modify it based on another variable.)
> > 
> > That would seem the better option to me at least...
> 
> To be clear, I'd definitely support dropping that existing RRECOMMENDS code,
> I think there are better ways to handle this now. I may even write that patch :)

I was thinking of something like the following untested patch:

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index f7978e266b..c87473cbb8 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -63,14 +63,6 @@ def add_package_and_files(d):
         # first in PACKAGES to be sure that nothing else gets LICENSE_FILES_DIRECTORY
         d.setVar('PACKAGES', "%s %s" % (pn_lic, packages))
         d.setVar('FILES_' + pn_lic, files)
-    for pn in packages.split():
-        if pn == pn_lic:
-            continue
-        rrecommends_pn = d.getVar('RRECOMMENDS_' + pn)
-        if rrecommends_pn:
-            d.setVar('RRECOMMENDS_' + pn, "%s %s" % (pn_lic, rrecommends_pn))
-        else:
-            d.setVar('RRECOMMENDS_' + pn, "%s" % (pn_lic))

 def copy_license_files(lic_files_paths, destdir):
     import shutil
diff --git a/meta/classes/license_image.bbclass b/meta/classes/license_image.bbclass
index 73cebb4d55..293a033855 100644
--- a/meta/classes/license_image.bbclass
+++ b/meta/classes/license_image.bbclass
@@ -279,3 +279,6 @@ python license_qa_dead_symlink() {
                 bb.error("broken symlink: " + full_path)
 }
 IMAGE_QA_COMMANDS += "license_qa_dead_symlink"
+
+IMAGE_INSTALL_LICENSES ??= "${LICENSE_CREATE_PACKAGE}"
+IMAGE_INSTALL_COMPLEMENTARY += "${oe.utils.conditional("IMAGE_INSTALL_LICENSES", "0", "", "*-lic", d)}"

I'm not sure whether it's safe to += an empty string onto the end of
IMAGE_INSTALL_COMPLEMENTARY, but I'm trying to avoid adding yet more
anonymous Python. :-)

Is that the sort of thing you meant?

Thanks.

Mike.

Re: [OE-core] license.bbclass does not add recommends to dynamic packages

[Richard Purdie]

On Wed, 2021-07-07 at 17:55 +0100, Mike Crowe wrote:
> On Wednesday 07 July 2021 at 17:48:20 +0100, Richard Purdie wrote:
> > On Wed, 2021-07-07 at 17:43 +0100, Richard Purdie via lists.openembedded.org wrote:
> > > On Wed, 2021-07-07 at 14:05 +0100, Mike Crowe wrote:
> > > > On Wednesday 07 July 2021 at 13:25:17 +0100, Richard Purdie wrote:
> > > > > On Wed, 2021-07-07 at 12:53 +0100, Mike Crowe via lists.openembedded.org wrote:
> > > > > > We're using LICENSE_CREATE_PACKAGE to create ${PN}-lic package files and
> > > > > > relying on the automatically generated recommends to cause them to be
> > > > > > installed in the image. This works well for most packages, but fails for
> > > > > > packages where we only install package created using PACKAGES_DYNAMIC.
> > > > > > 
> > > > > > For example, liborc is being installed in our image but that package lacks
> > > > > > a recommends for orc-lic, so the licences that apply to it are not being
> > > > > > installed. This is because license.bbclass:add_package_and_files iterates
> > > > > > only over the packages listed in PACKAGES.
> > > > > > 
> > > > > > Steps to reproduce on current master:
> > > > > > 
> > > > > > $ echo 'LICENSE_CREATE_PACKAGE = "1"' >> conf/local.conf
> > > > > > $ bitbake orc
> > > > > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/orc_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > > > >  Recommends: orc-lic
> > > > > > $ dpkg-deb -I tmp-glibc/deploy/ipk/armv7vet2hf-neon/liborc-0.4-0_0.4.32-r0_armv7vet2hf-neon.ipk|grep Recommends
> > > > > > $
> > > > > > 
> > > > > > (I would have expected the last command to produce the same output as the
> > > > > > penultimate one.)
> > > > > > 
> > > > > > Even if I could fathom out how to fix orc and any other recipes so that they
> > > > > > did add the ${PN}-lic dependency, I'd be worried about not noticing that
> > > > > > the problem affected other recipes in the future.
> > > > > > 
> > > > > > Is there a way to teach license.bbclass:add_package_and_files to add the
> > > > > > ${PN}-lic recommends for dynamic packages, or would it be necessary to
> > > > > > teach package.bbclass to do so?
> > > > > 
> > > > > That all sounds rather horrible :/.
> > > > > 
> > > > > Would IMAGE_INSTALL_COMPLEMENTARY += "*-lic" work instead?
> > > > 
> > > > That seems to have worked well.
> > > > 
> > > > I wonder whether this means that it would be better to stop adding the
> > > > recommends automatically and tell users that need this to use
> > > > IMAGE_INSTALL_COMPLEMENTARY instead (either directly, or by teaching
> > > > license_image.bbclass to modify it based on another variable.)
> > > 
> > > That would seem the better option to me at least...
> > 
> > To be clear, I'd definitely support dropping that existing RRECOMMENDS code,
> > I think there are better ways to handle this now. I may even write that patch :)
> 
> I was thinking of something like the following untested patch:
> 
> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> index f7978e266b..c87473cbb8 100644
> --- a/meta/classes/license.bbclass
> +++ b/meta/classes/license.bbclass
> @@ -63,14 +63,6 @@ def add_package_and_files(d):
>          # first in PACKAGES to be sure that nothing else gets LICENSE_FILES_DIRECTORY
>          d.setVar('PACKAGES', "%s %s" % (pn_lic, packages))
>          d.setVar('FILES_' + pn_lic, files)
> -    for pn in packages.split():
> -        if pn == pn_lic:
> -            continue
> -        rrecommends_pn = d.getVar('RRECOMMENDS_' + pn)
> -        if rrecommends_pn:
> -            d.setVar('RRECOMMENDS_' + pn, "%s %s" % (pn_lic, rrecommends_pn))
> -        else:
> -            d.setVar('RRECOMMENDS_' + pn, "%s" % (pn_lic))
> 
>  def copy_license_files(lic_files_paths, destdir):
>      import shutil

The above looks like the patch I just sent out! :)


> diff --git a/meta/classes/license_image.bbclass b/meta/classes/license_image.bbclass
> index 73cebb4d55..293a033855 100644
> --- a/meta/classes/license_image.bbclass
> +++ b/meta/classes/license_image.bbclass
> @@ -279,3 +279,6 @@ python license_qa_dead_symlink() {
>                  bb.error("broken symlink: " + full_path)
>  }
>  IMAGE_QA_COMMANDS += "license_qa_dead_symlink"
> +
> +IMAGE_INSTALL_LICENSES ??= "${LICENSE_CREATE_PACKAGE}"
> +IMAGE_INSTALL_COMPLEMENTARY += "${oe.utils.conditional("IMAGE_INSTALL_LICENSES", "0", "", "*-lic", d)}"
> 
> I'm not sure whether it's safe to += an empty string onto the end of
> IMAGE_INSTALL_COMPLEMENTARY, but I'm trying to avoid adding yet more
> anonymous Python. :-)
> 
> Is that the sort of thing you meant?

Yes, += to an empty string is fine. I'm torn on whether we should do this "magically" 
or whether users should opt in to it when needed on a per image (or distro) basis.

Cheers,

Richard