[RFC,1/3] recipetool: Separate licenses with & operator

Message ID 20211213123826.31605-1-stefan.herbrechtsmeier-oss@weidmueller.com
State Accepted, archived
Commit 60a84ecc53d20118c5e7f86dd3e3cafbfed1cf0a
Headers show
Series [RFC,1/3] recipetool: Separate licenses with & operator | expand

Commit Message

Stefan Herbrechtsmeier Dec. 13, 2021, 12:38 p.m. UTC
From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Separate licenses with & operator since it should be satisfied most use
cases and it is a reasonable assumption that all the licenses apply.
Furthermore flat, split and sort the licenses to minimize license string
changes.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 meta/lib/oeqa/selftest/cases/recipetool.py |  4 +--
 scripts/lib/recipetool/create.py           | 39 +++++++++++++++++-----
 2 files changed, 32 insertions(+), 11 deletions(-)

Comments

Alexander Kanavin Dec. 13, 2021, 12:57 p.m. UTC | #1
On Mon, 13 Dec 2021 at 13:38, Stefan Herbrechtsmeier <
stefan.herbrechtsmeier-oss@weidmueller.com> wrote:

> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Separate licenses with & operator since it should be satisfied most use
> cases and it is a reasonable assumption that all the licenses apply.
> Furthermore flat, split and sort the licenses to minimize license string
> changes.
>

I think this is better explained with an example. What did not work before,
and what works with this change?

Alex



