[PATCH] license: Drop adding RRECOMMENDS for license packages

[PATCH] license: Drop adding RRECOMMENDS for license packages

[Richard Purdie]

This changes behaviour when LICENSE_CREATE_PACKAGE is in use. Packages
no longer have RRECOMMENDS adding to them.

It was highlighted that this doesn't apply to PACKAGES_DYNAMIC, nor can
it easily be made to do so. There is also a much easier way to handle this
which is:

IMAGE_INSTALL_COMPLEMENTARY += "*-lic"

which works on a per image basis and doesn't change the underlying
package dependencies. I propose we switch to this instead.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes/license.bbclass | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index f7978e266b6..c87473cbb85 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
-- 
2.30.2

Re: [OE-core] [PATCH] license: Drop adding RRECOMMENDS for license packages

[Mike Crowe]

On Wednesday 07 July 2021 at 18:06:07 +0100, Richard Purdie wrote:
> This changes behaviour when LICENSE_CREATE_PACKAGE is in use. Packages
> no longer have RRECOMMENDS adding to them.
> 
> It was highlighted that this doesn't apply to PACKAGES_DYNAMIC, nor can
> it easily be made to do so. There is also a much easier way to handle this
> which is:
> 
> IMAGE_INSTALL_COMPLEMENTARY += "*-lic"
>
> which works on a per image basis and doesn't change the underlying
> package dependencies. I propose we switch to this instead.

The downside to this is that IMAGE_INSTALL_COMPLEMENTARY isn't documented
at https://www.yoctoproject.org/docs/latest/ref-manual/ref-manual.html so
I'd assumed it to be an implementation detail. (Adding it to the
documentation is presumably not difficult though.)

Since the equivalent for "*-dbg" is to add dbg-pkgs to IMAGE_FEATURES, I
think that adding:

 COMPLEMENTARY_GLOB[lic-pkgs] = "*-lic"

to license_image.bbclass is a cleaner solution. We could make the feature
imply LICENSE_CREATE_PACKAGE="1" but it feels strange for an image feature
to affect the packages when they are built so perhaps that isn't a good
idea.

Thanks.

Mike.

Re: [OE-core] [PATCH] license: Drop adding RRECOMMENDS for license packages

[Richard Purdie]

On Thu, 2021-07-08 at 09:26 +0100, Mike Crowe wrote:
> On Wednesday 07 July 2021 at 18:06:07 +0100, Richard Purdie wrote:
> > This changes behaviour when LICENSE_CREATE_PACKAGE is in use. Packages
> > no longer have RRECOMMENDS adding to them.
> > 
> > It was highlighted that this doesn't apply to PACKAGES_DYNAMIC, nor can
> > it easily be made to do so. There is also a much easier way to handle this
> > which is:
> > 
> > IMAGE_INSTALL_COMPLEMENTARY += "*-lic"
> > 
> > which works on a per image basis and doesn't change the underlying
> > package dependencies. I propose we switch to this instead.
> 
> The downside to this is that IMAGE_INSTALL_COMPLEMENTARY isn't documented
> at https://www.yoctoproject.org/docs/latest/ref-manual/ref-manual.html so
> I'd assumed it to be an implementation detail. (Adding it to the
> documentation is presumably not difficult though.)

It should be documented, I think it being missing is just an oversight.

> 
> Since the equivalent for "*-dbg" is to add dbg-pkgs to IMAGE_FEATURES, I
> think that adding:
> 
>  COMPLEMENTARY_GLOB[lic-pkgs] = "*-lic"
> 
> to license_image.bbclass is a cleaner solution.

Yes, I like that.

> We could make the feature
> imply LICENSE_CREATE_PACKAGE="1" but it feels strange for an image feature
> to affect the packages when they are built so perhaps that isn't a good
> idea.

I agree, I'm not sure that would be too helpful. Maybe a comment next to
the line above mentioning it needs to be set? We might also be able to
add a warning if it wasn't set but lic-pkgs was used?

Cheers,

Richard

Re: [OE-core] [PATCH] license: Drop adding RRECOMMENDS for license packages

[Mike Crowe]

