Message ID | 20220614151105.1890454-3-ptsneves@gmail.com |
---|---|
State | Accepted, archived |
Commit | 377fe11bc0d6939ab1aaebab1bf4e55adca1ab15 |
Headers | show |
Series | [v5,1/7] python: Avoid shebang overflow on python-config.py | expand |
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 :/ Cheers, Richard
Yes, when I was making the change I also realized the scope of the check was quite bigger than before. I am going to deliver the final changes requested and leave it up to you to decide on the merge. Let me know your decision so I can close the bug as well. Paulo Neves On Fri, Jun 17, 2022, 18:50 Richard Purdie < richard.purdie@linuxfoundation.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 :/ > > Cheers, > > Richard > >
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
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) }
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(-)