[PATCH v4 1/2] meta: license: fix non-SPDX license being removed from INCOMPATIBLE_LICENSE

[PATCH v4 1/2] meta: license: fix non-SPDX license being removed from INCOMPATIBLE_LICENSE

[Quentin Schulz]

A non-SPDX license (which is not an alias to an SPDX license) cannot
currently be marked as incompatible in INCOMPATIBLE_LICENSE.
In the current state, we take all INCOMPATIBLE_LICENSE and pass them
through expand_wildcard_licenses which is only adding SPDX licenses that
match the glob regexp of what is in INCOMPATIBLE_LICENSE (be it a direct
match to an SPDX license or via an alias).

This does not work well with custom licenses.

E.g.:

foo.bb:
LICENSE = "FooLicense"

conf/local.conf:
INCOMPATIBLE_LICENSE = "FooLicense"

`bitbake foo`

Gives no warning, no error, builds and packages successfully, because
INCOMPATIBLE_LICENSE is basically empty since FooLicense is neither in
SPDXLICENSEMAP nor in SRC_DISTRIBUTE_LICENSES.

Let's add the original licenses to the list returned by
expand_wildcard_licenses to be able to handle the aforementioned case.

INCOMPATIBLE_LICENSE = "FooLicense GPLv2 GPLv3+" used to "resolve" to
"GPLv2 GPLv3". It now resolves to "FooLicense GPLv2 GPLv3 GPLv3+" which
fixes the issue with custom licenses not being in SPDXLICENSEMAP or
SRC_DISTRIBUTE_LICENSES and thus being left out of the blacklisted
licenses.

I needed to pass a list to expand_wildcard_licenses from the
license_image class instead of the current output of map() because the
operator [:] does not work on this kind of type, and list(map()) or
anything that iterates over map() actually moves the iterator and breaks
the forloop right after in expand_wildcard_licenses.

Signed-off-by: Quentin Schulz <quentin.schulz@streamunlimited.com>
---

v3:
  - fix git context from v2, patch cleanly applies on master branch now,

v2:
  - fixed image building by replacing map(lambda) by a comprehensive list so
that we have a consistent input type for expand_wildcard_licenses,

 meta/classes/license.bbclass       | 2 +-
 meta/classes/license_image.bbclass | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/license.bbclass b/meta/classes/license.bbclass
