diff mbox series

[V2] insane.bbclass: avoid unnecessary rerun of do_patch

Message ID 20251217094747.3217558-1-Qi.Chen@windriver.com
State Accepted, archived
Commit 693b862db479ecb4c15b6e755ccb4ed30cf91fa8
Headers show
Series [V2] insane.bbclass: avoid unnecessary rerun of do_patch | expand

Commit Message

ChenQi Dec. 17, 2025, 9:47 a.m. UTC 
From: Chen Qi <Qi.Chen@windriver.com>

When toggling ptest for DISTRO_FEATURES, the do_patch function
gets rerun.

The dependency chain is:
do_patch -> do_qa_patch -> DISTRO_FEATURES{ptest}

Such rerun is not necessary. And it's kind of annoying because everything
gets rebuilt, including cross toolchain and recipes not using ptest.

The ERROR_QA and WARN_QA should be enough to trigger the re-run
if unimplemented-ptest is added to one of them.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes-global/insane.bbclass | 1 +
 1 file changed, 1 insertion(+)

Comments

Alexander Kanavin Dec. 17, 2025, 11:02 a.m. UTC | #1 
On Wed, 17 Dec 2025 at 10:48, Chen Qi via lists.openembedded.org
<Qi.Chen=windriver.com@lists.openembedded.org> wrote:
> When toggling ptest for DISTRO_FEATURES, the do_patch function
> gets rerun.
>
> The dependency chain is:
> do_patch -> do_qa_patch -> DISTRO_FEATURES{ptest}
>
> Such rerun is not necessary. And it's kind of annoying because everything
> gets rebuilt, including cross toolchain and recipes not using ptest.
>
> The ERROR_QA and WARN_QA should be enough to trigger the re-run
> if unimplemented-ptest is added to one of them.
...
> +do_qa_patch[vardepsexclude] = "DISTRO_FEATURES"

The offending code is this:

    if not bb.utils.contains('DISTRO_FEATURES', 'ptest', True, False, d):
        pass
    elif
(gigantic list of elif checks follows)

I wonder if we should rather drop this first check for DISTRO_FEATURES
instead. These are recipe-level checks, and the issues reported are
valid regardless of whether ptest is in someone's DISTRO_FEATURES or
not.

Using vardepsexclude is problematic because do_qa_patch may have other
uses of DISTRO_FEATURES in the future, and the same reasoning may not
apply to them.

Alex
ChenQi Dec. 17, 2025, 1:01 p.m. UTC | #2 
I agree. I'll send out V3.

Regards,
Qi

-----Original Message-----
From: Alexander Kanavin <alex.kanavin@gmail.com> 
Sent: Wednesday, December 17, 2025 7:02 PM
To: Chen, Qi <Qi.Chen@windriver.com>
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core][PATCH V2] insane.bbclass: avoid unnecessary rerun of do_patch

On Wed, 17 Dec 2025 at 10:48, Chen Qi via lists.openembedded.org <Qi.Chen=windriver.com@lists.openembedded.org> wrote:
> When toggling ptest for DISTRO_FEATURES, the do_patch function gets 
> rerun.
>
> The dependency chain is:
> do_patch -> do_qa_patch -> DISTRO_FEATURES{ptest}
>
> Such rerun is not necessary. And it's kind of annoying because 
> everything gets rebuilt, including cross toolchain and recipes not using ptest.
>
> The ERROR_QA and WARN_QA should be enough to trigger the re-run if 
> unimplemented-ptest is added to one of them.
...
> +do_qa_patch[vardepsexclude] = "DISTRO_FEATURES"

The offending code is this:

    if not bb.utils.contains('DISTRO_FEATURES', 'ptest', True, False, d):
        pass
    elif
(gigantic list of elif checks follows)

I wonder if we should rather drop this first check for DISTRO_FEATURES instead. These are recipe-level checks, and the issues reported are valid regardless of whether ptest is in someone's DISTRO_FEATURES or not.

Using vardepsexclude is problematic because do_qa_patch may have other uses of DISTRO_FEATURES in the future, and the same reasoning may not apply to them.

Alex
Yoann Congal Dec. 18, 2025, 11:09 a.m. UTC | #3 
Le mer. 17 déc. 2025 à 12:02, Alexander Kanavin via
lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
a écrit :
>
> On Wed, 17 Dec 2025 at 10:48, Chen Qi via lists.openembedded.org
> <Qi.Chen=windriver.com@lists.openembedded.org> wrote:
> > When toggling ptest for DISTRO_FEATURES, the do_patch function
> > gets rerun.
> >
> > The dependency chain is:
> > do_patch -> do_qa_patch -> DISTRO_FEATURES{ptest}
> >
> > Such rerun is not necessary. And it's kind of annoying because everything
> > gets rebuilt, including cross toolchain and recipes not using ptest.
> >
> > The ERROR_QA and WARN_QA should be enough to trigger the re-run
> > if unimplemented-ptest is added to one of them.
> ...
> > +do_qa_patch[vardepsexclude] = "DISTRO_FEATURES"
>
> The offending code is this:
>
>     if not bb.utils.contains('DISTRO_FEATURES', 'ptest', True, False, d):
>         pass
>     elif
> (gigantic list of elif checks follows)
>
> I wonder if we should rather drop this first check for DISTRO_FEATURES
> instead. These are recipe-level checks, and the issues reported are
> valid regardless of whether ptest is in someone's DISTRO_FEATURES or
> not.

The idea behind this check was to not make a user who has disabled
ptests pay the performance cost of reading/exploring a bunch of
file/directories.
But the test on ERROR/WARN QA should largely be enough for that goal.
I agree with you on dropping the first check for DISTRO_FEATURES.

> Using vardepsexclude is problematic because do_qa_patch may have other
> uses of DISTRO_FEATURES in the future, and the same reasoning may not
> apply to them.
>
> Alex
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#228024): https://lists.openembedded.org/g/openembedded-core/message/228024
> Mute This Topic: https://lists.openembedded.org/mt/116824171/4316185
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [yoann.congal@smile.fr]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/meta/classes-global/insane.bbclass b/meta/classes-global/insane.bbclass
index fed8163c3e..3185209b7f 100644
--- a/meta/classes-global/insane.bbclass
+++ b/meta/classes-global/insane.bbclass
@@ -1345,6 +1345,7 @@  python do_qa_patch() {
 
     oe.qa.exit_if_errors(d)
 }
+do_qa_patch[vardepsexclude] = "DISTRO_FEATURES"
 
 python do_qa_configure() {
     import subprocess