Re: [OE-core] [PATCH v4 1/2] lib/oe/qa,insane: Move extra error handling functions to library

[Mike Crowe <mac@mcrowe.com>]

On Friday 15 October 2021 at 15:09:08 +0100, Richard Purdie wrote:
> On Fri, 2021-10-15 at 14:59 +0100, Mike Crowe via lists.openembedded.org wrote:
> > Extract package_qa_write_error, package_qa_handle_error and
> > package_qa_add_message functions from insane.bbclass to lib/oe/qa.py and
> > drop the package_qa_ prefixes.
> > 
> > Update various bbclasses to use the new functions. No import is required
> > since base.bbclass puts oe.qa in OE_IMPORTS.
> > 
> > Stop requiring callers to manually track whether a fatal error has been
> > encountered via a "sane" flag. Instead replace the QA_SANE variable with
> > QA_ERRORS_FOUND and call oe.qa.exit_if_errors or
> > oe.qa.exit_with_message_if_errors at the end of each task.
> > 
> > Inspired by discussion resulting from
> > https://lists.openembedded.org/g/openembedded-core/message/156793 and
> > https://lists.openembedded.org/g/openembedded-core/message/156900
> > 
> > Signed-off-by: Mike Crowe <mac@mcrowe.com>
> > ---
> >  meta/classes/buildhistory.bbclass |   3 +-
> >  meta/classes/insane.bbclass       | 180 ++++++++++++------------------
> >  meta/classes/multilib.bbclass     |   3 +-
> >  meta/classes/package.bbclass      |  26 ++---
> >  meta/classes/ptest.bbclass        |   2 +-
> >  meta/lib/oe/qa.py                 |  34 ++++++
> >  6 files changed, 121 insertions(+), 127 deletions(-)
> > 
> > diff --git a/meta/classes/buildhistory.bbclass b/meta/classes/buildhistory.bbclass
> > index 7c44fec2d1..62d0d781a1 100644
> > --- a/meta/classes/buildhistory.bbclass
> > +++ b/meta/classes/buildhistory.bbclass
> > @@ -287,7 +287,7 @@ python buildhistory_emit_pkghistory() {
> >              r = bb.utils.vercmp((pkge, pkgv, pkgr), (last_pkge, last_pkgv, last_pkgr))
> >              if r < 0:
> >                  msg = "Package version for package %s went backwards which would break package feeds (from %s:%s-%s to %s:%s-%s)" % (pkg, last_pkge, last_pkgv, last_pkgr, pkge, pkgv, pkgr)
> > -                package_qa_handle_error("version-going-backwards", msg, d)
> > +                oe.qa.handle_error("version-going-backwards", msg, d)
> >  
> >          pkginfo = PackageInfo(pkg)
> >          # Apparently the version can be different on a per-package basis (see Python)
> > @@ -321,6 +321,7 @@ python buildhistory_emit_pkghistory() {
> >  
> >      # Create files-in-<package-name>.txt files containing a list of files of each recipe's package
> >      bb.build.exec_func("buildhistory_list_pkg_files", d)
> > +    oe.qa.exit_if_errors(d)
> 
> This is a change in behaviour since currently buildhistory doesn't do that and
> I'm not sure it should.

How about moving version-going-backwards from ERROR_QA to WARN_QA and
keeping the call to oe.qa.exit_if_errors?

It's somewhat confusing having version-going-backwards in ERROR_QA now if
it's not going to be fatal when all the other checks in that variable are
fatal. It also means that any future callers of oe.qa.handle_error from
this task would behave the same.

> 
> >  }
> >  
> >  python buildhistory_emit_outputsigs() {
> > diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> > index 433e4dfa33..2df5edf138 100644
> > --- a/meta/classes/insane.bbclass
> > +++ b/meta/classes/insane.bbclass
> > @@ -18,7 +18,7 @@
> >  #   files under exec_prefix
> >  #  -Check if the package name is upper case
> >  
> > -QA_SANE = "True"
> > +QA_ERRORS_FOUND = "False"
> > 
> 
> Lets just delete this please, we don't need a default value and it is confusing
> being isolated from the other code.

OK. I'll await your response to the above question and then send v5.

Thanks.

Mike.