[PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Peter Kjellerstedt]

There is no reason to set PACKAGECONFIG_class-native to the same value
as the default PACKAGECONFIG.

Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
---
 meta/recipes-devtools/file/file_5.38.bb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meta/recipes-devtools/file/file_5.38.bb b/meta/recipes-devtools/file/file_5.38.bb
index f5ad568936..007db4790d 100644
--- a/meta/recipes-devtools/file/file_5.38.bb
+++ b/meta/recipes-devtools/file/file_5.38.bb
@@ -19,7 +19,6 @@ S = "${WORKDIR}/git"
 inherit autotools update-alternatives
 
 PACKAGECONFIG ??= "zlib"
-PACKAGECONFIG_class-native ??= "zlib"
 PACKAGECONFIG[bz2] = "--enable-bzlib, --disable-bzlib, bzip2"
 PACKAGECONFIG[lzma] = "--enable-xzlib, --disable-xzlib, xz"
 PACKAGECONFIG[zlib] = "--enable-zlib, --disable-zlib, zlib"
-- 
2.21.3

Re: [OE-core] [PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Andre McCurdy]

On Tue, May 12, 2020 at 2:43 PM Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
>
> There is no reason to set PACKAGECONFIG_class-native to the same value
> as the default PACKAGECONFIG.

End users often don't know how to safely change PACKAGECONFIG from a
.bbappend and might try PACKAGECONFIG += "foo" or PACKAGECONFIG_append
= " foo" without realising that it could affect more than just the
target build.

Have an explicit PACKAGECONFIG_class-native keeps the native config
stable for users who haven't yet learning through experience that the
safe way to change PACKAGECONFIG is PACKAGECONFIG_append_class-target
= " foo".

So although this PACKAGECONFIG_class-native may seem redundant to the
experts, it helps make OE more robust for regular users.

> Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> ---
>  meta/recipes-devtools/file/file_5.38.bb | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/meta/recipes-devtools/file/file_5.38.bb b/meta/recipes-devtools/file/file_5.38.bb
> index f5ad568936..007db4790d 100644
> --- a/meta/recipes-devtools/file/file_5.38.bb
> +++ b/meta/recipes-devtools/file/file_5.38.bb
> @@ -19,7 +19,6 @@ S = "${WORKDIR}/git"
>  inherit autotools update-alternatives
>
>  PACKAGECONFIG ??= "zlib"
> -PACKAGECONFIG_class-native ??= "zlib"
>  PACKAGECONFIG[bz2] = "--enable-bzlib, --disable-bzlib, bzip2"
>  PACKAGECONFIG[lzma] = "--enable-xzlib, --disable-xzlib, xz"
>  PACKAGECONFIG[zlib] = "--enable-zlib, --disable-zlib, zlib"
> --
> 2.21.3
>
> 

Re: [OE-core] [PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Peter Kjellerstedt]

> -----Original Message-----
> From: Andre McCurdy <armccurdy@gmail.com>
> Sent: den 13 maj 2020 00:20
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> Cc: OE Core mailing list <openembedded-core@lists.openembedded.org>
> Subject: Re: [OE-core] [PATCH] file: Remove unneccessary override of
> PACKAGECONFIG for native
> 
> On Tue, May 12, 2020 at 2:43 PM Peter Kjellerstedt
> <peter.kjellerstedt@axis.com> wrote:
> >
> > There is no reason to set PACKAGECONFIG_class-native to the same value
> > as the default PACKAGECONFIG.
> 
> End users often don't know how to safely change PACKAGECONFIG from a
> .bbappend and might try PACKAGECONFIG += "foo" or PACKAGECONFIG_append
> = " foo" without realising that it could affect more than just the
> target build.
> 
> Have an explicit PACKAGECONFIG_class-native keeps the native config
> stable for users who haven't yet learning through experience that the
> safe way to change PACKAGECONFIG is PACKAGECONFIG_append_class-target
> = " foo".
> 
> So although this PACKAGECONFIG_class-native may seem redundant to the
> experts, it helps make OE more robust for regular users.

I have to disagree with this. Having just PACKAGECONFIG ??= "zlib" 
makes for a more consistent behavior between the different classes. 
E.g., if someone would add a .bbappend with PACKAGECONFIG = "zlib 
bz2", with the current recipe the behavior would be different when 
building file and nativesdk-file compared to file-native (or actually 
file-replacement-native as file-native is in ASSUME_PROVIDED). With 
only the default PACKAGECONFIG, the result would be the same also 
for file-native. 

A quick grep through OE-Core also seems to agree with me. There are 
a couple of other recipes that sets PACKAGECONFIG_class- defaults, 
but then it's due to the default for, e.g., native being different 
from target. The only other recipe I could find that sets multiple 
identical PACKAGECONFIG defaults is openssl.