>
> Signed-off-by: Stefan Herbrechtsmeier <
> stefan.herbrechtsmeier@weidmueller.com>
> ---
>
>  meta/lib/oeqa/selftest/cases/recipetool.py |  4 +--
>  scripts/lib/recipetool/create.py           | 39 +++++++++++++++++-----
>  2 files changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/meta/lib/oeqa/selftest/cases/recipetool.py
> b/meta/lib/oeqa/selftest/cases/recipetool.py
> index 439e41597c..95e4753976 100644
> --- a/meta/lib/oeqa/selftest/cases/recipetool.py
> +++ b/meta/lib/oeqa/selftest/cases/recipetool.py
> @@ -426,7 +426,7 @@ class RecipetoolCreateTests(RecipetoolBase):
>          checkvars = {}
>          checkvars['SUMMARY'] = 'Node Server Example'
>          checkvars['HOMEPAGE'] = '
> https://github.com/savoirfairelinux/node-server-example#readme'
> -        checkvars['LICENSE'] = set(['MIT', 'ISC', 'Unknown'])
> +        checkvars['LICENSE'] = 'BSD-3-Clause & ISC & MIT & Unknown'
>          urls = []
>          urls.append('npm://
> registry.npmjs.org/;package=@savoirfairelinux/node-server-example;version=${PV}
> <http://registry.npmjs.org/;package=@savoirfairelinux/node-server-example;version=$%7BPV%7D>
> ')
>          urls.append('npmsw://${THISDIR}/${BPN}/npm-shrinkwrap.json')
> @@ -483,7 +483,7 @@ class RecipetoolCreateTests(RecipetoolBase):
>          result = runCmd('recipetool create -o %s %s' % (temprecipe,
> srcuri))
>          self.assertTrue(os.path.isfile(recipefile))
>          checkvars = {}
> -        checkvars['LICENSE'] = set(['PSF', '&', 'BSD-3-Clause', 'GPL'])
> +        checkvars['LICENSE'] = 'BSD-3-Clause & GPL & PSF'
>          checkvars['LIC_FILES_CHKSUM'] =
> 'file://COPYING.txt;md5=35a23d42b615470583563132872c97d6'
>          checkvars['SRC_URI'] = '
> https://files.pythonhosted.org/packages/84/f4/5771e41fdf52aabebbadecc9381d11dea0fa34e4759b4071244fa094804c/docutils-${PV}.tar.gz
> '
>          checkvars['SRC_URI[md5sum]'] = 'c53768d63db3873b7d452833553469de'
> diff --git a/scripts/lib/recipetool/create.py
> b/scripts/lib/recipetool/create.py
> index 406c97f1c5..8e8a621b4f 100644
> --- a/scripts/lib/recipetool/create.py
> +++ b/scripts/lib/recipetool/create.py
> @@ -919,6 +919,22 @@ def split_value(value):
>      else:
>          return value
>
> +def fixup_license(value):
> +    # Ensure licenses with OR starts and ends with brackets
> +    if '|' in value:
> +        return '(' + value + ')'
> +    return value
> +
> +def tidy_licenses(value):
> +    """Flat, split and sort licenses"""
> +    from oe.license import flattened_licenses
> +    def _choose(a, b):
> +        str_a, str_b  = sorted((" & ".join(a), " & ".join(b)),
> key=str.casefold)
> +        return ["(%s | %s)" % (str_a, str_b)]
> +    if not isinstance(value, str):
> +        value = " & ".join(value)
> +    return sorted(list(set(flattened_licenses(value, _choose))),
> key=str.casefold)
> +
>  def handle_license_vars(srctree, lines_before, handled, extravalues, d):
>      lichandled = [x for x in handled if x[0] == 'license']
>      if lichandled:
> @@ -932,10 +948,13 @@ def handle_license_vars(srctree, lines_before,
> handled, extravalues, d):
>      lines = []
>      if licvalues:
>          for licvalue in licvalues:
> -            if not licvalue[0] in licenses:
> -                licenses.append(licvalue[0])
> +            license = licvalue[0]
> +            lics = tidy_licenses(fixup_license(license))
> +            lics = [lic for lic in lics if lic not in licenses]
> +            if len(lics):
> +                licenses.extend(lics)
>              lic_files_chksum.append('file://%s;md5=%s' % (licvalue[1],
> licvalue[2]))
> -            if licvalue[0] == 'Unknown':
> +            if license == 'Unknown':
>                  lic_unknown.append(licvalue[1])
>          if lic_unknown:
>              lines.append('#')
> @@ -944,9 +963,7 @@ def handle_license_vars(srctree, lines_before,
> handled, extravalues, d):
>              for licfile in lic_unknown:
>                  lines.append('#   %s' % licfile)
>
> -    extra_license = split_value(extravalues.pop('LICENSE', []))
> -    if '&' in extra_license:
> -        extra_license.remove('&')
> +    extra_license = tidy_licenses(extravalues.pop('LICENSE', ''))
>      if extra_license:
>          if licenses == ['Unknown']:
>              licenses = extra_license
> @@ -987,7 +1004,7 @@ def handle_license_vars(srctree, lines_before,
> handled, extravalues, d):
>          lines.append('# instead of &. If there is any doubt, check the
> accompanying documentation')
>          lines.append('# to determine which situation is applicable.')
>
> -    lines.append('LICENSE = "%s"' % ' & '.join(licenses))
> +    lines.append('LICENSE = "%s"' % ' & '.join(sorted(licenses,
> key=str.casefold)))
>      lines.append('LIC_FILES_CHKSUM = "%s"' % ' \\\n
> '.join(lic_files_chksum))
>      lines.append('')
>
> @@ -1226,6 +1243,7 @@ def split_pkg_licenses(licvalues, packages,
> outlines, fallback_licenses=[], pn='
>      """
>      pkglicenses = {pn: []}
>      for license, licpath, _ in licvalues:
> +        license = fixup_license(license)
>          for pkgname, pkgpath in packages.items():
>              if licpath.startswith(pkgpath + '/'):
>                  if pkgname in pkglicenses:
> @@ -1238,11 +1256,14 @@ def split_pkg_licenses(licvalues, packages,
> outlines, fallback_licenses=[], pn='
>              pkglicenses[pn].append(license)
>      outlicenses = {}
>      for pkgname in packages:
> -        license = ' '.join(list(set(pkglicenses.get(pkgname,
> ['Unknown'])))) or 'Unknown'
> +        # Assume AND operator between license files
> +        license = ' & '.join(list(set(pkglicenses.get(pkgname,
> ['Unknown'])))) or 'Unknown'
>          if license == 'Unknown' and pkgname in fallback_licenses:
>              license = fallback_licenses[pkgname]
> +        licenses = tidy_licenses(license)
> +        license = ' & '.join(licenses)
>          outlines.append('LICENSE:%s = "%s"' % (pkgname, license))
> -        outlicenses[pkgname] = license.split()
> +        outlicenses[pkgname] = licenses
>      return outlicenses
>
>  def read_pkgconfig_provides(d):
> --
> 2.30.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#159619):
> https://lists.openembedded.org/g/openembedded-core/message/159619
> Mute This Topic: https://lists.openembedded.org/mt/87696347/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
Stefan Herbrechtsmeier Dec. 13, 2021, 1 p.m. UTC | #2
Am 13.12.2021 um 13:57 schrieb Alexander Kanavin:
> On Mon, 13 Dec 2021 at 13:38, Stefan Herbrechtsmeier 
> <stefan.herbrechtsmeier-oss@weidmueller.com 
> <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>> wrote:
> 
>     From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com
>     <mailto:stefan.herbrechtsmeier@weidmueller.com>>
> 
>     Separate licenses with & operator since it should be satisfied most use
>     cases and it is a reasonable assumption that all the licenses apply.
>     Furthermore flat, split and sort the licenses to minimize license string
>     changes.
> 
> 
> I think this is better explained with an example. What did not work 
> before, and what works with this change?

Do you mean something like:

-LICENSE = "MIT ISC Unknown"
+LICENSE = "BSD-3-Clause & ISC & MIT & Unknown"
Stefan Herbrechtsmeier Dec. 13, 2021, 1:01 p.m. UTC | #3
Am 13.12.2021 um 14:00 schrieb Stefan Herbrechtsmeier via 
lists.openembedded.org:
> 
> 
> Am 13.12.2021 um 13:57 schrieb Alexander Kanavin:
>> On Mon, 13 Dec 2021 at 13:38, Stefan Herbrechtsmeier 
>> <stefan.herbrechtsmeier-oss@weidmueller.com 
>> <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>> wrote:
>>
>>     From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com
>>     <mailto:stefan.herbrechtsmeier@weidmueller.com>>
>>
>>     Separate licenses with & operator since it should be satisfied 
>> most use
>>     cases and it is a reasonable assumption that all the licenses apply.
>>     Furthermore flat, split and sort the licenses to minimize license 
>> string
>>     changes.
>>
>>
>> I think this is better explained with an example. What did not work 
>> before, and what works with this change?
> 
> Do you mean something like:
> 
> -LICENSE = "MIT ISC Unknown"
> +LICENSE = "BSD-3-Clause & ISC & MIT & Unknown"

Sorry I mean:

-LICENSE = "MIT ISC Unknown"
+LICENSE = "ISC & MIT & Unknown"
Alexander Kanavin Dec. 13, 2021, 1:15 p.m. UTC | #4
I mean that by reading the patch and the commit message it is not clear
what problem is being fixed. It needs to be explained by showing what
happens when there is no patch, and what happens when there is.

Alex

On Mon, 13 Dec 2021 at 14:01, Stefan Herbrechtsmeier <
stefan.herbrechtsmeier-oss@weidmueller.com> wrote:

> Am 13.12.2021 um 14:00 schrieb Stefan Herbrechtsmeier via
> lists.openembedded.org:
> >
> >
> > Am 13.12.2021 um 13:57 schrieb Alexander Kanavin:
> >> On Mon, 13 Dec 2021 at 13:38, Stefan Herbrechtsmeier
> >> <stefan.herbrechtsmeier-oss@weidmueller.com
> >> <mailto:stefan.herbrechtsmeier-oss@weidmueller.com>> wrote:
> >>
> >>     From: Stefan Herbrechtsmeier <
> stefan.herbrechtsmeier@weidmueller.com
> >>     <mailto:stefan.herbrechtsmeier@weidmueller.com>>
> >>
> >>     Separate licenses with & operator since it should be satisfied
> >> most use
> >>     cases and it is a reasonable assumption that all the licenses apply.
> >>     Furthermore flat, split and sort the licenses to minimize license
> >> string
> >>     changes.
> >>
> >>
> >> I think this is better explained with an example. What did not work
> >> before, and what works with this change?
> >
> > Do you mean something like:
> >
> > -LICENSE = "MIT ISC Unknown"
> > +LICENSE = "BSD-3-Clause & ISC & MIT & Unknown"
>
> Sorry I mean:
>
> -LICENSE = "MIT ISC Unknown"
> +LICENSE = "ISC & MIT & Unknown"
>

Patch

diff --git a/meta/lib/oeqa/selftest/cases/recipetool.py b/meta/lib/oeqa/selftest/cases/recipetool.py
index 439e41597c..95e4753976 100644
--- a/meta/lib/oeqa/selftest/cases/recipetool.py
+++ b/meta/lib/oeqa/selftest/cases/recipetool.py
@@ -426,7 +426,7 @@  class RecipetoolCreateTests(RecipetoolBase):
         checkvars = {}
         checkvars['SUMMARY'] = 'Node Server Example'
         checkvars['HOMEPAGE'] = 'https://github.com/savoirfairelinux/node-server-example#readme'
-        checkvars['LICENSE'] = set(['MIT', 'ISC', 'Unknown'])
+        checkvars['LICENSE'] = 'BSD-3-Clause & ISC & MIT & Unknown'
         urls = []
         urls.append('npm://registry.npmjs.org/;package=@savoirfairelinux/node-server-example;version=${PV}')
         urls.append('npmsw://${THISDIR}/${BPN}/npm-shrinkwrap.json')
@@ -483,7 +483,7 @@  class RecipetoolCreateTests(RecipetoolBase):
         result = runCmd('recipetool create -o %s %s' % (temprecipe, srcuri))
         self.assertTrue(os.path.isfile(recipefile))
         checkvars = {}
-        checkvars['LICENSE'] = set(['PSF', '&', 'BSD-3-Clause', 'GPL'])
+        checkvars['LICENSE'] = 'BSD-3-Clause & GPL & PSF'
         checkvars['LIC_FILES_CHKSUM'] = 'file://COPYING.txt;md5=35a23d42b615470583563132872c97d6'
         checkvars['SRC_URI'] = 'https://files.pythonhosted.org/packages/84/f4/5771e41fdf52aabebbadecc9381d11dea0fa34e4759b4071244fa094804c/docutils-${PV}.tar.gz'
         checkvars['SRC_URI[md5sum]'] = 'c53768d63db3873b7d452833553469de'
diff --git a/scripts/lib/recipetool/create.py b/scripts/lib/recipetool/create.py
index 406c97f1c5..8e8a621b4f 100644
--- a/scripts/lib/recipetool/create.py
+++ b/scripts/lib/recipetool/create.py
@@ -919,6 +919,22 @@  def split_value(value):
     else:
         return value
 
+def fixup_license(value):
+    # Ensure licenses with OR starts and ends with brackets
+    if '|' in value:
+        return '(' + value + ')'
+    return value
+
+def tidy_licenses(value):
+    """Flat, split and sort licenses"""
+    from oe.license import flattened_licenses
+    def _choose(a, b):
+        str_a, str_b  = sorted((" & ".join(a), " & ".join(b)), key=str.casefold)
+        return ["(%s | %s)" % (str_a, str_b)]
+    if not isinstance(value, str):
+        value = " & ".join(value)
+    return sorted(list(set(flattened_licenses(value, _choose))), key=str.casefold)
+
 def handle_license_vars(srctree, lines_before, handled, extravalues, d):
     lichandled = [x for x in handled if x[0] == 'license']
     if lichandled:
@@ -932,10 +948,13 @@  def handle_license_vars(srctree, lines_before, handled, extravalues, d):
     lines = []
     if licvalues:
         for licvalue in licvalues:
-            if not licvalue[0] in licenses:
-                licenses.append(licvalue[0])
+            license = licvalue[0]
+            lics = tidy_licenses(fixup_license(license))
+            lics = [lic for lic in lics if lic not in licenses]
+            if len(lics):
+                licenses.extend(lics)
             lic_files_chksum.append('file://%s;md5=%s' % (licvalue[1], licvalue[2]))
-            if licvalue[0] == 'Unknown':
+            if license == 'Unknown':
                 lic_unknown.append(licvalue[1])
         if lic_unknown:
             lines.append('#')
@@ -944,9 +963,7 @@  def handle_license_vars(srctree, lines_before, handled, extravalues, d):
             for licfile in lic_unknown:
                 lines.append('#   %s' % licfile)
 
-    extra_license = split_value(extravalues.pop('LICENSE', []))
-    if '&' in extra_license:
-        extra_license.remove('&')
+    extra_license = tidy_licenses(extravalues.pop('LICENSE', ''))
     if extra_license:
         if licenses == ['Unknown']:
             licenses = extra_license
@@ -987,7 +1004,7 @@  def handle_license_vars(srctree, lines_before, handled, extravalues, d):
         lines.append('# instead of &. If there is any doubt, check the accompanying documentation')
         lines.append('# to determine which situation is applicable.')
 
-    lines.append('LICENSE = "%s"' % ' & '.join(licenses))
+    lines.append('LICENSE = "%s"' % ' & '.join(sorted(licenses, key=str.casefold)))
     lines.append('LIC_FILES_CHKSUM = "%s"' % ' \\\n                    '.join(lic_files_chksum))
     lines.append('')
 
@@ -1226,6 +1243,7 @@  def split_pkg_licenses(licvalues, packages, outlines, fallback_licenses=[], pn='
     """
     pkglicenses = {pn: []}
     for license, licpath, _ in licvalues:
+        license = fixup_license(license)
         for pkgname, pkgpath in packages.items():
             if licpath.startswith(pkgpath + '/'):
                 if pkgname in pkglicenses:
@@ -1238,11 +1256,14 @@  def split_pkg_licenses(licvalues, packages, outlines, fallback_licenses=[], pn='
             pkglicenses[pn].append(license)
     outlicenses = {}
     for pkgname in packages:
-        license = ' '.join(list(set(pkglicenses.get(pkgname, ['Unknown'])))) or 'Unknown'
+        # Assume AND operator between license files
+        license = ' & '.join(list(set(pkglicenses.get(pkgname, ['Unknown'])))) or 'Unknown'
         if license == 'Unknown' and pkgname in fallback_licenses:
             license = fallback_licenses[pkgname]
+        licenses = tidy_licenses(license)
+        license = ' & '.join(licenses)
         outlines.append('LICENSE:%s = "%s"' % (pkgname, license))
-        outlicenses[pkgname] = license.split()
+        outlicenses[pkgname] = licenses
     return outlicenses
 
 def read_pkgconfig_provides(d):