diff mbox series

perl-version: make PERL* assignments non-immediate

Message ID 20230405003850.384813-1-patrick@stwcx.xyz
State New
Headers show
Series perl-version: make PERL* assignments non-immediate | expand

Commit Message

Patrick Williams April 5, 2023, 12:38 a.m. UTC
The perl-version.bbclass executes functions which can depend on
variables potentially populated by native, such as `libdir`.  The
sanity `native-last` suggests that recipes should `inherit native`
last, but when that is done the variables like PERLVERSION end up
as `None`, since `${STAGING_LIBDIR}` needs `${libdir}` which is not
yet populated (by native).

Switch these variables to be non-immediate, which then allows the
`${libdir}` to be properly populated.

It appears that meta-openembedded/meta-perl does not use either
PERLVERSION or PERLARCH, nor is it used anywhere in `poky`.
meta-security/meta-perl uses PERLVERSION in one recipe's `do_install`
step.  OpenBMC's meta-phosphor similarly uses PERLVERSION in a
`do_install` step.  Therefore, it should be safe to make this
non-immediate.

Fixes: openbmc/openbmc#3770
Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
---
 meta/classes-recipe/perl-version.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Richard Purdie April 5, 2023, 8:24 a.m. UTC | #1
On Tue, 2023-04-04 at 19:38 -0500, Patrick Williams wrote:
> The perl-version.bbclass executes functions which can depend on
> variables potentially populated by native, such as `libdir`.  The
> sanity `native-last` suggests that recipes should `inherit native`
> last, but when that is done the variables like PERLVERSION end up
> as `None`, since `${STAGING_LIBDIR}` needs `${libdir}` which is not
> yet populated (by native).
> 
> Switch these variables to be non-immediate, which then allows the
> `${libdir}` to be properly populated.
> 
> It appears that meta-openembedded/meta-perl does not use either
> PERLVERSION or PERLARCH, nor is it used anywhere in `poky`.
> meta-security/meta-perl uses PERLVERSION in one recipe's `do_install`
> step.  OpenBMC's meta-phosphor similarly uses PERLVERSION in a
> `do_install` step.  Therefore, it should be safe to make this
> non-immediate.
> 
> Fixes: openbmc/openbmc#3770
> Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
> ---
>  meta/classes-recipe/perl-version.bbclass | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes-recipe/perl-version.bbclass b/meta/classes-recipe/perl-version.bbclass
> index 269ac9eb31..e4913dd502 100644
> --- a/meta/classes-recipe/perl-version.bbclass
> +++ b/meta/classes-recipe/perl-version.bbclass
> @@ -26,7 +26,7 @@ def get_perl_version(d):
>              return m.group(1)
>      return None
>  
> -PERLVERSION := "${@get_perl_version(d)}"
> +PERLVERSION = "${@get_perl_version(d)}"
>  PERLVERSION[vardepvalue] = ""
>  
>  
> @@ -49,7 +49,7 @@ def get_perl_arch(d):
>              return m.group(1)
>      return None
>  
> -PERLARCH := "${@get_perl_arch(d)}"
> +PERLARCH = "${@get_perl_arch(d)}"
>  PERLARCH[vardepvalue] = ""
>  
>  # Determine the staged arch of perl-native from the perl configuration file

Most code seems to use ${@get_perl_version(d)} and ${@get_perl_arch(d)}
directly so perhaps we should just remove the above instead and use
these in meta-security too?

Cheers,

Richard
Patrick Williams April 5, 2023, 12:07 p.m. UTC | #2
On Wed, Apr 05, 2023 at 09:24:24AM +0100, Richard Purdie wrote:
> On Tue, 2023-04-04 at 19:38 -0500, Patrick Williams wrote:
> >  
> > -PERLVERSION := "${@get_perl_version(d)}"
> > +PERLVERSION = "${@get_perl_version(d)}"
> >  PERLVERSION[vardepvalue] = ""
> >  
> >  
> > @@ -49,7 +49,7 @@ def get_perl_arch(d):
> >              return m.group(1)
> >      return None
> >  
> > -PERLARCH := "${@get_perl_arch(d)}"
> > +PERLARCH = "${@get_perl_arch(d)}"
> >  PERLARCH[vardepvalue] = ""
> >  
> 
> Most code seems to use ${@get_perl_version(d)} and ${@get_perl_arch(d)}
> directly so perhaps we should just remove the above instead and use
> these in meta-security too?

I submitted the change to meta-security[1] and changed the one in
openbmc/meta-phosphor[2] as well.

Do you want me to go ahead and delete these variables?  Is there any
documentation / change-log that I should update as well?

1. https://lore.kernel.org/yocto/20230405113726.469558-1-patrick@stwcx.xyz/T/#u
2. https://gerrit.openbmc.org/c/openbmc/openbmc/+/62164
Richard Purdie April 5, 2023, 10:21 p.m. UTC | #3
On Wed, 2023-04-05 at 07:07 -0500, Patrick Williams wrote:
> On Wed, Apr 05, 2023 at 09:24:24AM +0100, Richard Purdie wrote:
> > On Tue, 2023-04-04 at 19:38 -0500, Patrick Williams wrote:
> > >  
> > > -PERLVERSION := "${@get_perl_version(d)}"
> > > +PERLVERSION = "${@get_perl_version(d)}"
> > >  PERLVERSION[vardepvalue] = ""
> > >  
> > >  
> > > @@ -49,7 +49,7 @@ def get_perl_arch(d):
> > >              return m.group(1)
> > >      return None
> > >  
> > > -PERLARCH := "${@get_perl_arch(d)}"
> > > +PERLARCH = "${@get_perl_arch(d)}"
> > >  PERLARCH[vardepvalue] = ""
> > >  
> > 
> > Most code seems to use ${@get_perl_version(d)} and ${@get_perl_arch(d)}
> > directly so perhaps we should just remove the above instead and use
> > these in meta-security too?
> 
> I submitted the change to meta-security[1] and changed the one in
> openbmc/meta-phosphor[2] as well.
> 
> Do you want me to go ahead and delete these variables?  Is there any
> documentation / change-log that I should update as well?
> 
> 1. https://lore.kernel.org/yocto/20230405113726.469558-1-patrick@stwcx.xyz/T/#u
> 2. https://gerrit.openbmc.org/c/openbmc/openbmc/+/62164

Since nothing key seems to be using them any more we should just be
able to remove them. 

We should probably make a note in the migration guide for the next
release and check the variables aren't mentioned in the manuals but at
a quick look I don't think they are. The migration guide is still being
worked on for the current release so queuing the change might confuse
things more than it helps right now :/.

Cheers,

Richard
diff mbox series

Patch

diff --git a/meta/classes-recipe/perl-version.bbclass b/meta/classes-recipe/perl-version.bbclass
index 269ac9eb31..e4913dd502 100644
--- a/meta/classes-recipe/perl-version.bbclass
+++ b/meta/classes-recipe/perl-version.bbclass
@@ -26,7 +26,7 @@  def get_perl_version(d):
             return m.group(1)
     return None
 
-PERLVERSION := "${@get_perl_version(d)}"
+PERLVERSION = "${@get_perl_version(d)}"
 PERLVERSION[vardepvalue] = ""
 
 
@@ -49,7 +49,7 @@  def get_perl_arch(d):
             return m.group(1)
     return None
 
-PERLARCH := "${@get_perl_arch(d)}"
+PERLARCH = "${@get_perl_arch(d)}"
 PERLARCH[vardepvalue] = ""
 
 # Determine the staged arch of perl-native from the perl configuration file