Re: [OE-core] [PATCH v3 2/2] license: Allow treating missing license as error

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

On Thu, 2021-10-14 at 18:09 +0100, Mike Crowe wrote:
> On Thursday 14 October 2021 at 17:54:18 +0100, Richard Purdie wrote:
> > On Thu, 2021-10-14 at 17:42 +0100, Mike Crowe via lists.openembedded.org wrote:
> > > On Wednesday 13 October 2021 at 11:48:05 +0100, Mike Crowe wrote:
> > > > Use the same WARN_WA and ERROR_QA variables as insane.bbclass to allow
> > > > individual recipes, the distro or other configuration to determine
> > > > whether a missing licence should be treated as a warning (as it is now)
> > > > or as an error.
> > > > 
> > > > oe.qa.handle_error isn't immediately fatal, so track the overall sanity
> > > > state and call bb.fatal if required at the end to ensure that the task
> > > > really fails. If only bb.error is used then do_populate_lic isn't re-run
> > > > on subsequent builds which could lead to the error being missed.
> > > > 
> > > > It seems odd for the license- error classes to be listed in
> > > > insane.bbclass but implemented in license.bbclass. All recommendations
> > > > for somewhere else to put them gratefully received.
> 
> [snip]
> 
> > > Patch series v4 coming soon.
> > 
> > I did have to rework it a bit as I also ran into a few issues and was just
> > trying to get around to writing them up.
> > 
> > I added:
> > 
> > http://git.yoctoproject.org/cgit.cgi/poky/commit/?h=master-next&id=9e980f1a71e8b6af5ff6da9b02fac0f8bfd9d4fb
> > 
> > which means you can drop the imports.
> > 
> > You can use the nonlocal option to help the "sane" problem:
> > 
> > http://git.yoctoproject.org/cgit.cgi/poky/diff/meta/classes/license.bbclass?h=master-next&id=110fa545ac84e560691c7d9e0d1e6e8f70c66980
> > 
> > The autobuilder also shows issues as some of the functions you moved are
> > referenced in other classes. The patch became:
> > 
> > http://git.yoctoproject.org/cgit.cgi/poky/commit/?h=master-next&id=110fa545ac84e560691c7d9e0d1e6e8f70c66980
> 
> Thanks for doing that. I thought I'd searched for other occurrences, but I
> must have not done so properly. :(
> 
> > I was also just thinking we should probably add a new function and add calls at
> > the end of the tasks like:
> > 
> > oe.qa.exit_if_errors(d)
> > 
> > with a definition similar to:
> > 
> > def exit_if_errors(d):
> >     qa_sane = d.getVar("QA_SANE")
> >     if not qa_sane:
> >         bb.fatal("Fatal QA errors were found.")
> > 
> > and then renaming QA_SANE to something like QA_FATAL_ERRORS and exitting if set.
> 
> That sounds like it would neaten things up greatly and avoid the risk of
> accidentally failing to make the error fatal. It ought to mean that many of the sane local
> variables can be removed too.
> 
> Would you like me to provide new versions of the existing patches or add
> new patches on top of the ones already in master-next?

I had half a patch for this last bit so I pushed it into master-next. I also
merged by oe.qa import change.

I'm happy for you to take over from here and rework the patches into whatever
makes the best sense for a series now we know what we're aiming for? I think
there is a more cleanup of the "sane" variables that can be done beyond my quick
patch.

Cheers,

Richard