[PATCH] classes/license: copy licenses even if there are no checksummed files

[PATCH] classes/license: copy licenses even if there are no checksummed files

[Ross Burton]

Previously do_populate_lic would bail writing the license files (such as the MIT
license text) to deploy/licenses/${PN}/ if there were no files listed in
LIC_FILES_CHKSUM.

However this means that recipes that generate their content (such as os-release)
or are otherwise "interesting" (such as perf) don't have their license files
copied over, resulting in warnings from do_rootfs.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/classes/license.bbclass | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index 660b85f..afcfbfc 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -390,7 +390,7 @@ def find_license_files(d):
     from collections import defaultdict, OrderedDict
 
     # All the license files for the package
-    lic_files = d.getVar('LIC_FILES_CHKSUM', True)
+    lic_files = d.getVar('LIC_FILES_CHKSUM', True) or ""
     pn = d.getVar('PN', True)
     # The license files are located in S/LIC_FILE_CHECKSUM.
     srcdir = d.getVar('S', True)
@@ -469,12 +469,6 @@ def find_license_files(d):
     if not generic_directory:
         bb.fatal("COMMON_LICENSE_DIR is unset. Please set this in your distro config")
 
-    if not lic_files:
-        # No recipe should have an invalid license file. This is checked else
-        # where, but let's be pedantic
-        bb.note(pn + ": Recipe file does not have license file information.")
-        return lic_files_paths
-
     for url in lic_files.split():
         try:
             (type, host, path, user, pswd, parm) = bb.fetch.decodeurl(url)
-- 
2.8.1

Re: [PATCH] classes/license: copy licenses even if there are no checksummed files

[Beth 'pidge' Flanagan]

On Wed, 2016-11-02 at 15:07 +0000, Ross Burton wrote:
> Previously do_populate_lic would bail writing the license files (such
> as the MIT
> license text) to deploy/licenses/${PN}/ if there were no files listed
> in
> LIC_FILES_CHKSUM.
> 
> However this means that recipes that generate their content (such as
> os-release)
> or are otherwise "interesting" (such as perf) don't have their
> license files
> copied over, resulting in warnings from do_rootfs.
> 

It's been a bit since I've looked at license.bbclass, but I guess my
question here is this seems to take out all notification that a recipe
doesn't have a LIC_FILES_CHKSUM, correct?

I'd rather we maybe kept the note in and maybe whitelisted recipes that
did interesting things so that didn't appear than just ignored when a
recipe was missing it. 

Because if we don't at least bb.note it, I can see people just ignoring
it entirely (which would be bad in a lot of ways)

-b

