update-alternatives.bbclass - is this a leftover from old override syntax?

Hi,

I stumbled on some code in update-alternatives.bbclass that I suspect is 
missed when we changed to new override syntax but I'm a bit unsure and 
would like some more eyes and comments on this. Doing debugging with 
logging I see that the class is fetching variables like 
'ALTERNATIVE_${PN}' in the code at:

https://git.yoctoproject.org/poky/tree/meta/classes-recipe/update-alternatives.bbclass#n89

I think a patch to correct things could be:

+                d.appendVar('%s_VARDEPS_%s' % (v,p), ' %s:%s' % (flag, 
d.getVarFlag('%s:%s' % (v,p), flag, False)))

  def ua_extend_depends(d):
      if not 'virtual/update-alternatives' in d.getVar('PROVIDES'):


This is just found by review and I can not find any failure or fault 
that is caused by this. That's why I'm trying to get some more eyes on 
this and see if someone can fully understand how this is connected. With 
the patch variable flags for ALTERNATIVES:${PN} is fetched instead which 
should be more up to date with what we have in recipe meta data.

Is this something to send a patch for to fix old override syntax?

If yes on the first question; as we haven't found any issues with the 
old code is it still needed?

Best regards,
/Peter

On Tue, 2023-05-02 at 11:14 +0200, Peter Bergin wrote:
> Hi,
> 
> I stumbled on some code in update-alternatives.bbclass that I suspect is 
> missed when we changed to new override syntax but I'm a bit unsure and 
> would like some more eyes and comments on this. Doing debugging with 
> logging I see that the class is fetching variables like 
> 'ALTERNATIVE_${PN}' in the code at:
> 
> https://git.yoctoproject.org/poky/tree/meta/classes-recipe/update-alternatives.bbclass#n89
> 
> I think a patch to correct things could be:
> 
> diff --git a/meta/classes-recipe/update-alternatives.bbclass 
> b/meta/classes-recipe/update-alternatives.bbclass
> index 36a7497fec..b153e1b297 100644
> --- a/meta/classes-recipe/update-alternatives.bbclass
> +++ b/meta/classes-recipe/update-alternatives.bbclass
> @@ -86,10 +86,10 @@ def gen_updatealternativesvardeps(d):
> 
>       for p in pkgs:
>           for v in vars:
> -            for flag in sorted((d.getVarFlags("%s_%s" % (v,p)) or 
> {}).keys()):
> +            for flag in sorted((d.getVarFlags("%s:%s" % (v,p)) or 
> {}).keys()):
>                   if flag == "doc" or flag == "vardeps" or flag == 
> "vardepsexp":
>                       continue
> -                d.appendVar('%s_VARDEPS_%s' % (v,p), ' %s:%s' % (flag, 
> d.getVarFlag('%s_%s' % (v,p), flag, False)))
> +                d.appendVar('%s_VARDEPS_%s' % (v,p), ' %s:%s' % (flag, 
> d.getVarFlag('%s:%s' % (v,p), flag, False)))
> 
>   def ua_extend_depends(d):
>       if not 'virtual/update-alternatives' in d.getVar('PROVIDES'):
> 
> 
> This is just found by review and I can not find any failure or fault 
> that is caused by this. That's why I'm trying to get some more eyes on 
> this and see if someone can fully understand how this is connected. With 
> the patch variable flags for ALTERNATIVES:${PN} is fetched instead which 
> should be more up to date with what we have in recipe meta data.
> 
> Is this something to send a patch for to fix old override syntax?
> 
> If yes on the first question; as we haven't found any issues with the 
> old code is it still needed?

It does look like a bug to me. The code is setting up variable
dependencies so it means the task hashes might not be accurately
capturing all information, meaning things might not rebuild when they
should. It will be a bug, just not one anyone has spotted :/.

We should fix it, can you send a patch please?

Cheers,

Richard

On 2023-05-02 11:38, Richard Purdie wrote:
> We should fix it, can you send a patch please?

Yes - will do!

/Peter