On Thursday 08 July 2021 at 09:33:12 +0100, Richard Purdie wrote:
> On Thu, 2021-07-08 at 09:26 +0100, Mike Crowe wrote:
> > On Wednesday 07 July 2021 at 18:06:07 +0100, Richard Purdie wrote:
> > > This changes behaviour when LICENSE_CREATE_PACKAGE is in use. Packages
> > > no longer have RRECOMMENDS adding to them.
> > > 
> > > It was highlighted that this doesn't apply to PACKAGES_DYNAMIC, nor can
> > > it easily be made to do so. There is also a much easier way to handle this
> > > which is:
> > > 
> > > IMAGE_INSTALL_COMPLEMENTARY += "*-lic"
> > > 
> > > which works on a per image basis and doesn't change the underlying
> > > package dependencies. I propose we switch to this instead.

[snip]

> > Since the equivalent for "*-dbg" is to add dbg-pkgs to IMAGE_FEATURES, I
> > think that adding:
> > 
> >  COMPLEMENTARY_GLOB[lic-pkgs] = "*-lic"
> > 
> > to license_image.bbclass is a cleaner solution.
> 
> Yes, I like that.

It seems to work well for me.

> > We could make the feature
> > imply LICENSE_CREATE_PACKAGE="1" but it feels strange for an image feature
> > to affect the packages when they are built so perhaps that isn't a good
> > idea.
> 
> I agree, I'm not sure that would be too helpful. Maybe a comment next to
> the line above mentioning it needs to be set? We might also be able to
> add a warning if it wasn't set but lic-pkgs was used?

The comment is easy, as is documenting the image feature. If I add
"lic-pkgs" to EXTRA_IMAGE_FEATURES but don't set LICENSE_CREATE_PACKAGE="1"
then there are no errors when building the image. I added:
--8<--
python() {
    if not oe.data.typed_value('LICENSE_CREATE_PACKAGE', d):
        features = set(oe.data.typed_value('IMAGE_FEATURES', d))
        if 'lic-pkgs' in features:
            bb.error("'lic-pkgs' in IMAGE_FEATURES but LICENSE_CREATE_PACKAGE not enabled to generate -lic packages")
}
-->8--
to license_image.bbclass which reports the problem, but does so for every
image even if it isn't being built. An alternative would be to add a new
function to ROOTFS_PREPROCESS_COMMAND which does the check at image
creation time.

Do you have a preference, or is there a better way that I've missed?

Thanks.

Mike.

Re: [OE-core] [PATCH] license: Drop adding RRECOMMENDS for license packages

[Mike Crowe]

On Wednesday 07 July 2021 at 18:06:07 +0100, Richard Purdie wrote:
> This changes behaviour when LICENSE_CREATE_PACKAGE is in use. Packages
> no longer have RRECOMMENDS adding to them.
> 
> It was highlighted that this doesn't apply to PACKAGES_DYNAMIC, nor can
> it easily be made to do so. There is also a much easier way to handle this
> which is:
> 
> IMAGE_INSTALL_COMPLEMENTARY += "*-lic"
> 
> which works on a per image basis and doesn't change the underlying
> package dependencies. I propose we switch to this instead.
> 
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes/license.bbclass | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
> index f7978e266b6..c87473cbb85 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
> -- 
> 2.30.2
> 

It turns out that this has an interesting side effect when
IMAGE_FEATURES also contains "dbg-pkgs".

If python3-core is installed in my image then dbg-pkgs causes python3-dbg
to be installed. python3-dbg recommends gdbm-dbg, so libgdbm-dbg also gets
installed.

Prior to this change, libgdbm-dbg recommends gdbm-lic, so libgdbm-lic gets
installed too.

After this change, libgdbm-dbg no longer recommends gdbm-lic and since
libgdbm itself isn't installed and the complementary globs have already
been matched libgdbm-lic is not installed.

(Other recipes that have multiple binary packages with dependencies that
aren't all installed are affected similarly.)

I don't think this really matters in general, and don't propose to change
anything, but it is a surprise if you have a check that goes through the
results of image_list_installed_packages(d) confirming that all the licence
files have been installed and it gets upset that libgdbm-lic isn't there.

Thanks.

Mike.