Re: [OE-core] [PATCH v5 3/7] insane.bbclass: Make do_qa_staging check shebangs

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

On Fri, 2022-06-17 at 17:50 +0100, Richard Purdie via
lists.openembedded.org wrote:
> On Tue, 2022-06-14 at 17:11 +0200, Paulo Neves wrote:
> > As reported in the bug report [1], there was no check for shebang
> > sizes on native scripts and now this is fixed.
> > 
> > The path scope of the qa_staging was increased from just checking
> > libdir to all the relevant SYSROOT_DIRS.
> > 
> > It is possible to skip this check through INSANE_SKIP.
> > 
> > [1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=11053
> > 
> > Signed-off-by: Paulo Neves <ptsneves@gmail.com>
> > ---
> >  meta/classes/insane.bbclass | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> > index 9ca84bace9..b2951a48fe 100644
> > --- a/meta/classes/insane.bbclass
> > +++ b/meta/classes/insane.bbclass
> > @@ -630,6 +630,11 @@ def qa_check_staged(path,d):
> >          bb.note("Recipe %s skipping qa checking: pkgconfig" % d.getVar('PN'))
> >          skip_pkgconfig = True
> >  
> > +    skip_shebang_size = False
> > +    if 'shebang-size' in skip:
> > +        bb.note("Recipe %s skipping qa checkking: shebang-size" % d.getVar('PN'))
> > +        skip_shebang_size = True
> > +
> >      # find all .la and .pc files
> >      # read the content
> >      # and check for stuff that looks wrong
> > @@ -651,6 +656,13 @@ def qa_check_staged(path,d):
> >                          error_msg = "%s failed sanity test (tmpdir) in path %s" % (file,root)
> >                          oe.qa.handle_error("pkgconfig", error_msg, d)
> >  
> > +            if not skip_shebang_size:
> > +                errors = {}
> > +                package_qa_check_shebang_size(path, "", d, None, errors)
> > +                for e in errors:
> > +                    oe.qa.handle_error(e, errors[e], d)
> > +
> > +
> >  # Run all package-wide warnfuncs and errorfuncs
> >  def package_qa_package(warnfuncs, errorfuncs, package, d):
> >      warnings = {}
> > @@ -1139,7 +1151,9 @@ addtask do_package_qa_setscene
> >  
> >  python do_qa_staging() {
> >      bb.note("QA checking staging")
> > -    qa_check_staged(d.expand('${SYSROOT_DESTDIR}${libdir}'), d)
> > +    sysroot_destdir = d.expand('${SYSROOT_DESTDIR}')
> > +    for sysroot_dir in d.expand('${SYSROOT_DIRS}').split():
> > +        qa_check_staged(sysroot_destdir + sysroot_dir, d)
> >      oe.qa.exit_with_message_if_errors("QA staging was broken by the package built above", d)
> >  }
> 
> 
> I'm a little worried about the performance implications of this, we're
> going from scanning files in libdir to scanning nearly all files in the
> sysroots, reading from many of them. In isolation that doesn't seem
> much but I suspect the IO will impact builds overall.
> 
> That leaves me a little torn on this change :/

Sorry about the delay on this. My concern isn't an issue as I wasn't
remembering the exact context of this code correctly. The patches have
therefore merged, thanks!

Cheers,

Richard