diff mbox

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

Message ID 1478099232-23987-1-git-send-email-ross.burton@intel.com
State Superseded
Headers show

Commit Message

Ross Burton Nov. 2, 2016, 3:07 p.m. UTC
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(-)

-- 
2.8.1

-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Comments

Beth 'pidge' Flanagan Nov. 2, 2016, 6:37 p.m. UTC | #1
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)
Ross Burton Nov. 2, 2016, 8:22 p.m. UTC | #2
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
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Ross Burton Nov. 2, 2016, 8:42 p.m. UTC | #3
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
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
Beth 'pidge' Flanagan Nov. 2, 2016, 10:42 p.m. UTC | #4
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

>
-- 
_______________________________________________
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core
diff mbox

Patch

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)