| Message ID | 20250815094509.3034151-1-philip.lorenz@bmw.de |
|---|---|
| State | Deferred |
| Delegated to: | Steve Sakoman |
| Headers | show |
| Series | [kirkstone,1/2] insane: Improve patch warning/error handling | expand |
This patch set is causing warnings for various non-oe-core layers used in autobuilder testing: meta-intel https://autobuilder.yoctoproject.org/valkyrie/?#/builders/41/builds/2086/steps/12/logs/warnings meta-virtualization https://autobuilder.yoctoproject.org/valkyrie/?#/builders/89/builds/2099/steps/12/logs/warnings https://autobuilder.yoctoproject.org/valkyrie/?#/builders/89/builds/2099/steps/16/logs/warnings meta-agl https://autobuilder.yoctoproject.org/valkyrie/?#/builders/55/builds/2088/steps/13/logs/warnings meta-gplv2 https://autobuilder.yoctoproject.org/valkyrie/?#/builders/33/builds/1299/steps/11/logs/warnings I'm not inclined to take this patch set since it is adding a lot of noise to my testing. If you submit patches for those layers to deal with the above issues, or work with the layer maintainers to fix them I am happy to reconsider. Thanks! Steve On Fri, Aug 15, 2025 at 2:53 AM Philip Lorenz via lists.openembedded.org <philip.lorenz=bmw.de@lists.openembedded.org> wrote: > > From: Richard Purdie <richard.purdie@linuxfoundation.org> > > Currently, whilst patch errors or warnings are shown, the errors don't stop builds. > The configuration isn't very configurable from WARN_QA and ERROR_QA either. > > This patch: > * Uses the standard mechanisms to handle the patch fuzz warnings/errors > * Makes Upstream-Status checking configurable from WARN/ERROR_QA > * Allows that checking to be used with non-core layers > * Makes patch-fuzz an error by default > > (From OE-Core rev: 76a685bfcf927593eac67157762a53259089ea8a) > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > (cherry picked from commit 3c3fd6a65e8103f74ae382d196d486b31a168b39) > > The backported commit was modified to not mark "patch-fuzz" as an error > by default (which retains compatibility with kirkstone behaviour). > > Signed-off-by: Philip Lorenz <philip.lorenz@bmw.de> > --- > meta/classes/insane.bbclass | 24 ++++++++++++++++-------- > 1 file changed, 16 insertions(+), 8 deletions(-) > > diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass > index f4b4c05e3d8..495bdda652c 100644 > --- a/meta/classes/insane.bbclass > +++ b/meta/classes/insane.bbclass > @@ -28,6 +28,7 @@ WARN_QA ?= " libdir xorg-driver-abi \ > missing-update-alternatives native-last missing-ptest \ > license-exists license-no-generic license-syntax license-format \ > license-incompatible license-file-missing obsolete-license \ > + patch-status-noncore \ > " > ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \ > perms dep-cmp pkgvarcheck perm-config perm-line perm-link \ > @@ -1182,24 +1183,27 @@ python do_qa_patch() { > msg += " devtool modify %s\n" % d.getVar('PN') > msg += " devtool finish --force-patch-refresh %s <layer_path>\n\n" % d.getVar('PN') > msg += "Don't forget to review changes done by devtool!\n" > - if bb.utils.filter('ERROR_QA', 'patch-fuzz', d): > - bb.error(msg) > - elif bb.utils.filter('WARN_QA', 'patch-fuzz', d): > - bb.warn(msg) > - msg = "Patch log indicates that patches do not apply cleanly." > + msg += "\nPatch log indicates that patches do not apply cleanly." > oe.qa.handle_error("patch-fuzz", msg, d) > > # Check if the patch contains a correctly formatted and spelled Upstream-Status > import re > from oe import patch > > + allpatches = False > + if bb.utils.filter('ERROR_QA', 'patch-status-noncore', d) or bb.utils.filter('WARN_QA', 'patch-status-noncore', d): > + allpatches = True > + > coremeta_path = os.path.join(d.getVar('COREBASE'), 'meta', '') > for url in patch.src_patches(d): > (_, _, fullpath, _, _, _) = bb.fetch.decodeurl(url) > > # skip patches not in oe-core > + patchtype = "patch-status-core" > if not os.path.abspath(fullpath).startswith(coremeta_path): > - continue > + patchtype = "patch-status-noncore" > + if not allpatches: > + continue > > kinda_status_re = re.compile(r"^.*upstream.*status.*$", re.IGNORECASE | re.MULTILINE) > strict_status_re = re.compile(r"^Upstream-Status: (Pending|Submitted|Denied|Accepted|Inappropriate|Backport|Inactive-Upstream)( .+)?$", re.MULTILINE) > @@ -1212,9 +1216,13 @@ python do_qa_patch() { > > if not match_strict: > if match_kinda: > - bb.error("Malformed Upstream-Status in patch\n%s\nPlease correct according to %s :\n%s" % (fullpath, guidelines, match_kinda.group(0))) > + msg = "Malformed Upstream-Status in patch\n%s\nPlease correct according to %s :\n%s" % (fullpath, guidelines, match_kinda.group(0)) > + oe.qa.handle_error(patchtype, msg, d) > else: > - bb.error("Missing Upstream-Status in patch\n%s\nPlease add according to %s ." % (fullpath, guidelines)) > + msg = "Missing Upstream-Status in patch\n%s\nPlease add according to %s ." % (fullpath, guidelines) > + oe.qa.handle_error(patchtype, msg, d) > + > + oe.qa.exit_if_errors(d) > } > > python do_qa_configure() { > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#221879): https://lists.openembedded.org/g/openembedded-core/message/221879 > Mute This Topic: https://lists.openembedded.org/mt/114715380/3620601 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [steve@sakoman.com] > -=-=-=-=-=-=-=-=-=-=-=- >
Hi Steve, On 19.08.25 18:42, Steve Sakoman wrote: > This patch set is causing warnings for various non-oe-core layers used > in autobuilder testing: > > meta-intel > https://autobuilder.yoctoproject.org/valkyrie/?#/builders/41/builds/2086/steps/12/logs/warnings > > meta-virtualization > https://autobuilder.yoctoproject.org/valkyrie/?#/builders/89/builds/2099/steps/12/logs/warnings > https://autobuilder.yoctoproject.org/valkyrie/?#/builders/89/builds/2099/steps/16/logs/warnings > > meta-agl > https://autobuilder.yoctoproject.org/valkyrie/?#/builders/55/builds/2088/steps/13/logs/warnings > > meta-gplv2 > https://autobuilder.yoctoproject.org/valkyrie/?#/builders/33/builds/1299/steps/11/logs/warnings > > I'm not inclined to take this patch set since it is adding a lot of > noise to my testing. > > If you submit patches for those layers to deal with the above issues, > or work with the layer maintainers to fix them I am happy to > reconsider. One alternative idea: I can also drop the addition of patch-status-noncore to WARN_QA which avoids raising these warnings for layers (which would have been the better choice to begin with as this ultimately avoids any behaviour changes in the default configuration). Given that the warning merely informs about missing metadata in patch files and this missing metadata was already added to the layers when the QA check was merged to master I'd argue that patching all those layers will just introduce noise with very limited benefit. Does removing it from WARN_QA sound sensible to you? The rationale for backporting this to kirkstone is to make sure that patch-fuzz QA issues, when classified as errors, actually abort the task (and therefore no longer create a sstate cache entry). Otherwise the QA issue will be bypassed as long as the sstate cache entry remains available and only reappear once the offending recipe is rebuilt. Thanks, Philip
diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass index f4b4c05e3d8..495bdda652c 100644 --- a/meta/classes/insane.bbclass +++ b/meta/classes/insane.bbclass @@ -28,6 +28,7 @@ WARN_QA ?= " libdir xorg-driver-abi \ missing-update-alternatives native-last missing-ptest \ license-exists license-no-generic license-syntax license-format \ license-incompatible license-file-missing obsolete-license \ + patch-status-noncore \ " ERROR_QA ?= "dev-so debug-deps dev-deps debug-files arch pkgconfig la \ perms dep-cmp pkgvarcheck perm-config perm-line perm-link \ @@ -1182,24 +1183,27 @@ python do_qa_patch() { msg += " devtool modify %s\n" % d.getVar('PN') msg += " devtool finish --force-patch-refresh %s <layer_path>\n\n" % d.getVar('PN') msg += "Don't forget to review changes done by devtool!\n" - if bb.utils.filter('ERROR_QA', 'patch-fuzz', d): - bb.error(msg) - elif bb.utils.filter('WARN_QA', 'patch-fuzz', d): - bb.warn(msg) - msg = "Patch log indicates that patches do not apply cleanly." + msg += "\nPatch log indicates that patches do not apply cleanly." oe.qa.handle_error("patch-fuzz", msg, d) # Check if the patch contains a correctly formatted and spelled Upstream-Status import re from oe import patch + allpatches = False + if bb.utils.filter('ERROR_QA', 'patch-status-noncore', d) or bb.utils.filter('WARN_QA', 'patch-status-noncore', d): + allpatches = True + coremeta_path = os.path.join(d.getVar('COREBASE'), 'meta', '') for url in patch.src_patches(d): (_, _, fullpath, _, _, _) = bb.fetch.decodeurl(url) # skip patches not in oe-core + patchtype = "patch-status-core" if not os.path.abspath(fullpath).startswith(coremeta_path): - continue + patchtype = "patch-status-noncore" + if not allpatches: + continue kinda_status_re = re.compile(r"^.*upstream.*status.*$", re.IGNORECASE | re.MULTILINE) strict_status_re = re.compile(r"^Upstream-Status: (Pending|Submitted|Denied|Accepted|Inappropriate|Backport|Inactive-Upstream)( .+)?$", re.MULTILINE) @@ -1212,9 +1216,13 @@ python do_qa_patch() { if not match_strict: if match_kinda: - bb.error("Malformed Upstream-Status in patch\n%s\nPlease correct according to %s :\n%s" % (fullpath, guidelines, match_kinda.group(0))) + msg = "Malformed Upstream-Status in patch\n%s\nPlease correct according to %s :\n%s" % (fullpath, guidelines, match_kinda.group(0)) + oe.qa.handle_error(patchtype, msg, d) else: - bb.error("Missing Upstream-Status in patch\n%s\nPlease add according to %s ." % (fullpath, guidelines)) + msg = "Missing Upstream-Status in patch\n%s\nPlease add according to %s ." % (fullpath, guidelines) + oe.qa.handle_error(patchtype, msg, d) + + oe.qa.exit_if_errors(d) } python do_qa_configure() {