Bug 9499

Summary: license.bbclass find_license_files does not process LICENSE_pn variables correctly
Product: [Build System, Metadata & Runtime] OE-Core Reporter: Ola Nilsson <ola.x.nilsson>
Component: deploymentAssignee: Markus Lehtonen <markus.lehtonen>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: Medium+ CC: elizabeth.flanagan, richard.purdie, sgw
Version: 2.0   
Target Milestone: 2.2 M3   
Hardware: x86   
OS: Multiple   
Whiteboard:
OS type for building Yocto: --- Type of Regression: ---
Verified: Documentation change: Don't know

Description Ola Nilsson 2016-04-21 11:46:22 UTC
The first loop in find_license_files seems to have a few issues

* d.getVar('PACKAGES', True) needs a .split() at the end for the loop to process packages instead of letters.

* The assignment to license_types in the loop will add a leading ' & ' before the first license which will trigger a SyntaxError exception from FindVisitor.visit_string later in the function.

The function does not seem to handle _both_ a LICENSE variable and LICENSE_pn variables.  I could not find any documentation regarding this so I'm not sure what should be supported.  Our recipe was using LICENSE="MIT" and then the LICENSE_pn variables were using other licenses - not MIT.
Comment 1 Markus Lehtonen 2016-05-18 12:49:52 UTC
Hmm, interesting that this seems to have been broken for years (actually always, from the very beginning, if I read the history correctly) without nobody noticing.

I don't (yet) know how this is supposed to be working, either. You're supposed to have something in LICENSE anyway (not defining it fails to build). What is the additional value of having LICENSE_pn instead of just listing them in LICENSE, and, if we want to support those, how LICENSE and LICENSE_pn should be combined. Just concatenate, I suppose(?)
Comment 2 Markus Lehtonen 2016-05-18 12:51:39 UTC
Hmm, interesting that this seems to have been broken for years (actually always, from the very beginning, if I read the history correctly) without nobody noticing.

I don't (yet) know how this is supposed to be working, either. You're supposed to have something in LICENSE anyway (not defining it fails to build). What is the additional value of having LICENSE_pn instead of just listing them in LICENSE, and, if we want to support those, how LICENSE and LICENSE_pn should be combined. Just concatenate, I suppose(?)
Comment 3 Beth Flanagan 2016-05-18 13:12:41 UTC
(In reply to comment #0)
> The first loop in find_license_files seems to have a few issues
> 
> * d.getVar('PACKAGES', True) needs a .split() at the end for the loop to
> process packages instead of letters.
> 
> * The assignment to license_types in the loop will add a leading ' & '
> before the first license which will trigger a SyntaxError exception from
> FindVisitor.visit_string later in the function.
> 
> The function does not seem to handle _both_ a LICENSE variable and
> LICENSE_pn variables.  I could not find any documentation regarding this so
> I'm not sure what should be supported.  Our recipe was using LICENSE="MIT"
> and then the LICENSE_pn variables were using other licenses - not MIT.

So, LICENSE should contain all the licenses for all the packages for that recipe whereas LICENSE_pn should be the finer grained license for the specific packages. You wouldn't have LICENSE_pn have say, GPL-2.0 without having that in LICENSE.
Comment 4 Markus Lehtonen 2016-05-18 13:21:48 UTC
If license is supposed to contain all LICENSE_pn values then we could just drop the for loop iterating over LICENSE_pn, right(?)
Comment 5 Ola Nilsson 2016-05-18 13:40:54 UTC
(In reply to comment #3)
> (In reply to comment #0)
> > The first loop in find_license_files seems to have a few issues
> > 
> > * d.getVar('PACKAGES', True) needs a .split() at the end for the loop to
> > process packages instead of letters.
> > 
> > * The assignment to license_types in the loop will add a leading ' & '
> > before the first license which will trigger a SyntaxError exception from
> > FindVisitor.visit_string later in the function.
> > 
> > The function does not seem to handle _both_ a LICENSE variable and
> > LICENSE_pn variables.  I could not find any documentation regarding this so
> > I'm not sure what should be supported.  Our recipe was using LICENSE="MIT"
> > and then the LICENSE_pn variables were using other licenses - not MIT.
> 
> So, LICENSE should contain all the licenses for all the packages for that
> recipe whereas LICENSE_pn should be the finer grained license for the
> specific packages. You wouldn't have LICENSE_pn have say, GPL-2.0 without
> having that in LICENSE.

This makes perfect sense to me, but a better error message would be nice.

I was writing a recipe for an external toolchain, and was basing it on the linaro external toolchain recipe. 

That recipe only sets LICENSE to MIT and then in a comment specifies that this is only the license for the recipe.  When I looked at the resulting .spec-file I realised that the license information of the various packages were wrong.  I then set LICENSE for each package separately based on the package content.  This worked on Dizzy, but when we upgraded to Jethro there was a lot of warnings for the licenses, which lead me to this code.
Comment 6 Markus Lehtonen 2016-08-03 13:51:19 UTC
Re-sent the patch and added a patch for LICENSE_<pkg> verification:
https://patchwork.openembedded.org/series/1095/