Re: [OE-core][PATCH] package: Add support for INSANE_SKIP for incompatible-license

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

On Tue, 2024-07-16 at 09:57 +0200, Alexander Kanavin via
lists.openembedded.org wrote:
> On Mon, 15 Jul 2024 at 21:07, Ryan Eatmon via lists.openembedded.org
> <reatmon=ti.com@lists.openembedded.org> wrote:
> > 
> > With the move to make more warnings into errors it is inevitable
> > that we
> > will need more hooks to skip the errors on a recipe by recipe
> > basis.
> > This patch just adds INSANE_SKIP support for the incompatible-
> > license check.
> 
> I do not think this is a good idea. This was a warning before, the
> warning was never fixed, and now, instead of addressing the issue,
> the
> error should be suppressed so that it's *never* going to be fixed?
> 
> You can still revert to a warning if you so wish, but in general,
> global INCOMPATIBLE_LICENSE is essentialy deprecated in favour a
> per-image one, is there a reason you are still using that?

The ERROR_QA change is going to take a bit of adjustment for people.
There are some things in there which recipes will need to tweak for
various reasons (e.g. pre-built binaries). After much thought, I (and
others) concluded it was better to have recipes marked up with the
issues rather than have it as some "random" warning in the build people
ignored. I therefore think it is the right move but we need to support
people in marking up the recipes (and ultimately ideally fixing some of
the issues).

With regard to the patch, I think the key question is whether we want
to add INSANE_SKIP support to every call site (potentially) or whether
there is some better function abstraction we can use.

The implementation in do_package_qa is:

        skip = set((d.getVar('INSANE_SKIP') or "").split() +
                   (d.getVar('INSANE_SKIP:' + package) or "").split())

which shows the first issue with this patch - INSANE_SKIP itself isn't
looked at (for a recipe wide disable).

So I think we need a new common function alongside oe.qa.handle_error()
which adds the pkg option and checks accordingly?

Cheers,

Richard