//Peter

> > Signed-off-by: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > ---
> >  meta/recipes-devtools/file/file_5.38.bb | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/meta/recipes-devtools/file/file_5.38.bb b/meta/recipes-devtools/file/file_5.38.bb
> > index f5ad568936..007db4790d 100644
> > --- a/meta/recipes-devtools/file/file_5.38.bb
> > +++ b/meta/recipes-devtools/file/file_5.38.bb
> > @@ -19,7 +19,6 @@ S = "${WORKDIR}/git"
> >  inherit autotools update-alternatives
> >
> >  PACKAGECONFIG ??= "zlib"
> > -PACKAGECONFIG_class-native ??= "zlib"
> >  PACKAGECONFIG[bz2] = "--enable-bzlib, --disable-bzlib, bzip2"
> >  PACKAGECONFIG[lzma] = "--enable-xzlib, --disable-xzlib, xz"
> >  PACKAGECONFIG[zlib] = "--enable-zlib, --disable-zlib, zlib"
> > --
> > 2.21.3
> >
> > 

Re: [OE-core] [PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Andre McCurdy]

On Wed, May 13, 2020 at 10:14 AM Peter Kjellerstedt
<peter.kjellerstedt@axis.com> wrote:
>
> > -----Original Message-----
> > From: Andre McCurdy <armccurdy@gmail.com>
> > Sent: den 13 maj 2020 00:20
> > To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
> > Cc: OE Core mailing list <openembedded-core@lists.openembedded.org>
> > Subject: Re: [OE-core] [PATCH] file: Remove unneccessary override of
> > PACKAGECONFIG for native
> >
> > On Tue, May 12, 2020 at 2:43 PM Peter Kjellerstedt
> > <peter.kjellerstedt@axis.com> wrote:
> > >
> > > There is no reason to set PACKAGECONFIG_class-native to the same value
> > > as the default PACKAGECONFIG.
> >
> > End users often don't know how to safely change PACKAGECONFIG from a
> > .bbappend and might try PACKAGECONFIG += "foo" or PACKAGECONFIG_append
> > = " foo" without realising that it could affect more than just the
> > target build.
> >
> > Have an explicit PACKAGECONFIG_class-native keeps the native config
> > stable for users who haven't yet learning through experience that the
> > safe way to change PACKAGECONFIG is PACKAGECONFIG_append_class-target
> > = " foo".
> >
> > So although this PACKAGECONFIG_class-native may seem redundant to the
> > experts, it helps make OE more robust for regular users.
>
> I have to disagree with this. Having just PACKAGECONFIG ??= "zlib"
> makes for a more consistent behavior between the different classes.

I don't see why a consistent configuration between the different
classes is a goal. We should instead try to keep them independent so
that changes to one don't accidentally affect the others. The
configuration for the target is determined by the functionality
required in the final image (ie something determined by the user) and
the configuration for native and nativesdk is mostly determined by the
requirements of the build environment. The requirements for the build
environment are fairly static and not something a typical user would
want to mess with.

> E.g., if someone would add a .bbappend with PACKAGECONFIG = "zlib
> bz2", with the current recipe the behavior would be different when
> building file and nativesdk-file compared to file-native (or actually
> file-replacement-native as file-native is in ASSUME_PROVIDED). With
> only the default PACKAGECONFIG, the result would be the same also
> for file-native.

Having an over-ride for native but not considering nativesdk is
certainly an issue which seems to affect many recipes in oe-core. But
the fix should be adding an over-ride for nativesdk rather than
removing the over-ride for native.

(I don't know why this problem doesn't come up more often. Perhaps
because while there are users who like to change PACKAGECONFIG values
and users who build for nativesdk the overlap between them is actually
quite small?).

> A quick grep through OE-Core also seems to agree with me. There are
> a couple of other recipes that sets PACKAGECONFIG_class- defaults,
> but then it's due to the default for, e.g., native being different
> from target. The only other recipe I could find that sets multiple
> identical PACKAGECONFIG defaults is openssl.

Yes. Unfortunately making recipes robust for users who want to change
PACKAGECONFIG values doesn't seem to be a high priority for oe-core.
Why else would default PACKAGECONFIG values be assigned with ??=
instead of ?= other than as a trap for new users?

Re: [OE-core] [PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Richard Purdie]

On Wed, 2020-05-13 at 12:30 -0700, Andre McCurdy wrote:
> Yes. Unfortunately making recipes robust for users who want to change
> PACKAGECONFIG values doesn't seem to be a high priority for oe-core.
> Why else would default PACKAGECONFIG values be assigned with ??=
> instead of ?= other than as a trap for new users?

