diff mbox series

sanity: check variable GO_IMPORT

Message ID 20250225180939.21341-1-gavrosc@yahoo.com
State New
Headers show
Series sanity: check variable GO_IMPORT | expand

Commit Message

Christos Gavros Feb. 25, 2025, 6:09 p.m. UTC
Check if the variable GO_IMPORT is
assigned with a value. If not generate an error.
Fixes [YOCTO #15763]

CC: Yoann Congal <yoann.congal@smile.fr>
CC: Randy MacLeod <randy.macleod@windriver.com>
CC: Alexander Kanavin <alex.kanavin@gmail.com>
Signed-off-by: Christos Gavros <gavrosc@yahoo.com>
---
 meta/classes-global/sanity.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Randy MacLeod Feb. 25, 2025, 8:33 p.m. UTC | #1
On 2025-02-25 1:09 p.m., Christos Gavros wrote:
> Check if the variable GO_IMPORT is
> assigned with a value. If not generate an error.
> Fixes [YOCTO #15763]
>
> CC: Yoann Congal<yoann.congal@smile.fr>
> CC: Randy MacLeod<randy.macleod@windriver.com>
> CC: Alexander Kanavin<alex.kanavin@gmail.com>
> Signed-off-by: Christos Gavros<gavrosc@yahoo.com>
> ---
>   meta/classes-global/sanity.bbclass | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/meta/classes-global/sanity.bbclass b/meta/classes-global/sanity.bbclass
> index 1bae998f74..01e48b4347 100644
> --- a/meta/classes-global/sanity.bbclass
> +++ b/meta/classes-global/sanity.bbclass
> @@ -1010,6 +1010,10 @@ def check_sanity_everybuild(status, d):
>           if '/dash' not in real_sh and '/bash' not in real_sh:
>               status.addresult("Error, /bin/sh links to %s, must be dash or bash\n" % real_sh)
>   
> +    # Check if a value is assigned to GO_IMPORT variable
> +    if not d.getVar('GO_IMPORT'):
> +        status.addresult("GO_IMPORT variable is not assigned with a value")
> +
>   def check_sanity(sanity_data):
>       class SanityStatus(object):
>           def __init__(self):

Thanks Christos,

I know I suggested that you try sending this to see what people say and 
to get some guidance but
this break all the builds so clearly we can't merge it. I did expect you 
to do a build though... ;-)

❯ bitbake busybox
...
ERROR:  OE-core's config sanity checker detected a potential 
misconfiguration.
     Either fix the cause of this error or at your own risk disable the 
checker (see sanity.conf).
     Following is the list of potential problems / advisories:

     GO_IMPORT variable is not assigned with a value

We need a sanity check at the recipe level for go-based recipes only.
I'm not sure how to do that.

I may do a bit of digging and reply to my own email with a suggestion of 
a better approach
unless someone beats me to it!
Christos Gavros Feb. 25, 2025, 8:37 p.m. UTC | #2
hi Randy

I did a build and I wrote a comment in the bug15763 that is generating an error!
I always built and test as much as I can before sending patches.
Richard Purdie Feb. 25, 2025, 10:43 p.m. UTC | #3
On Tue, 2025-02-25 at 15:33 -0500, Randy MacLeod via lists.openembedded.org wrote:
>  On 2025-02-25 1:09 p.m., Christos Gavros wrote:
>  Check if the variable GO_IMPORT is
> > assigned with a value. If not generate an error.
> > Fixes [YOCTO #15763]
> > 
> > CC: Yoann Congal <yoann.congal@smile.fr>
> > CC: Randy MacLeod <randy.macleod@windriver.com>
> > CC: Alexander Kanavin <alex.kanavin@gmail.com>
> > Signed-off-by: Christos Gavros <gavrosc@yahoo.com>
> > ---
> >  meta/classes-global/sanity.bbclass | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/meta/classes-global/sanity.bbclass b/meta/classes-global/sanity.bbclass
> > index 1bae998f74..01e48b4347 100644
> > --- a/meta/classes-global/sanity.bbclass
> > +++ b/meta/classes-global/sanity.bbclass
> > @@ -1010,6 +1010,10 @@ def check_sanity_everybuild(status, d):
> >          if '/dash' not in real_sh and '/bash' not in real_sh:
> >              status.addresult("Error, /bin/sh links to %s, must be dash or bash\n" % real_sh)
> >  
> > +    # Check if a value is assigned to GO_IMPORT variable
> > +    if not d.getVar('GO_IMPORT'):
> > +        status.addresult("GO_IMPORT variable is not assigned with a value")
> > +
> >  def check_sanity(sanity_data):
> >      class SanityStatus(object):
> >          def __init__(self):
> >  
>  
> Thanks Christos,
>  
> I know I suggested that you try sending this to see what people say and to get some guidance but
>  this break all the builds so clearly we can't merge it. I did expect you to do a build though... ;-)
>   
> ❯ bitbake busybox
>  ...
>  ERROR:  OE-core's config sanity checker detected a potential misconfiguration.
>      Either fix the cause of this error or at your own risk disable the checker (see sanity.conf).
>      Following is the list of potential problems / advisories:
>  
>      GO_IMPORT variable is not assigned with a value
>  
> We need a sanity check at the recipe level for go-based recipes only.
>  I'm not sure how to do that.
>  
> I may do a bit of digging and reply to my own email with a suggestion of a better approach unless someone beats me to it!

The code is close, the location is wrong as "sanity test" is misleading
you.

The place this is used is in go.bbclass so I'd suggest we need a check
in that class alongside where it is used. It could be as simple as the
code above but in the form:

python () {
    # Check if a value is assigned to GO_IMPORT variable
    if not d.getVar('GO_IMPORT'):
        bb.fatal("GO_IMPORT variable needs to be set to use the go bbclass but has no value")
}

Cheers,

Richard
Christos Gavros Feb. 26, 2025, 10 a.m. UTC | #4
hi

I will make the changes, test it and submit v2 next days!
Christos Gavros Feb. 27, 2025, 1:15 p.m. UTC | #5
hi Richard

I have added the code as you suggested in go.bbclass and then i created meta-mylayer and try to build there " go-helloworld_0.1.bb". It builds fine.

When i commented GO_IMPORT in the recipe the error i am getting is:

ERROR: ExpansionError during parsing /home/cg/CG/Programming/Yocto/poky/meta-mylayer/recipes-go/go-helloworld/go-helloworld.bb             | ETA:  --:--:--
bb.data_smart.ExpansionError: Failure expanding variable GO_SRCURI_DESTSUFFIX, expression was ${@os.path.join(os.path.basename(d.getVar('S')), 'src', d.getVar('GO_IMPORT')) + '/'} which triggered exception TypeError: join() argument must be str, bytes, or os.PathLike object, not 'NoneType'
The variable dependency chain for the failure is: GO_SRCURI_DESTSUFFIX -> SRC_URI

This is caused from this line in the recipe: SRC_URI = " git://go.googlesource.com/example;branch=master;protocol=https;destsuffix=${GO_SRCURI_DESTSUFFIX} ( git://go.googlesource.com/example;branch=master;protocol=https;destsuffix=$%7BGO_SRCURI_DESTSUFFIX%7D ) "

So it looks that expanding GO_SRCURI_DESTSUFFIX variable is executed before anonymous python function and the error "GO_IMPORT is not set" will not be generated.

If you don't have anything to comment , I will try to dive deeper and see how I can adapt it...

Br
Christos
Richard Purdie Feb. 27, 2025, 1:20 p.m. UTC | #6
On Thu, 2025-02-27 at 05:15 -0800, Christos Gavros via lists.openembedded.org wrote:
> hi Richard
>  
> I have added the code as you suggested in go.bbclass and then i created meta-mylayer and try to build there "go-helloworld_0.1.bb". It builds fine.
> 
>  
> When i commented GO_IMPORT in the recipe the error i am getting is:
>  
> 
> 
> 
> 
> ERROR: ExpansionError during parsing /home/cg/CG/Programming/Yocto/poky/meta-mylayer/recipes-go/go-helloworld/go-helloworld.bb             | ETA:  --:--:--
> bb.data_smart.ExpansionError: Failure expanding variable GO_SRCURI_DESTSUFFIX, expression was ${@os.path.join(os.path.basename(d.getVar('S')), 'src', d.getVar('GO_IMPORT')) + '/'} which triggered exception TypeError: join() argument must be str, bytes, or os.PathLike object, not 'NoneType'
> The variable dependency chain for the failure is: GO_SRCURI_DESTSUFFIX -> SRC_URI
>  
> 
> 
> This is caused from this line in the recipe: SRC_URI = "git://go.googlesource.com/example;branch=master;protocol=https;destsuffix=${GO_SRCURI_DESTSUFFIX}"
>  
> So it looks that expanding GO_SRCURI_DESTSUFFIX variable is executed before anonymous python function and the error "GO_IMPORT is not set" will not be generated.
>  
> If you don't have anything to comment , I will try to dive deeper and see how I can adapt it...
> 

That is unfortunate. As you say, it is expanding it earlier than the
anonymous python.

One slightly evil idea that comes to mind might be:

GO_IMPORT ??= "${@bb.fatal("The recipe needs to set GO_IMPORT for go.bbclass to work")}"

Not sure if that would work or not! It may still give a traceback
although I'm hoping it doesn't.


Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/classes-global/sanity.bbclass b/meta/classes-global/sanity.bbclass
index 1bae998f74..01e48b4347 100644
--- a/meta/classes-global/sanity.bbclass
+++ b/meta/classes-global/sanity.bbclass
@@ -1010,6 +1010,10 @@  def check_sanity_everybuild(status, d):
         if '/dash' not in real_sh and '/bash' not in real_sh:
             status.addresult("Error, /bin/sh links to %s, must be dash or bash\n" % real_sh)
 
+    # Check if a value is assigned to GO_IMPORT variable
+    if not d.getVar('GO_IMPORT'):
+        status.addresult("GO_IMPORT variable is not assigned with a value")
+
 def check_sanity(sanity_data):
     class SanityStatus(object):
         def __init__(self):