Patchwork allarch: Allow class to be included but overridden

login
register
mail settings
Submitter Richard Purdie
Date Nov. 25, 2012, 8:23 p.m.
Message ID <1353874980.21863.20.camel@ted>
Download mbox | patch
Permalink /patch/39559/
State Accepted
Commit 7dd91402b719ac62b51088f234354f82bfa9c4b6
Headers show

Comments

Richard Purdie - Nov. 25, 2012, 8:23 p.m.
We have cases where we'd like to inherit this class by default but allow
special cases to override it. This change makes the code of the class
conditional on PACKAGE_ARCH remaining set to "all", allowing it to be
overridden. packagegroup usage is one case this is desirable.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
Otavio Salvador - Nov. 26, 2012, 11:08 a.m.
On Sun, Nov 25, 2012 at 6:23 PM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> We have cases where we'd like to inherit this class by default but allow
> special cases to override it. This change makes the code of the class
> conditional on PACKAGE_ARCH remaining set to "all", allowing it to be
> overridden. packagegroup usage is one case this is desirable.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---

Richard, I fail to see the usecase of packagroup. Can you clarify?
Richard Purdie - Nov. 26, 2012, 11:17 a.m.
On Mon, 2012-11-26 at 09:08 -0200, Otavio Salvador wrote:
> On Sun, Nov 25, 2012 at 6:23 PM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > We have cases where we'd like to inherit this class by default but allow
> > special cases to override it. This change makes the code of the class
> > conditional on PACKAGE_ARCH remaining set to "all", allowing it to be
> > overridden. packagegroup usage is one case this is desirable.
> >
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> 
> Richard, I fail to see the usecase of packagroup. Can you clarify?

packagegroup.bbclass previously set PACKAGE_ARCH = "all" which was
confusing other code. Anything using the all package arch should really
use allarch so we are consistent about how we set the various variables.

Even though packagegroup.bbclass now uses allarch, we need specific
packagegroup packages to be able to override the class e.g. marking
themselves machine specific. This is why we need to make this change.

Cheers,

Richard
Otavio Salvador - Nov. 26, 2012, 11:19 a.m.
On Mon, Nov 26, 2012 at 9:17 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> On Mon, 2012-11-26 at 09:08 -0200, Otavio Salvador wrote:
>> On Sun, Nov 25, 2012 at 6:23 PM, Richard Purdie
>> <richard.purdie@linuxfoundation.org> wrote:
>> > We have cases where we'd like to inherit this class by default but allow
>> > special cases to override it. This change makes the code of the class
>> > conditional on PACKAGE_ARCH remaining set to "all", allowing it to be
>> > overridden. packagegroup usage is one case this is desirable.
>> >
>> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>> > ---
>>
>> Richard, I fail to see the usecase of packagroup. Can you clarify?
>
> packagegroup.bbclass previously set PACKAGE_ARCH = "all" which was
> confusing other code. Anything using the all package arch should really
> use allarch so we are consistent about how we set the various variables.
>
> Even though packagegroup.bbclass now uses allarch, we need specific
> packagegroup packages to be able to override the class e.g. marking
> themselves machine specific. This is why we need to make this change.

My only concern is if we could make allarch to raise an exception if
you inherit it and set package arch to another value?

The only bad effect about it is we won't be able to change something
in a bbappend which makes the package arch specific that way.
Richard Purdie - Nov. 26, 2012, 11:22 a.m.
On Mon, 2012-11-26 at 09:19 -0200, Otavio Salvador wrote:
> On Mon, Nov 26, 2012 at 9:17 AM, Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Mon, 2012-11-26 at 09:08 -0200, Otavio Salvador wrote:
> >> On Sun, Nov 25, 2012 at 6:23 PM, Richard Purdie
> >> <richard.purdie@linuxfoundation.org> wrote:
> >> > We have cases where we'd like to inherit this class by default but allow
> >> > special cases to override it. This change makes the code of the class
> >> > conditional on PACKAGE_ARCH remaining set to "all", allowing it to be
> >> > overridden. packagegroup usage is one case this is desirable.
> >> >
> >> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> >> > ---
> >>
> >> Richard, I fail to see the usecase of packagroup. Can you clarify?
> >
> > packagegroup.bbclass previously set PACKAGE_ARCH = "all" which was
> > confusing other code. Anything using the all package arch should really
> > use allarch so we are consistent about how we set the various variables.
> >
> > Even though packagegroup.bbclass now uses allarch, we need specific
> > packagegroup packages to be able to override the class e.g. marking
> > themselves machine specific. This is why we need to make this change.
> 
> My only concern is if we could make allarch to raise an exception if
> you inherit it and set package arch to another value?

We're deciding that we're supporting that exact usecase though?

> The only bad effect about it is we won't be able to change something
> in a bbappend which makes the package arch specific that way.

Why not? This should be perfectly usable from a .bbappend?

Cheers,

Richard

Patch

diff --git a/meta/classes/allarch.bbclass b/meta/classes/allarch.bbclass
index 21157e5..8669470 100644
--- a/meta/classes/allarch.bbclass
+++ b/meta/classes/allarch.bbclass
@@ -4,20 +4,25 @@ 
 
 PACKAGE_ARCH = "all"
 
-# No need for virtual/libc or a cross compiler
-INHIBIT_DEFAULT_DEPS = "1"
+python () {
+    # Allow this class to be included but overridden - only set
+    # the values if we're still "all" package arch.
+    if d.getVar("PACKAGE_ARCH") == "all":
+        # No need for virtual/libc or a cross compiler
+        d.setVar("INHIBIT_DEFAULT_DEPS","1")
 
-# Set these to a common set of values, we shouldn't be using them other that for WORKDIR directory
-# naming anyway
-TARGET_ARCH = "allarch"
-TARGET_OS = "linux"
-TARGET_CC_ARCH = "none"
-TARGET_LD_ARCH = "none"
-TARGET_AS_ARCH = "none"
-PACKAGE_EXTRA_ARCHS = ""
+        # Set these to a common set of values, we shouldn't be using them other that for WORKDIR directory
+        # naming anyway
+        d.setVar("TARGET_ARCH", "allarch")
+        d.setVar("TARGET_OS", "linux")
+        d.setVar("TARGET_CC_ARCH", "none")
+        d.setVar("TARGET_LD_ARCH", "none")
+        d.setVar("TARGET_AS_ARCH", "none")
+        d.setVar("PACKAGE_EXTRA_ARCHS", "")
 
-# No need to do shared library processing or debug symbol handling
-EXCLUDE_FROM_SHLIBS = "1"
-INHIBIT_PACKAGE_DEBUG_SPLIT = "1"
-INHIBIT_PACKAGE_STRIP = "1"
+        # No need to do shared library processing or debug symbol handling
+        d.setVar("EXCLUDE_FROM_SHLIBS", "1")
+        d.setVar("INHIBIT_PACKAGE_DEBUG_SPLIT", "1")
+        d.setVar("INHIBIT_PACKAGE_STRIP", "1")
+}