Patchwork [1/1] core-image-lsb: enforce pam as a needed distro feature

login
register
mail settings
Submitter Cristian Iorga
Date March 31, 2014, 9:51 a.m.
Message ID <9e2ace2e2747ecda37004ea44d2e70de0586e185.1396259433.git.cristian.iorga@intel.com>
Download mbox | patch
Permalink /patch/69679/
State Accepted
Commit f4dc1e568636f7bfe216f4fb1386bbed4ee305af
Headers show

Comments

Cristian Iorga - March 31, 2014, 9:51 a.m.
core-image-lsb only gave a warning:
"WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES,
PAM won't work correctly"
when the proper DISTRO was not set for it.
default choice would be DISTRO = "poky-lsb",
but not necessarily, depending on each custom distro.

This fix will enforce the proper usage of pam
as a distro feature for core-image-lsb by giving
an error instead of just a warning.

Fixes [YOCTO #6073]

Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
---
 meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
Richard Purdie - March 31, 2014, 9:58 a.m.
On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> core-image-lsb only gave a warning:
> "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES,
> PAM won't work correctly"
> when the proper DISTRO was not set for it.
> default choice would be DISTRO = "poky-lsb",
> but not necessarily, depending on each custom distro.
> 
> This fix will enforce the proper usage of pam
> as a distro feature for core-image-lsb by giving
> an error instead of just a warning.
> 
> Fixes [YOCTO #6073]
> 
> Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> ---
>  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-extended/images/core-image-lsb.bb b/meta/recipes-extended/images/core-image-lsb.bb
> index ed316a6..ab61c6e 100644
> --- a/meta/recipes-extended/images/core-image-lsb.bb
> +++ b/meta/recipes-extended/images/core-image-lsb.bb
> @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
>      packagegroup-core-lsb \
>      "
>  
> -inherit core-image
> +inherit core-image distro_features_check
> +
> +REQUIRED_DISTRO_FEATURES = "pam"


I have a feeling the autobuilder builds core-image-lsb in situations
where DISTRO=poky, although I could be wrong. Have you checked?

Cheers,

Richard
Cristian Iorga - March 31, 2014, 10:09 a.m.
No, but I will.

But is this a negative point of the patch, or the autobuilder should be changed?
I acted upon some personal observations, a discussion with Paul (he suggested the change) and also on this guide:
https://wiki.yoctoproject.org/wiki/LSB_Result (in which distro is set to poky-lsb).

Regards,
Cristian

-----Original Message-----
From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org] 
Sent: Monday, March 31, 2014 12:58 PM
To: Iorga, Cristian
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH 1/1] core-image-lsb: enforce pam as a needed distro feature

On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> core-image-lsb only gave a warning:
> "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES, PAM 
> won't work correctly"
> when the proper DISTRO was not set for it.
> default choice would be DISTRO = "poky-lsb", but not necessarily, 
> depending on each custom distro.
> 
> This fix will enforce the proper usage of pam as a distro feature for 
> core-image-lsb by giving an error instead of just a warning.
> 
> Fixes [YOCTO #6073]
> 
> Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> ---
>  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-extended/images/core-image-lsb.bb 
> b/meta/recipes-extended/images/core-image-lsb.bb
> index ed316a6..ab61c6e 100644
> --- a/meta/recipes-extended/images/core-image-lsb.bb
> +++ b/meta/recipes-extended/images/core-image-lsb.bb
> @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
>      packagegroup-core-lsb \
>      "
>  
> -inherit core-image
> +inherit core-image distro_features_check
> +
> +REQUIRED_DISTRO_FEATURES = "pam"


I have a feeling the autobuilder builds core-image-lsb in situations where DISTRO=poky, although I could be wrong. Have you checked?

Cheers,

Richard
Cristian Iorga - March 31, 2014, 10:40 a.m.
Hello,
I have checked, on the autobuilder the right association between LSB-enabled images and proper distro (in our case, poky-lsb) is in place.
As such, my patch should not cause any trouble.
Regards,
Cristian

-----Original Message-----
From: openembedded-core-bounces@lists.openembedded.org [mailto:openembedded-core-bounces@lists.openembedded.org] On Behalf Of Iorga, Cristian
Sent: Monday, March 31, 2014 1:10 PM
To: Richard Purdie
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH 1/1] core-image-lsb: enforce pam as a needed distro feature

No, but I will.

