Re: [OE-core] [poky-contrib][PATCH 3/4] package.bbclass: Add handle_qa_error check

["Christopher Larson" <kergoth@gmail.com>]

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

Indeed, bitbake only requires python 3.5.0 right now, we can't use 3.6 or
3.7 features in it or oe-core at this time.

On Fri, Mar 12, 2021 at 9:44 AM Peter Kjellerstedt <
peter.kjellerstedt@axis.com> wrote:

> > -----Original Message-----
> > From: openembedded-core@lists.openembedded.org <openembedded-
> > core@lists.openembedded.org> On Behalf Of Meh Mbeh Ida Delphine
> > Sent: den 10 mars 2021 00:31
> > To: openembedded-core@lists.openembedded.org
> > Subject: [OE-core] [poky-contrib][PATCH 3/4] package.bbclass: Add
> > handle_qa_error check
> >
> > package_qa_handle_error() from insane.bbclass handles display of warnings
> > by allowing the user decide whether they want the issues treated as
> > warnings, errors or hidden completely. This is done by setting a variable
> > "license_source_spdx" in local.conf.
> >
> > Signed-off-by: Ida Delphine <idadelm@gmail.com>
> > ---
> >  meta/classes/package.bbclass | 128 ++++++++++++++++++-----------------
> >  1 file changed, 65 insertions(+), 63 deletions(-)
> >
> > diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
> > index 4e1be661b3..8c0a49ad76 100644
> > --- a/meta/classes/package.bbclass
> > +++ b/meta/classes/package.bbclass
> > @@ -1552,6 +1552,7 @@ PKGDATA_VARS = "PN PE PV PR PKGE PKGV PKGR LICENSE
> > DESCRIPTION SUMMARY RDEPENDS
> >
> >  python emit_pkgdata() {
> >      import oe.license
> > +    import bb.utils
> >      from glob import glob
> >      import json
> >      import subprocess
> > @@ -1712,70 +1713,71 @@ fi
> >          write_extra_runtime_pkgs(global_variants, packages, pkgdatadir)
> >
> >      sourceresult = d.getVar('TEMPDBGSRCMAPPING', False)
> > -    sources = {}
> > -    if sourceresult:
> > -        for r in sourceresult:
> > -            sources[r[0]] = r[1]
> > -        with open(data_file + ".srclist", 'w') as f:
> > -            f.write(json.dumps(sources, sort_keys=True))
> > -
> > -        filelics = {}
> > -        for dirent in [d.getVar('PKGD'),
> d.getVar('STAGING_DIR_TARGET')]:
> > -            p = subprocess.Popen(["grep", 'SPDX-License-Identifier:',
> '-R', '-I'], stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=dirent)
> > -            out, err = p.communicate()
> > -            if p.returncode == 0:
> > -                for l in out.decode("utf-8").split("\n"):
> > -                    l = l.strip()
> > -                    if not l:
> > -                        continue
> > -                    l = l.split(":")
> > -                    if len(l) < 3:
> > -                        bb.warn(str(l))
> > -                        continue
> > -                    fn = "/" + l[0]
> > -                    lic = l[2].strip()
> > -                    if lic.endswith("*/"):
> > -                        lic = lic[:-2]
> > -                    lic = lic.strip()
> > -                    filelics[fn] = lic
> > -        with open(data_file + ".filelics", 'w') as f:
> > -            f.write(json.dumps(filelics, sort_keys=True))
> > -
> > -        computedlics = {}
> > -        computedpkglics = {}
> > -        for r in sourceresult:
> > -            for subf in r[1]:
> > -                if subf in filelics:
> > -                    if r[0] not in computedlics:
> > -                        computedlics[r[0]] = set()
> > -                    computedlics[r[0]].add(filelics[subf])
> > -
> > -
> > -        dvar = d.getVar('PKGD')
> > -        for f in computedlics:
> > -            shortf = f.replace(dvar, "")
> > -            found = False
> > -            for pkg in filemap:
> > -                if shortf in filemap[pkg]:
> > -                    found = True
> > -                    if pkg not in computedpkglics:
> > -                        computedpkglics[pkg] = set()
> > -                    computedpkglics[pkg].update(computedlics[f])
> > -            if not found:
> > -                bb.warn("%s not in %s" % (f, str(filemap)))
> > -
> > -        for pkg in computedpkglics:
> > -            lic = d.getVar('LICENSE_%s' % (pkg))
> > -            if not lic:
> > -                lic = d.getVar('LICENSE')
> > -
> > -            # Splits the LICENSE values and canonicalise each license
> > -            # in the set of split license(s)
> > -            spdx_lic = oe.license.split_spdx_lic(lic, d)
> > +    if bb.utils.contains("license_source_spdx", "1", True, False, d):
>
> This kind of bitbake variable that controls functionality is typically in
> upper case. It also needs to be documented in the manual, and it should
> have
> a default value (??=). It would also make more sense to use
> bb.utils.to_boolean() than bb.utils.contains(). You can also do:
>
>     if not bb.utils.to_boolean(d.getVar("license_source_spdx")):
>         return
>
> thereby avoiding the reindentation, making code review easier.
>
> > +        sources = {}
> > +        if sourceresult:
> > +            for r in sourceresult:
> > +                sources[r[0]] = r[1]
> > +            with open(data_file + ".srclist", 'w') as f:
> > +                f.write(json.dumps(sources, sort_keys=True))
> > +
> > +            filelics = {}
> > +            for dirent in [d.getVar('PKGD'),
> d.getVar('STAGING_DIR_TARGET')]:
> > +                p = subprocess.Popen(["grep",
> 'SPDX-License-Identifier:', '-R', '-I'], stdout=subprocess.PIPE,
> stderr=subprocess.PIPE, cwd=dirent)
> > +                out, err = p.communicate()
> > +                if p.returncode == 0:
> > +                    for l in out.decode("utf-8").split("\n"):
> > +                        l = l.strip()
> > +                        if not l:
> > +                            continue
> > +                        l = l.split(":")
> > +                        if len(l) < 3:
> > +                            bb.warn(str(l))
> > +                            continue
> > +                        fn = "/" + l[0]
> > +                        lic = l[2].strip()
> > +                        if lic.endswith("*/"):
> > +                            lic = lic[:-2]
> > +                        lic = lic.strip()
> > +                        filelics[fn] = lic
> > +            with open(data_file + ".filelics", 'w') as f:
> > +                f.write(json.dumps(filelics, sort_keys=True))
> > +
> > +            computedlics = {}
> > +            computedpkglics = {}
> > +            for r in sourceresult:
> > +                for subf in r[1]:
> > +                    if subf in filelics:
> > +                        if r[0] not in computedlics:
> > +                            computedlics[r[0]] = set()
> > +                        computedlics[r[0]].add(filelics[subf])
> > +
> > +
> > +            dvar = d.getVar('PKGD')
> > +            for f in computedlics:
> > +                shortf = f.replace(dvar, "")
> > +                found = False
> > +                for pkg in filemap:
> > +                    if shortf in filemap[pkg]:
> > +                        found = True
> > +                        if pkg not in computedpkglics:
> > +                            computedpkglics[pkg] = set()
> > +                        computedpkglics[pkg].update(computedlics[f])
> > +                if not found:
> > +                    bb.warn("%s not in %s" % (f, str(filemap)))
> >
> > -            # Displays warnings for licenses found in the recipes and
> not sources
> > -            if spdx_lic - computedpkglics[pkg]:
> > -                bb.warn("License for package %s is %s vs %s" % (pkg,
> computedpkglics[pkg], spdx_lic))
> > +            for pkg in computedpkglics:
> > +                lic = d.getVar('LICENSE_%s' % (pkg))
> > +                if not lic:
> > +                    lic = d.getVar('LICENSE')
> > +
> > +                # Splits the LICENSE values and canonicalise each
> license
> > +                # in the set of split license(s)
> > +                spdx_lic = oe.license.split_spdx_lic(lic, d)
> > +
> > +                # Displays warnings for licenses found in the recipes
> and not sources
> > +                if spdx_lic - computedpkglics[pkg]:
> > +                    package_qa_handle_error("license_source_spdx",
> f'License for package {pkg} is {computedpkglics[pkg]}(source SPDX headers)
> vs {spdx_lic} (LICENSE)', d)
>
> I'm all for using f'' strings, as I think it makes for a lot more readable
> code. However, I am not sure it has been approved for use in the OE code
> yet.
>
> >  }
> >  emit_pkgdata[dirs] = "${PKGDESTWORK}/runtime
> ${PKGDESTWORK}/runtime-reverse ${PKGDESTWORK}/runtime-rprovides"
> >
> > --
> > 2.25.1
>
>
> 
>
>

-- 
Christopher Larson
kergoth at gmail dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Senior Software Engineer, Mentor Graphics

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