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 10:00 +0100, Richard Purdie via
lists.openembedded.org wrote:
> 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?

I'd also note this:

# Add the package specific INSANE_SKIPs to the sstate dependencies
python() {
    pkgs = (d.getVar('PACKAGES') or '').split()
    for pkg in pkgs:
        d.appendVarFlag("do_package_qa", "vardeps", " INSANE_SKIP:{}".format(pkg))
}

since people do want things to rebuild correctly when you set/unset one
of these options :/.

That does complicate things a lot for the issue at hand.

Cheers,

Richard