But is this a negative point of the patch, or the autobuilder should be changed?
I acted upon some personal observations, a discussion with Paul (he suggested the change) and also on this guide:
https://wiki.yoctoproject.org/wiki/LSB_Result (in which distro is set to poky-lsb).

Regards,
Cristian

-----Original Message-----
From: Richard Purdie [mailto:richard.purdie@linuxfoundation.org]
Sent: Monday, March 31, 2014 12:58 PM
To: Iorga, Cristian
Cc: openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH 1/1] core-image-lsb: enforce pam as a needed distro feature

On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> core-image-lsb only gave a warning:
> "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES, PAM 
> won't work correctly"
> when the proper DISTRO was not set for it.
> default choice would be DISTRO = "poky-lsb", but not necessarily, 
> depending on each custom distro.
> 
> This fix will enforce the proper usage of pam as a distro feature for 
> core-image-lsb by giving an error instead of just a warning.
> 
> Fixes [YOCTO #6073]
> 
> Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> ---
>  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/recipes-extended/images/core-image-lsb.bb
> b/meta/recipes-extended/images/core-image-lsb.bb
> index ed316a6..ab61c6e 100644
> --- a/meta/recipes-extended/images/core-image-lsb.bb
> +++ b/meta/recipes-extended/images/core-image-lsb.bb
> @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
>      packagegroup-core-lsb \
>      "
>  
> -inherit core-image
> +inherit core-image distro_features_check
> +
> +REQUIRED_DISTRO_FEATURES = "pam"


I have a feeling the autobuilder builds core-image-lsb in situations where DISTRO=poky, although I could be wrong. Have you checked?

Cheers,

Richard

--
Stanacar, StefanX - March 31, 2014, 11:58 a.m.
On Mon, 2014-03-31 at 10:58 +0100, Richard Purdie wrote:
> On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> > core-image-lsb only gave a warning:
> > "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES,
> > PAM won't work correctly"
> > when the proper DISTRO was not set for it.
> > default choice would be DISTRO = "poky-lsb",
> > but not necessarily, depending on each custom distro.
> > 
> > This fix will enforce the proper usage of pam
> > as a distro feature for core-image-lsb by giving
> > an error instead of just a warning.
> > 
> > Fixes [YOCTO #6073]
> > 
> > Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> > ---
> >  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/meta/recipes-extended/images/core-image-lsb.bb b/meta/recipes-extended/images/core-image-lsb.bb
> > index ed316a6..ab61c6e 100644
> > --- a/meta/recipes-extended/images/core-image-lsb.bb
> > +++ b/meta/recipes-extended/images/core-image-lsb.bb
> > @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
> >      packagegroup-core-lsb \
> >      "
> >  
> > -inherit core-image
> > +inherit core-image distro_features_check
> > +
> > +REQUIRED_DISTRO_FEATURES = "pam"
> 
> 
> I have a feeling the autobuilder builds core-image-lsb in situations
> where DISTRO=poky, although I could be wrong. Have you checked?
> 

FWIW, all the -lsb buildsets are done with DISTRO=poky-lsb on the AB.
Unfortuntely we do have one problematic build.
So the answer to your question is: we don't have core-image-lsb builds
with DISTRO=poky but we do have a lib64-core-image-lsb-sdk image built
with DISTRO=poky and no pam in DISTRO_FEATURES, see the last build on
nightly-multilib... :(

Cheers,
Stefan

> Cheers,
> 
> Richard
>
Paul Eggleton - March 31, 2014, 12:18 p.m.
On Monday 31 March 2014 11:58:49 Stanacar, StefanX wrote:
> On Mon, 2014-03-31 at 10:58 +0100, Richard Purdie wrote:
> > On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> > > core-image-lsb only gave a warning:
> > > "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES,
> > > PAM won't work correctly"
> > > when the proper DISTRO was not set for it.
> > > default choice would be DISTRO = "poky-lsb",
> > > but not necessarily, depending on each custom distro.
> > > 
> > > This fix will enforce the proper usage of pam
> > > as a distro feature for core-image-lsb by giving
> > > an error instead of just a warning.
> > > 
> > > Fixes [YOCTO #6073]
> > > 
> > > Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> > > ---
> > > 
> > >  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/recipes-extended/images/core-image-lsb.bb
> > > b/meta/recipes-extended/images/core-image-lsb.bb index ed316a6..ab61c6e
> > > 100644
> > > --- a/meta/recipes-extended/images/core-image-lsb.bb
> > > +++ b/meta/recipes-extended/images/core-image-lsb.bb
> > > @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
> > > 
> > >      packagegroup-core-lsb \
> > >      "
> > > 
> > > -inherit core-image
> > > +inherit core-image distro_features_check
> > > +
> > > +REQUIRED_DISTRO_FEATURES = "pam"
> > 
> > I have a feeling the autobuilder builds core-image-lsb in situations
> > where DISTRO=poky, although I could be wrong. Have you checked?
> 
> FWIW, all the -lsb buildsets are done with DISTRO=poky-lsb on the AB.
> Unfortuntely we do have one problematic build.
> So the answer to your question is: we don't have core-image-lsb builds
> with DISTRO=poky but we do have a lib64-core-image-lsb-sdk image built
> with DISTRO=poky and no pam in DISTRO_FEATURES, see the last build on
> nightly-multilib... :(

If we're doing this then we should be changing the autobuilder so it doesn't. 
LSB images should not be built in non-LSB configuration.

Cheers,
Paul
Richard Purdie - March 31, 2014, 5:03 p.m.
On Mon, 2014-03-31 at 13:18 +0100, Paul Eggleton wrote:
> On Monday 31 March 2014 11:58:49 Stanacar, StefanX wrote:
> > On Mon, 2014-03-31 at 10:58 +0100, Richard Purdie wrote:
> > > On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> > > > core-image-lsb only gave a warning:
> > > > "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES,
> > > > PAM won't work correctly"
> > > > when the proper DISTRO was not set for it.
> > > > default choice would be DISTRO = "poky-lsb",
> > > > but not necessarily, depending on each custom distro.
> > > > 
> > > > This fix will enforce the proper usage of pam
> > > > as a distro feature for core-image-lsb by giving
> > > > an error instead of just a warning.
> > > > 
> > > > Fixes [YOCTO #6073]
> > > > 
> > > > Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> > > > ---
> > > > 
> > > >  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
> > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/meta/recipes-extended/images/core-image-lsb.bb
> > > > b/meta/recipes-extended/images/core-image-lsb.bb index ed316a6..ab61c6e
> > > > 100644
> > > > --- a/meta/recipes-extended/images/core-image-lsb.bb
> > > > +++ b/meta/recipes-extended/images/core-image-lsb.bb
> > > > @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
> > > > 
> > > >      packagegroup-core-lsb \
> > > >      "
> > > > 
> > > > -inherit core-image
> > > > +inherit core-image distro_features_check
> > > > +
> > > > +REQUIRED_DISTRO_FEATURES = "pam"
> > > 
> > > I have a feeling the autobuilder builds core-image-lsb in situations
> > > where DISTRO=poky, although I could be wrong. Have you checked?
> > 
> > FWIW, all the -lsb buildsets are done with DISTRO=poky-lsb on the AB.
> > Unfortuntely we do have one problematic build.
> > So the answer to your question is: we don't have core-image-lsb builds
> > with DISTRO=poky but we do have a lib64-core-image-lsb-sdk image built
> > with DISTRO=poky and no pam in DISTRO_FEATURES, see the last build on
> > nightly-multilib... :(
> 
> If we're doing this then we should be changing the autobuilder so it doesn't. 
> LSB images should not be built in non-LSB configuration.

I kind of disagree with that, the LSB image can take into account
configuration in other parts of the system. If pam isn't configured, I'm
not sure that should automatically make it completely unbuildable...

Cheers,

Richard
Paul Eggleton - March 31, 2014, 5:23 p.m.
On Monday 31 March 2014 18:03:32 Richard Purdie wrote:
> On Mon, 2014-03-31 at 13:18 +0100, Paul Eggleton wrote:
> > On Monday 31 March 2014 11:58:49 Stanacar, StefanX wrote:
> > > On Mon, 2014-03-31 at 10:58 +0100, Richard Purdie wrote:
> > > > On Mon, 2014-03-31 at 12:51 +0300, Cristian Iorga wrote:
> > > > > core-image-lsb only gave a warning:
> > > > > "WARNING: Building libpam but 'pam' isn't in DISTRO_FEATURES,
> > > > > PAM won't work correctly"
> > > > > when the proper DISTRO was not set for it.
> > > > > default choice would be DISTRO = "poky-lsb",
> > > > > but not necessarily, depending on each custom distro.
> > > > > 
> > > > > This fix will enforce the proper usage of pam
> > > > > as a distro feature for core-image-lsb by giving
> > > > > an error instead of just a warning.
> > > > > 
> > > > > Fixes [YOCTO #6073]
> > > > > 
> > > > > Signed-off-by: Cristian Iorga <cristian.iorga@intel.com>
> > > > > ---
> > > > > 
> > > > >  meta/recipes-extended/images/core-image-lsb.bb | 4 +++-
> > > > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/meta/recipes-extended/images/core-image-lsb.bb
> > > > > b/meta/recipes-extended/images/core-image-lsb.bb index
> > > > > ed316a6..ab61c6e
> > > > > 100644
> > > > > --- a/meta/recipes-extended/images/core-image-lsb.bb
> > > > > +++ b/meta/recipes-extended/images/core-image-lsb.bb
> > > > > @@ -9,4 +9,6 @@ IMAGE_INSTALL = "\
> > > > > 
> > > > >      packagegroup-core-lsb \
> > > > >      "
> > > > > 
> > > > > -inherit core-image
> > > > > +inherit core-image distro_features_check
> > > > > +
> > > > > +REQUIRED_DISTRO_FEATURES = "pam"
> > > > 
> > > > I have a feeling the autobuilder builds core-image-lsb in situations
> > > > where DISTRO=poky, although I could be wrong. Have you checked?
> > > 
> > > FWIW, all the -lsb buildsets are done with DISTRO=poky-lsb on the AB.
> > > Unfortuntely we do have one problematic build.
> > > So the answer to your question is: we don't have core-image-lsb builds
> > > with DISTRO=poky but we do have a lib64-core-image-lsb-sdk image built
> > > with DISTRO=poky and no pam in DISTRO_FEATURES, see the last build on
> > > nightly-multilib... :(
> > 
> > If we're doing this then we should be changing the autobuilder so it
> > doesn't. LSB images should not be built in non-LSB configuration.
> 
> I kind of disagree with that, the LSB image can take into account
> configuration in other parts of the system. If pam isn't configured, I'm
> not sure that should automatically make it completely unbuildable...

For other situations I would agree, but the fact that the image has "lsb" in 
its name will naturally lead users to assume that that the output will be 
something LSB-compliant, but if poky-lsb or some other similar distro 
configuration is not used to build it then it will not be. Building something 
that's almost-LSB but not quite but is still labelled LSB just dilutes the 
meaning of LSB, and therefore doesn't seem to me to be particularly desirable.

Since we can't just check if DISTRO is "poky-lsb", an alternative check would 
be to look for "linuxstdbase" in OVERRIDES since we ought to be able to rely 
upon that given usage within other recipes.

Cheers,
Paul
Ross Burton - March 31, 2014, 5:31 p.m.
On 31 March 2014 18:03, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> I kind of disagree with that, the LSB image can take into account
> configuration in other parts of the system. If pam isn't configured, I'm
> not sure that should automatically make it completely unbuildable...

An image that claims to be LSB compliant but due to
not-immediately-obvious DISTRO_FEATURES isn't actually doesn't seem
like a good idea to me, fwiw.  If the LSB image requires PAM, X11 and
so on then it should require the features.

Ross
Mark Hatle - March 31, 2014, 5:57 p.m.
On 3/31/14, 12:31 PM, Burton, Ross wrote:
> On 31 March 2014 18:03, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
>> I kind of disagree with that, the LSB image can take into account
>> configuration in other parts of the system. If pam isn't configured, I'm
>> not sure that should automatically make it completely unbuildable...
>
> An image that claims to be LSB compliant but due to
> not-immediately-obvious DISTRO_FEATURES isn't actually doesn't seem
> like a good idea to me, fwiw.  If the LSB image requires PAM, X11 and
> so on then it should require the features.

I agree the image with the name LSB should enforce the items it knows the LSB 
depends on.  If the user patches that away then it becomes their issue.

--Mark

> Ross
>

Patch

diff --git a/meta/recipes-extended/images/core-image-lsb.bb b/meta/recipes-extended/images/core-image-lsb.bb
index ed316a6..ab61c6e 100644
--- a/meta/recipes-extended/images/core-image-lsb.bb
+++ b/meta/recipes-extended/images/core-image-lsb.bb
@@ -9,4 +9,6 @@  IMAGE_INSTALL = "\
     packagegroup-core-lsb \
     "
 
-inherit core-image
+inherit core-image distro_features_check
+
+REQUIRED_DISTRO_FEATURES = "pam"