Message ID | 20240715190738.32206-1-reatmon@ti.com |
---|---|
State | New |
Headers | show |
Series | package: Add support for INSANE_SKIP for incompatible-license | expand |
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
diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py index e6b46a0dc5..a2b9019db6 100644 --- a/meta/lib/oe/package.py +++ b/meta/lib/oe/package.py @@ -1411,8 +1411,11 @@ def populate_packages(d): for pkg in packages: licenses = d.getVar('_exclude_incompatible-' + pkg) if licenses: - msg = "Excluding %s from packaging as it has incompatible license(s): %s" % (pkg, licenses) - oe.qa.handle_error("incompatible-license", msg, d) + if "incompatible-license" in (d.getVar('INSANE_SKIP:' + pn) or "").split(): + bb.note("Package %s skipping QA tests: incompatible-license" % pn) + else: + msg = "Excluding %s from packaging as it has incompatible license(s): %s" % (pkg, licenses) + oe.qa.handle_error("incompatible-license", msg, d) else: package_list.append(pkg) d.setVar('PACKAGES', ' '.join(package_list))
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(-)