I'm not sure I understand that, equally I'm tired so I'll assume I'm
just missing something.

I will say that there is no intention to make OE-Core hard to use or
not robust, its only as good a the people contributing make it. In this
case I'm not sure I understand/see the problem.

If we should be using ?= instead of ??= then I'm open to an explanation
why and patches to improve things.

Unfortunately we do have quite some legacy and ??= vs ?= has been
troublesome in the many different contexts we have (recipe, class,
conf, inc and so on).

Cheers,

Richard

Re: [OE-core] [PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Andre McCurdy]

On Wed, May 13, 2020 at 2:31 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Wed, 2020-05-13 at 12:30 -0700, Andre McCurdy wrote:
> > Yes. Unfortunately making recipes robust for users who want to change
> > PACKAGECONFIG values doesn't seem to be a high priority for oe-core.
> > Why else would default PACKAGECONFIG values be assigned with ??=
> > instead of ?= other than as a trap for new users?
>
> I'm not sure I understand that, equally I'm tired so I'll assume I'm
> just missing something.
>
> I will say that there is no intention to make OE-Core hard to use or
> not robust, its only as good a the people contributing make it. In this
> case I'm not sure I understand/see the problem.
>
> If we should be using ?= instead of ??= then I'm open to an explanation
> why and patches to improve things.

If the default is assigned with ??= then trying to add to it with +=
results in the default value being lost. If the default is assigned
with ?= then adding to it with += works as expected.

To robustly enable a new PACKAGECONFIG option, users need to learn to
use PACKAGECONFIG_append (and obviously learn about the leading space
requirement there too) and not to try PACKAGECONFIG +=.

The reference manual does at least have an example which uses _append.
However it doesn't mention that += isn't safe or that _append will
affect native(sdk) and therefore _append_class-target is the safest
approach.

> Unfortunately we do have quite some legacy and ??= vs ?= has been
> troublesome in the many different contexts we have (recipe, class,
> conf, inc and so on).

Re: [OE-core] [PATCH] file: Remove unneccessary override of PACKAGECONFIG for native

[Peter Kjellerstedt]

> -----Original Message-----
> From: Andre McCurdy <armccurdy@gmail.com>
> Sent: den 14 maj 2020 00:49
> To: Richard Purdie <richard.purdie@linuxfoundation.org>
> Cc: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; OE Core mailing
> list <openembedded-core@lists.openembedded.org>
> Subject: Re: [OE-core] [PATCH] file: Remove unneccessary override of
> PACKAGECONFIG for native
> 
> On Wed, May 13, 2020 at 2:31 PM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Wed, 2020-05-13 at 12:30 -0700, Andre McCurdy wrote:
> > > Yes. Unfortunately making recipes robust for users who want to change
> > > PACKAGECONFIG values doesn't seem to be a high priority for oe-core.
> > > Why else would default PACKAGECONFIG values be assigned with ??=
> > > instead of ?= other than as a trap for new users?
> >
> > I'm not sure I understand that, equally I'm tired so I'll assume I'm
> > just missing something.
> >
> > I will say that there is no intention to make OE-Core hard to use or
> > not robust, its only as good a the people contributing make it. In this
> > case I'm not sure I understand/see the problem.
> >
> > If we should be using ?= instead of ??= then I'm open to an explanation
> > why and patches to improve things.
> 
> If the default is assigned with ??= then trying to add to it with +=
> results in the default value being lost. If the default is assigned
> with ?= then adding to it with += works as expected.
> 
> To robustly enable a new PACKAGECONFIG option, users need to learn to
> use PACKAGECONFIG_append (and obviously learn about the leading space
> requirement there too) and not to try PACKAGECONFIG +=.
> 
> The reference manual does at least have an example which uses _append.
> However it doesn't mention that += isn't safe or that _append will
> affect native(sdk) and therefore _append_class-target is the safest
> approach.
> 
> > Unfortunately we do have quite some legacy and ??= vs ?= has been
> > troublesome in the many different contexts we have (recipe, class,
> > conf, inc and so on).

Actually, I have always wondered why the recipes in OE-Core typically 
use PACKAGECONFIG ??= "..." when no other variables (in the recipes) 
are defined that way. After all, if I want to override the default 
value in a bbappend as PACKAGECONFIG = "...", that works just as well 
if the variable is defined using = rather than ?= or ??=. And if I do 
it in local.conf, I would typically do it using PACKAGECONFIG_pn-foo 
and then it also doesn't matter whether = or ??= was used to define 
the default in the recipe. Add to that the complications with not 
being able to use += together with ??= in a sensible way, it even 
makes less sense.

For the record, in our own recipes we use PACKAGECONFIG = "..." in the 
recipes and that would be my recommendation for OE-Core as well.

//Peter