From: Richard Purdie <richard.purdie@linuxfoundation.org>
To: Phil Blundell <pb@pbcl.net>
Cc: OpenEmbedded, Otavio Salvador <otavio@ossystems.com.br>,
List <openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE
Date: Tue, 09 Apr 2013 13:22:14 +0100 [thread overview]
Message-ID: <1365510134.12407.76.camel@ted> (raw)
In-Reply-To: <1365501911.7011.38.camel@phil-desktop.brightsign>
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
prev parent reply other threads:[~2013-04-09 12:39 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-08 21:42 [PATCH v4] base.bbclass: Fix matching of MACHINEOVERRIDES in COMPATIBLE_MACHINE Otavio Salvador
2013-04-09 10:05 ` Phil Blundell
2013-04-09 11:20 ` Otavio Salvador
2013-04-09 12:22 ` Richard Purdie [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1365510134.12407.76.camel@ted \
--to=richard.purdie@linuxfoundation.org \
--cc=openembedded-core@lists.openembedded.org \
--cc=otavio@ossystems.com.br \
--cc=pb@pbcl.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.