package: Add support for INSANE_SKIP for incompatible-license

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.

Signed-off-by: Ryan Eatmon <reatmon@ti.com>
---
 meta/lib/oe/package.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

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?

Alex

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

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