[04/21] insane.bbclass: add a check that Upstream-Status patch tag is present and correctly formed

Signed-off-by: Alexander Kanavin <alex@linutronix.de>
---
 meta/classes/insane.bbclass | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

On 24.11.21 09:08, Alexander Kanavin wrote:
> Signed-off-by: Alexander Kanavin <alex@linutronix.de>
> ---
>   meta/classes/insane.bbclass | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/meta/classes/insane.bbclass b/meta/classes/insane.bbclass
> index 27b1a00fb9..240f3aad62 100644
> --- a/meta/classes/insane.bbclass
> +++ b/meta/classes/insane.bbclass
> @@ -1167,6 +1167,30 @@ python do_qa_patch() {
>                   bb.warn(msg)
>               msg = "Patch 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
> +
> +    for url in patch.src_patches(d):
> +       (_, _, fullpath, _, _, _) = bb.fetch.decodeurl(url)
> +
> +       # skip patches not in oe-core
> +       if '/meta/' not in fullpath:
> +           continue

Can we have that configurable please?
This is somehow a hardcoded assumption I don't really like.
Maybe it's worth a shot to match it against BBFILE_PATTERN_core...

> +
> +       content = open(fullpath, encoding='utf-8', errors='ignore').read()
> +       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)( .+)?$", re.MULTILINE)
> +       match_kinda = kinda_status_re.search(content)
> +       match_strict = strict_status_re.search(content)
> +       guidelines = "https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines#Patch_Header_Recommendations:_Upstream-Status"
> +
> +       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)))
> +           else:
> +               bb.error("Missing Upstream-Status in patch\n%s\nPlease add according to %s ." % (fullpath, guidelines))

Why is there such a fuzzy (kinda) pattern? - IMO the strict pattern 
should be enough, stating Malformed or missing - both cases are equally 
wrong, aren't they?

>   }
>   
>   python do_qa_configure() {
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#158660): https://lists.openembedded.org/g/openembedded-core/message/158660
> Mute This Topic: https://lists.openembedded.org/mt/87277236/3647476
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [kweihmann@outlook.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>

On Wed, 24 Nov 2021 at 09:19, Konrad Weihmann <kweihmann@outlook.com> wrote:

> > +       # skip patches not in oe-core
> > +       if '/meta/' not in fullpath:
> > +           continue
>
> Can we have that configurable please?
> This is somehow a hardcoded assumption I don't really like.
> Maybe it's worth a shot to match it against BBFILE_PATTERN_core...
>

Sure, I can adjust this.


> > +       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)))
> > +           else:
> > +               bb.error("Missing Upstream-Status in patch\n%s\nPlease
> add according to %s ." % (fullpath, guidelines))
>
> Why is there such a fuzzy (kinda) pattern? - IMO the strict pattern
> should be enough, stating Malformed or missing - both cases are equally
> wrong, aren't they?
>

It's to give a more precise diagnostic. If the line is present but
malformed, it helps to say that and print it; if it's missing, there is
nothing to print.

Alex

On 24.11.21 09:30, Alexander Kanavin wrote:
> On Wed, 24 Nov 2021 at 09:19, Konrad Weihmann <kweihmann@outlook.com 
> <mailto:kweihmann@outlook.com>> wrote:
> 
>      > +       # skip patches not in oe-core
>      > +       if '/meta/' not in fullpath:
>      > +           continue
> 
>     Can we have that configurable please?
>     This is somehow a hardcoded assumption I don't really like.
>     Maybe it's worth a shot to match it against BBFILE_PATTERN_core...
> 
> 
> Sure, I can adjust this.

Just a thought - it's maybe worth disabling the skip of the core layer 
on AB while patchwork isn't really in a good state to catch such stuff

> 
>      > +       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)))
>      > +           else:
>      > +               bb.error("Missing Upstream-Status in
>     patch\n%s\nPlease add according to %s ." % (fullpath, guidelines))
> 
>     Why is there such a fuzzy (kinda) pattern? - IMO the strict pattern
>     should be enough, stating Malformed or missing - both cases are equally
>     wrong, aren't they?
> 
> 
> It's to give a more precise diagnostic. If the line is present but 
> malformed, it helps to say that and print it; if it's missing, there is 
> nothing to print.

Makes sense

> 
> Alex
>