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

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Fri, 2021-10-15 at 15:17 +0100, Mike Crowe wrote:
> 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.

I'd prefer it to stay in ERROR_QA. I guess it standardises things to add the
exit_if_errors call.

Cheers,

Richard