index ed91a4b4db..adca881c85 100644
--- a/meta/classes/license.bbclass
+++ b/meta/classes/license.bbclass
@@ -268,7 +268,7 @@ def expand_wildcard_licenses(d, wildcard_licenses):
     wildcards from SPDXLICENSEMAP flags and SRC_DISTRIBUTE_LICENSES values.
     """
     import fnmatch
-    licenses = []
+    licenses = wildcard_licenses[:]
     spdxmapkeys = d.getVarFlags('SPDXLICENSEMAP').keys()
     for wld_lic in wildcard_licenses:
         spdxflags = fnmatch.filter(spdxmapkeys, wld_lic)
diff --git a/meta/classes/license_image.bbclass b/meta/classes/license_image.bbclass
index 6fb76be48e..2cfda81c99 100644
--- a/meta/classes/license_image.bbclass
+++ b/meta/classes/license_image.bbclass
@@ -40,7 +40,7 @@ def write_license_files(d, license_manifest, pkg_dic, rootfs=True):
     import stat
 
     bad_licenses = (d.getVar("INCOMPATIBLE_LICENSE") or "").split()
-    bad_licenses = map(lambda l: canonical_license(d, l), bad_licenses)
+    bad_licenses = [canonical_license(d, l) for l in bad_licenses]
     bad_licenses = expand_wildcard_licenses(d, bad_licenses)
 
     with open(license_manifest, "w") as license_file:
-- 
2.17.1

[PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Quentin Schulz]

One bug went unnoticed without these selftests: an INCOMPATIBLE_LICENSE
with a non-SPDX license for a package with that non-SPDX license wasn't
enforcing the denial of build for said package.

While adding a test for that particular case, let's add a few more so
that we cover a handful more use cases of INCOMPATIBLE_LICENSE.

Signed-off-by: Quentin Schulz <quentin.schulz@streamunlimited.com>
---

added in v4

 .../license/incompatible-license-alias.bb     |  3 ++
 .../license/incompatible-license.bb           |  3 ++
 .../license/incompatible-nonspdx-license.bb   |  3 ++
 .../oeqa/selftest/cases/incompatible_lic.py   | 42 +++++++++++++++++++
 4 files changed, 51 insertions(+)
 create mode 100644 meta-selftest/recipes-test/license/incompatible-license-alias.bb
 create mode 100644 meta-selftest/recipes-test/license/incompatible-license.bb
 create mode 100644 meta-selftest/recipes-test/license/incompatible-nonspdx-license.bb
 create mode 100644 meta/lib/oeqa/selftest/cases/incompatible_lic.py

diff --git a/meta-selftest/recipes-test/license/incompatible-license-alias.bb b/meta-selftest/recipes-test/license/incompatible-license-alias.bb
new file mode 100644
index 0000000000..e0b4e13c26
--- /dev/null
+++ b/meta-selftest/recipes-test/license/incompatible-license-alias.bb
@@ -0,0 +1,3 @@
+SUMMARY = "Recipe with an alias of an SPDX license"
+DESCRIPTION = "Is licensed with an alias of an SPDX license to be used for testing"
+LICENSE = "GPLv3"
diff --git a/meta-selftest/recipes-test/license/incompatible-license.bb b/meta-selftest/recipes-test/license/incompatible-license.bb
new file mode 100644
index 0000000000..1728ad76b7
--- /dev/null
+++ b/meta-selftest/recipes-test/license/incompatible-license.bb
@@ -0,0 +1,3 @@
+SUMMARY = "Recipe with an SPDX license"
+DESCRIPTION = "Is licensed with an SPDX license to be used for testing"
+LICENSE = "GPL-3.0"
diff --git a/meta-selftest/recipes-test/license/incompatible-nonspdx-license.bb b/meta-selftest/recipes-test/license/incompatible-nonspdx-license.bb
new file mode 100644
index 0000000000..35af0966ef
--- /dev/null
+++ b/meta-selftest/recipes-test/license/incompatible-nonspdx-license.bb
@@ -0,0 +1,3 @@
+SUMMARY = "Recipe with a non-SPDX license"
+DESCRIPTION = "Is licensed with a non-SPDX license to be used for testing"
+LICENSE = "FooLicense"
diff --git a/meta/lib/oeqa/selftest/cases/incompatible_lic.py b/meta/lib/oeqa/selftest/cases/incompatible_lic.py
new file mode 100644
index 0000000000..4e0d1f53d3
--- /dev/null
+++ b/meta/lib/oeqa/selftest/cases/incompatible_lic.py
@@ -0,0 +1,42 @@
+from oeqa.selftest.case import OESelftestTestCase
+from oeqa.utils.commands import bitbake
+from oeqa.core.decorator.oeid import OETestID
+
+class IncompatibleLicenseTests(OESelftestTestCase):
+
+    def test_lic(self, pn, pn_lic, lic):
+        error_msg = 'ERROR: Nothing PROVIDES \'%s\'\n%s was skipped: it has an incompatible license: %s' % (pn, pn, pn_lic)
+
+        self.write_config("INCOMPATIBLE_LICENSE += \"%s\"" % (lic))
+
+        result = bitbake(pn, ignore_status=True)
+        if error_msg not in result.output:
+            raise AssertionError(result.output)
+
+    # Verify that a package with an SPDX license (from SRC_DISTRIBUTE_LICENSES)
+    # cannot be built when INCOMPATIBLE_LICENSE contains this SPDX license
+    def test_incompatible_spdx_license(self):
+        self.test_lic('incompatible-license', 'GPL-3.0', 'GPL-3.0')
+
+    # Verify that a package with an SPDX license (from SRC_DISTRIBUTE_LICENSES)
+    # cannot be built when INCOMPATIBLE_LICENSE contains an alias (in
+    # SPDXLICENSEMAP) of this SPDX license
+    def test_incompatible_alias_spdx_license(self):
+        self.test_lic('incompatible-license', 'GPL-3.0', 'GPLv3')
+
+    # Verify that a package with an alias (from SPDXLICENSEMAP) to an SPDX
+    # license cannot be built when INCOMPATIBLE_LICENSE contains this SPDX
+    # license
+    def test_incompatible_spdx_license_alias(self):
+        self.test_lic('incompatible-license-alias', 'GPLv3', 'GPL-3.0')
+
+    # Verify that a package with an alias (from SPDXLICENSEMAP) to an SPDX
+    # license cannot be built when INCOMPATIBLE_LICENSE contains this alias
+    def test_incompatible_alias_spdx_license_alias(self):
+        self.test_lic('incompatible-license-alias', 'GPLv3', 'GPLv3')
+
+    # Verify that a package with a non-SPDX license (neither in
+    # SRC_DISTRIBUTE_LICENSES nor in SPDXLICENSEMAP) cannot be built when
+    # INCOMPATIBLE_LICENSE contains this license
+    def test_incompatible_nonspdx_license(self):
+        self.test_lic('incompatible-nonspdx-license', 'FooLicense', 'FooLicense')
-- 
2.17.1

✗ patchtest: failure for "[v4] meta: license: fix non-SP..." and 1 more

[Patchwork]

== Series Details ==

Series: "[v4] meta: license: fix non-SP..." and 1 more
Revision: 1
URL   : https://patchwork.openembedded.org/series/18087/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at f79c95f6a8)



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Burton, Ross]

On Tue, 11 Jun 2019 at 15:27, Quentin Schulz
<quentin.schulz@streamunlimited.com> wrote:
> +from oeqa.core.decorator.oeid import OETestID

This was removed a while ago, can you rebase the patches on top of
master please?

In this case you can just remove the import, but please verify the
behaviour with master.

Ross

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Quentin Schulz]

Hi Ross,

On Tue, Jun 11, 2019 at 03:49:31PM +0100, Burton, Ross wrote:
> On Tue, 11 Jun 2019 at 15:27, Quentin Schulz
> <quentin.schulz@streamunlimited.com> wrote:
> > +from oeqa.core.decorator.oeid import OETestID
> 
> This was removed a while ago, can you rebase the patches on top of
> master please?
> 
> In this case you can just remove the import, but please verify the
> behaviour with master.
> 

Sorry for the noise, apparently forgot to pull master and didn't even
see my patch was already there *sigh*.

I'll remove the import and test on master with and without the patch you
already merged.

Do you want me to send a v5 or a separate patch series is better for
you?

Quentin
-- 
StreamUnlimited Engineering GmbH
High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, 1100 Vienna, Austria
quentin.schulz@streamunlimited.com, www.streamunlimited.com

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Burton, Ross]

On Tue, 11 Jun 2019 at 15:52, Quentin Schulz
<quentin.schulz@streamunlimited.com> wrote:
> Sorry for the noise, apparently forgot to pull master and didn't even
> see my patch was already there *sigh*.
>
> I'll remove the import and test on master with and without the patch you
> already merged.
>
> Do you want me to send a v5 or a separate patch series is better for
> you?

Either is fine.  Note that in the broken case - where the recipe isn't
skipped - the test goes ahead and compiles stuff.  Can you also try
using --dry-run to see if that results in the same test behaviour, but
without attempting compilation.

Ross

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Quentin Schulz]

On Tue, Jun 11, 2019 at 04:52:16PM +0200, Quentin Schulz wrote:
> Hi Ross,
> 
> On Tue, Jun 11, 2019 at 03:49:31PM +0100, Burton, Ross wrote:
> > On Tue, 11 Jun 2019 at 15:27, Quentin Schulz
> > <quentin.schulz@streamunlimited.com> wrote:
> > > +from oeqa.core.decorator.oeid import OETestID
> > 
> > This was removed a while ago, can you rebase the patches on top of
> > master please?
> > 
> > In this case you can just remove the import, but please verify the
> > behaviour with master.
> > 
> 
> Sorry for the noise, apparently forgot to pull master and didn't even
> see my patch was already there *sigh*.
> 
> I'll remove the import and test on master with and without the patch you
> already merged.
> 
> Do you want me to send a v5 or a separate patch series is better for
> you?
> 

Also, I factored out everything into a function that shouldn't be run as
a test but when doing:
oe-selftest --run-tests incompatible_lic.IncompatibleLicenseTests
it tries to run the function and thus fails the test. Is that okay? if
not, how do you usually proceed?

Quentin

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Burton, Ross]

On Tue, 11 Jun 2019 at 15:56, Quentin Schulz
<quentin.schulz@streamunlimited.com> wrote:
> Also, I factored out everything into a function that shouldn't be run as
> a test but when doing:
> oe-selftest --run-tests incompatible_lic.IncompatibleLicenseTests
> it tries to run the function and thus fails the test. Is that okay? if
> not, how do you usually proceed?

So Python unittest's behaviour is that any function called test_*() is
a test case and is executed, so just don't name any helper functions
like this test_*.

Ross

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Burton, Ross]

On Tue, 11 Jun 2019 at 15:55, Burton, Ross <ross.burton@intel.com> wrote:
> Either is fine.  Note that in the broken case - where the recipe isn't
> skipped - the test goes ahead and compiles stuff.  Can you also try
> using --dry-run to see if that results in the same test behaviour, but
> without attempting compilation.

FWIW:

-        result = bitbake(pn, ignore_status=True)
+        result = bitbake("%s --dry-run" % pn, ignore_status=True)

Still results in the nonspdx case failing as expected without the fix,
but without any compilation attempted.

Ross

Re: [PATCH v4 2/2] selftests: add tests for INCOMPATIBLE_LICENSE

[Quentin Schulz]

On Tue, Jun 11, 2019 at 03:57:23PM +0100, Burton, Ross wrote:
> On Tue, 11 Jun 2019 at 15:56, Quentin Schulz
> <quentin.schulz@streamunlimited.com> wrote:
> > Also, I factored out everything into a function that shouldn't be run as
> > a test but when doing:
> > oe-selftest --run-tests incompatible_lic.IncompatibleLicenseTests
> > it tries to run the function and thus fails the test. Is that okay? if
> > not, how do you usually proceed?
> 
> So Python unittest's behaviour is that any function called test_*() is
> a test case and is executed, so just don't name any helper functions
> like this test_*.
> 

Ah, that's what I was missing. Thanks.

Quentin
-- 
StreamUnlimited Engineering GmbH
High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, 1100 Vienna, Austria
quentin.schulz@streamunlimited.com, www.streamunlimited.com