> Signed-off-by: Ross Burton <ross.burton@intel.com>
> ---
>  meta/classes/license.bbclass | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/meta/classes/license.bbclass
> b/meta/classes/license.bbclass
> index 660b85f..afcfbfc 100644
> --- a/meta/classes/license.bbclass
> +++ b/meta/classes/license.bbclass
> @@ -390,7 +390,7 @@ def find_license_files(d):
>      from collections import defaultdict, OrderedDict
>  
>      # All the license files for the package
> -    lic_files = d.getVar('LIC_FILES_CHKSUM', True)
> +    lic_files = d.getVar('LIC_FILES_CHKSUM', True) or ""
>      pn = d.getVar('PN', True)
>      # The license files are located in S/LIC_FILE_CHECKSUM.
>      srcdir = d.getVar('S', True)
> @@ -469,12 +469,6 @@ def find_license_files(d):
>      if not generic_directory:
>          bb.fatal("COMMON_LICENSE_DIR is unset. Please set this in
> your distro config")
>  
> -    if not lic_files:
> -        # No recipe should have an invalid license file. This is
> checked else
> -        # where, but let's be pedantic
> -        bb.note(pn + ": Recipe file does not have license file
> information.")
> -        return lic_files_paths
> -
>      for url in lic_files.split():
>          try:
>              (type, host, path, user, pswd, parm) =
> bb.fetch.decodeurl(url)

Re: [PATCH] classes/license: copy licenses even if there are no checksummed files

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 835 bytes --]

On 2 November 2016 at 18:37, Beth 'pidge' Flanagan <pidge@toganlabs.com>
wrote:

> It's been a bit since I've looked at license.bbclass, but I guess my
> question here is this seems to take out all notification that a recipe
> doesn't have a LIC_FILES_CHKSUM, correct?
>
> I'd rather we maybe kept the note in and maybe whitelisted recipes that
> did interesting things so that didn't appear than just ignored when a
> recipe was missing it.
>
> Because if we don't at least bb.note it, I can see people just ignoring
> it entirely (which would be bad in a lot of ways)
>

No, this check was belt-and-braces (the other check is a bb.error()).  Also
bb.note() from tasks don't appear in the user-visible log by default,
you've got to dig into the task log to see them which is buried in the work
directories.

Ross

[-- Attachment #2: Type: text/html, Size: 1381 bytes --]

Re: [PATCH] classes/license: copy licenses even if there are no checksummed files

[Beth 'pidge' Flanagan]

[-- Attachment #1: Type: text/plain, Size: 1067 bytes --]

On Wed, 2016-11-02 at 20:22 +0000, Burton, Ross wrote:
> 
> On 2 November 2016 at 18:37, Beth 'pidge' Flanagan 
> om> wrote:
> > It's been a bit since I've looked at license.bbclass, but I guess
> > my
> > question here is this seems to take out all notification that a
> > recipe
> > doesn't have a LIC_FILES_CHKSUM, correct?
> > 
> > I'd rather we maybe kept the note in and maybe whitelisted recipes
> > that
> > did interesting things so that didn't appear than just ignored when
> > a
> > recipe was missing it. 
> > 
> > Because if we don't at least bb.note it, I can see people just
> > ignoring
> > it entirely (which would be bad in a lot of ways)
> > 
> No, this check was belt-and-braces (the other check is a
> bb.error()).  Also bb.note() from tasks don't appear in the user-
> visible log by default, you've got to dig into the task log to see
> them which is buried in the work directories.
Ah, correct (I generally see them since I'm piping most of the time).
Ok, in that case then yes, that makes sense.
-b
> > Ross
> 

[-- Attachment #2: Type: text/html, Size: 1598 bytes --]

Re: [PATCH] classes/license: copy licenses even if there are no checksummed files

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

On 2 November 2016 at 15:07, Ross Burton <ross.burton@intel.com> wrote:

> Previously do_populate_lic would bail writing the license files (such as
> the MIT
> license text) to deploy/licenses/${PN}/ if there were no files listed in
> LIC_FILES_CHKSUM.
>
> However this means that recipes that generate their content (such as
> os-release)
> or are otherwise "interesting" (such as perf) don't have their license
> files
> copied over, resulting in warnings from do_rootfs.
>

I just did a bit more digging that I meant to do earlier but failed.  This
has recently become apparent because of
oe-core b18fa5f2f2f46afc6fdc58f4d29679dea9c36c43 which removed redundant
LIC_FILES_CHKSUMs from recipes.  For example, os-release had this:

LIC_FILES_CHKSUM =
"file://${COREBASE}/meta/COPYING.MIT;md5=3da9cfbcb788c80a0384361b4de20420"

Which prompts the license code to copy COPYING.MIT and generic_MIT into
deploy/licenses.  It was removed because it generally serves no purpose:
you're verifying a license statement that has no relation to the contents
of the recipe (in this case, the content is generated, but other recipes
have no content).  The side effect was that the files are not copied, so my
patch changes the behaviour so that the recipe wide license file is
*always* copied even if there are no files in LIC_FILES_CHKSUM.

I'll update the commit message to reflect this.

Ross

[-- Attachment #2: Type: text/html, Size: 2130 bytes --]

[PATCH] classes/license: copy licenses even if there are no checksummed files

[Ross Burton]

Previously do_populate_lic would bail writing the license files (such as the MIT
license text) to deploy/licenses/${PN}/ if there were no files listed in
LIC_FILES_CHKSUM.  In general this isn't a problem since LIC_FILES_CHKSUM was
effectively a required field, and for packages which don't have traditional
content was generally set to a separate license such as
${COREBASE}/meta/COPYING.MIT.

However LIC_FILES_CHKSUM is now only required if there are sources in SRC_URI,
so oe-core b18fa5f removed these now redundant values. The unexpected side
effect of this is that that recipes that generate their content (such as
os-release) or are otherwise "interesting" (such as perf) don't have their
license files copied over, resulting in warnings from do_rootfs.

Resolve this by not returning early if LIC_FILES_CHKSUM is empty, and always
copy the references license files.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/classes/license.bbclass | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index 660b85f..afcfbfc 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -390,7 +390,7 @@ def find_license_files(d):
     from collections import defaultdict, OrderedDict
 
     # All the license files for the package
-    lic_files = d.getVar('LIC_FILES_CHKSUM', True)
+    lic_files = d.getVar('LIC_FILES_CHKSUM', True) or ""
     pn = d.getVar('PN', True)
     # The license files are located in S/LIC_FILE_CHECKSUM.
     srcdir = d.getVar('S', True)
@@ -469,12 +469,6 @@ def find_license_files(d):
     if not generic_directory:
         bb.fatal("COMMON_LICENSE_DIR is unset. Please set this in your distro config")
 
-    if not lic_files:
-        # No recipe should have an invalid license file. This is checked else
-        # where, but let's be pedantic
-        bb.note(pn + ": Recipe file does not have license file information.")
-        return lic_files_paths
-
     for url in lic_files.split():
         try:
             (type, host, path, user, pswd, parm) = bb.fetch.decodeurl(url)
-- 
2.8.1