[PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE

[PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE

[Otavio Salvador]

When a MACHINEOVERRIDES has more than one value, split by ':' as usual
OVERRIDES, this were not being properly checked in COMPATIBLE_MACHINE
matching as we need to iterate over each SoC family and check if it is
compatible or not.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
Changes for v4:
- Fix wrong assignment of compat_machines

 meta/classes/base.bbclass | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index abd6a52..641316d 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -515,11 +515,12 @@ python () {
         need_machine = d.getVar('COMPATIBLE_MACHINE', True)
         if need_machine:
             import re
-            this_machine = d.getVar('MACHINE', True)
-            if this_machine and not re.match(need_machine, this_machine):
-                this_soc_family = d.getVar('SOC_FAMILY', True)
-                if (this_soc_family and not re.match(need_machine, this_soc_family)) or not this_soc_family:
-                    raise bb.parse.SkipPackage("incompatible with machine %s (not in COMPATIBLE_MACHINE)" % this_machine)
+            compat_machines = (d.getVar('MACHINEOVERRIDES', True) or "").split(":")
+            for m in compat_machines:
+                if re.match(need_machine, m):
+                    break
+            else:
+                raise bb.parse.SkipPackage("incompatible with machine %s (not in COMPATIBLE_MACHINE)" % d.getVar('MACHINE', True))
 
 
         bad_licenses = (d.getVar('INCOMPATIBLE_LICENSE', True) or "").split()
-- 
1.8.1

Re: [PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE

[Phil Blundell]

On Mon, 2013-04-08 at 18:42 -0300, Otavio Salvador wrote:
> When a MACHINEOVERRIDES has more than one value, split by ':' as usual
> OVERRIDES, this were not being properly checked in COMPATIBLE_MACHINE
> matching as we need to iterate over each SoC family and check if it is
> compatible or not.

[...]

>              import re
> -            this_machine = d.getVar('MACHINE', True)
> -            if this_machine and not re.match(need_machine, this_machine):
> -                this_soc_family = d.getVar('SOC_FAMILY', True)
> -                if (this_soc_family and not re.match(need_machine, this_soc_family)) or not this_soc_family:
> -                    raise bb.parse.SkipPackage("incompatible with machine %s (not in COMPATIBLE_MACHINE)" % this_machine)
> +            compat_machines = (d.getVar('MACHINEOVERRIDES', True) or "").split(":")
> +            for m in compat_machines:
> +                if re.match(need_machine, m):

The checkin comment above doesn't really capture the semantic changes
going on in this patch.  The comment implies that there was previously
an issue with MACHINEOVERRIDES not being split when it contains multiple
entries, but this is misleading: prior to this patch, COMPATIBLE_MACHINE
wasn't being matched against MACHINEOVERRIDES at all.  So, the effect of
this is that anything in MACHINEOVERRIDES (which could potentially be
quite a large set of strings) will be considered as a candidate for
matching COMPATIBLE_MACHINE.

It's not abundantly clear to me whether this is a good thing or not, but
it certainly ought to be accurately described in the commit log.

p.

Re: [PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE

[Otavio Salvador]

On Tue, Apr 9, 2013 at 7:05 AM, Phil Blundell <pb@pbcl.net> wrote:
> On Mon, 2013-04-08 at 18:42 -0300, Otavio Salvador wrote:
>> When a MACHINEOVERRIDES has more than one value, split by ':' as usual
>> OVERRIDES, this were not being properly checked in COMPATIBLE_MACHINE
>> matching as we need to iterate over each SoC family and check if it is
>> compatible or not.
>
> [...]
>
>>              import re
>> -            this_machine = d.getVar('MACHINE', True)
>> -            if this_machine and not re.match(need_machine, this_machine):
>> -                this_soc_family = d.getVar('SOC_FAMILY', True)
>> -                if (this_soc_family and not re.match(need_machine, this_soc_family)) or not this_soc_family:
>> -                    raise bb.parse.SkipPackage("incompatible with machine %s (not in COMPATIBLE_MACHINE)" % this_machine)
>> +            compat_machines = (d.getVar('MACHINEOVERRIDES', True) or "").split(":")
>> +            for m in compat_machines:
>> +                if re.match(need_machine, m):
>
> The checkin comment above doesn't really capture the semantic changes
> going on in this patch.  The comment implies that there was previously
> an issue with MACHINEOVERRIDES not being split when it contains multiple
> entries, but this is misleading: prior to this patch, COMPATIBLE_MACHINE
> wasn't being matched against MACHINEOVERRIDES at all.  So, the effect of
> this is that anything in MACHINEOVERRIDES (which could potentially be
> quite a large set of strings) will be considered as a candidate for
> matching COMPATIBLE_MACHINE.
>
> It's not abundantly clear to me whether this is a good thing or not, but
> it certainly ought to be accurately described in the commit log.

In fact it was being matched as SOC_FAMILY, in fact, extended
MACHINEOVERRIDES. I will send a new version with improved commit log
in either case.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br

Re: [PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE

[Richard Purdie]

On Tue, 2013-04-09 at 11:05 +0100, Phil Blundell wrote:
> On Mon, 2013-04-08 at 18:42 -0300, Otavio Salvador wrote:
> > When a MACHINEOVERRIDES has more than one value, split by ':' as usual
> > OVERRIDES, this were not being properly checked in COMPATIBLE_MACHINE
> > matching as we need to iterate over each SoC family and check if it is
> > compatible or not.
> 
> [...]
> 
> >              import re
> > -            this_machine = d.getVar('MACHINE', True)
> > -            if this_machine and not re.match(need_machine, this_machine):
> > -                this_soc_family = d.getVar('SOC_FAMILY', True)
> > -                if (this_soc_family and not re.match(need_machine, this_soc_family)) or not this_soc_family:
> > -                    raise bb.parse.SkipPackage("incompatible with machine %s (not in COMPATIBLE_MACHINE)" % this_machine)
> > +            compat_machines = (d.getVar('MACHINEOVERRIDES', True) or "").split(":")
> > +            for m in compat_machines:
> > +                if re.match(need_machine, m):
> 
> The checkin comment above doesn't really capture the semantic changes
> going on in this patch.  The comment implies that there was previously
> an issue with MACHINEOVERRIDES not being split when it contains multiple
> entries, but this is misleading: prior to this patch, COMPATIBLE_MACHINE
> wasn't being matched against MACHINEOVERRIDES at all.  So, the effect of
> this is that anything in MACHINEOVERRIDES (which could potentially be
> quite a large set of strings) will be considered as a candidate for
> matching COMPATIBLE_MACHINE.
> 
> It's not abundantly clear to me whether this is a good thing or not, but
> it certainly ought to be accurately described in the commit log.

Agreed. We're getting close to the release so in this case I've
rewritten the commit message since I need to get things pulled together
for the -rc build. People do need to think more about commit messages
and how to convey the right information in them...

Cheers,

Richard