[PATCH] base.bbclass: fix soc-family test

[PATCH] base.bbclass: fix soc-family test

[Frans Meulenbroeks]

see http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-September/023680.html

Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
---
 classes/base.bbclass |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/classes/base.bbclass b/classes/base.bbclass
index 299e875..2c9ad89 100644
--- a/classes/base.bbclass
+++ b/classes/base.bbclass
@@ -337,7 +337,7 @@ python () {
             this_machine = bb.data.getVar('MACHINE', d, 1)
             if this_machine and not re.match(need_machine, this_machine):
                 this_soc_family = bb.data.getVar('SOC_FAMILY', d, 1)
-                if this_soc_family and not re.match(need_machine, this_soc_family):
+                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" % this_machine)
 
         need_target = bb.data.getVar('COMPATIBLE_TARGET_SYS', d, 1)
-- 
1.6.4.2

Re: [PATCH] base.bbclass: fix soc-family test

[Frans Meulenbroeks]

2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
> see http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-September/023680.html
>
> Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> ---
>  classes/base.bbclass |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/classes/base.bbclass b/classes/base.bbclass
> index 299e875..2c9ad89 100644
> --- a/classes/base.bbclass
> +++ b/classes/base.bbclass
> @@ -337,7 +337,7 @@ python () {
>             this_machine = bb.data.getVar('MACHINE', d, 1)
>             if this_machine and not re.match(need_machine, this_machine):
>                 this_soc_family = bb.data.getVar('SOC_FAMILY', d, 1)
> -                if this_soc_family and not re.match(need_machine, this_soc_family):
> +                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" % this_machine)
>
>         need_target = bb.data.getVar('COMPATIBLE_TARGET_SYS', d, 1)
> --
> 1.6.4.2
>

Bump.

This is out for a week now without any feedback, although the patch is
quite trivial.

Actually I am quite disappointed by the developers who created and
pushed the SOC_FAMILY patch.
They were very eager to push this change in a day, without waiting for
the discussion on it to conclude, but they seem to be not-so-willing
to review a fix for a problem they caused (let alone resolve the
problem). What is even somewhat irritating is that one of the people
involved is often in the front seat when it comes to criticizing
others if they make a mistake.

Frans.

Re: [PATCH] base.bbclass: fix soc-family test

[Phil Blundell]

On Thu, 2010-09-09 at 09:11 +0200, Frans Meulenbroeks wrote:
> 2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
> > -                if this_soc_family and not re.match(need_machine, this_soc_family):
> > +                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" % this_machine)

I am still far from convinced that this hunk of SOC_FAMILY code is
desirable to have in the first place but, if it's going to stay there,
clearly it should be fixed so that it doesn't cause problems for
non-users.  

So, yes, your patch looks fine to me.

p.

Re: [PATCH] base.bbclass: fix soc-family test

[Leon Woestenberg]

Hello,

On Thu, Sep 9, 2010 at 11:30 AM, Phil Blundell <philb@gnu.org> wrote:
> On Thu, 2010-09-09 at 09:11 +0200, Frans Meulenbroeks wrote:
>> 2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
>> > -                if this_soc_family and not re.match(need_machine, this_soc_family):
>> > +                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" % this_machine)
>
> I am still far from convinced that this hunk of SOC_FAMILY code is
> desirable to have in the first place but, if it's going to stay there,
> clearly it should be fixed so that it doesn't cause problems for
> non-users.
>
Same feeling here.

I don't think we should clutter OpenEmbedded with fixes for one rare
case and leave it unattended / non-understood / unmaintained for all
other cases.

Hands up for those who can maintain SOC_FAMILY support. :-)

Regards,
-- 
Leon

Re: [PATCH] base.bbclass: fix soc-family test

[Frans Meulenbroeks]

2010/9/9 Phil Blundell <philb@gnu.org>:
> On Thu, 2010-09-09 at 09:11 +0200, Frans Meulenbroeks wrote:
>> 2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
>> > -                if this_soc_family and not re.match(need_machine, this_soc_family):
>> > +                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" % this_machine)
>
> I am still far from convinced that this hunk of SOC_FAMILY code is
> desirable to have in the first place but, if it's going to stay there,
> clearly it should be fixed so that it doesn't cause problems for
> non-users.
>
> So, yes, your patch looks fine to me.
>
> p.
>
Phil, thanks for the feedback.

I'm not too sure on the usefulness of it either, but there is some
breakage so we should either revert the patch or fix it.

Actually the SOC_FAMILY got pushed before the review was concluded.
Technically it got the ack's but the discussion was still ongoing when
this was pushed.
What also makes it fishy is that all acks are from the same company
(which is the same one as the person who submitted the patch):

Signed-off-by: Chase Maupin <chase.maupin@ti.com>
Acked-by: Denys Dmytriyenko <denys@ti.com>
Acked-by: Koen Kooi <k-kooi@ti.com>
Signed-off-by: Koen Kooi <koen@openembedded.org>

I would suggest modifying the commit policy disallowing these kind of
things, saying the two Ack's must be from two developers not
affiliated with the same company.
(actually we might even want to take it further and saying that global
changes that affect everyone would require ACK's from developers from
more than one distro).

Question to the TSC: should the SOC_FAMILY patch be reverted and put
on hold until there is agreement whether or not we want this?

Frans

Re: [PATCH] base.bbclass: fix soc-family test

[Maupin, Chase]

> -----Original Message-----
> From: openembedded-devel-bounces@lists.openembedded.org
> [mailto:openembedded-devel-bounces@lists.openembedded.org] On Behalf Of
> Frans Meulenbroeks
> Sent: Thursday, September 09, 2010 5:12 AM
> To: openembedded-devel@lists.openembedded.org; tsc@openembedded.org
> Subject: Re: [oe] [PATCH] base.bbclass: fix soc-family test
> 
> 2010/9/9 Phil Blundell <philb@gnu.org>:
> > On Thu, 2010-09-09 at 09:11 +0200, Frans Meulenbroeks wrote:
> >> 2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
> >> > -                if this_soc_family and not re.match(need_machine,
> this_soc_family):
> >> > +                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" % this_machine)
> >
> > I am still far from convinced that this hunk of SOC_FAMILY code is
> > desirable to have in the first place but, if it's going to stay there,
> > clearly it should be fixed so that it doesn't cause problems for
> > non-users.
> >
> > So, yes, your patch looks fine to me.
> >
> > p.
> >
> Phil, thanks for the feedback.
> 
> I'm not too sure on the usefulness of it either, but there is some
> breakage so we should either revert the patch or fix it.
> 
> Actually the SOC_FAMILY got pushed before the review was concluded.
> Technically it got the ack's but the discussion was still ongoing when
> this was pushed.
> What also makes it fishy is that all acks are from the same company
> (which is the same one as the person who submitted the patch):
> 
> Signed-off-by: Chase Maupin <chase.maupin@ti.com>
> Acked-by: Denys Dmytriyenko <denys@ti.com>
> Acked-by: Koen Kooi <k-kooi@ti.com>
> Signed-off-by: Koen Kooi <koen@openembedded.org>
> 
> I would suggest modifying the commit policy disallowing these kind of
> things, saying the two Ack's must be from two developers not
> affiliated with the same company.
> (actually we might even want to take it further and saying that global
> changes that affect everyone would require ACK's from developers from
> more than one distro).
> 
> Question to the TSC: should the SOC_FAMILY patch be reverted and put
> on hold until there is agreement whether or not we want this?

Frans,

As the person who submitted this change I'd like to ask that it not be reverted.  I am using it currently so that when defining COMPATIBLE_MACHINE for recipes targeted at OMAP3 devices I do not have to keep a rolling list of all the various OMAP3 devices.  This can quickly become:

COMPATIBLE_MACHINE = "dm37x-evm|am37x-evm|omap3evm|am3517-evm|beagleboard|......"

Instead by allowing SOC_FAMILY to be used as a COMPATIBLE_MACHINE this can be handled cleanly with:

COMPATIBLE_MACHINE = "omap3"

Please consider this use case.  I would much prefer if your fix was put into the base.bbclass than if this were removed.

> 
> Frans
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel

Re: [PATCH] base.bbclass: fix soc-family test

[Frans Meulenbroeks]

2010/9/9 Maupin, Chase <chase.maupin@ti.com>:

>>
>> I'm not too sure on the usefulness of it either, but there is some
>> breakage so we should either revert the patch or fix it.
>>
>> Actually the SOC_FAMILY got pushed before the review was concluded.
>> Technically it got the ack's but the discussion was still ongoing when
>> this was pushed.
>> What also makes it fishy is that all acks are from the same company
>> (which is the same one as the person who submitted the patch):
>>
>> Signed-off-by: Chase Maupin <chase.maupin@ti.com>
>> Acked-by: Denys Dmytriyenko <denys@ti.com>
>> Acked-by: Koen Kooi <k-kooi@ti.com>
>> Signed-off-by: Koen Kooi <koen@openembedded.org>
>>
>> I would suggest modifying the commit policy disallowing these kind of
>> things, saying the two Ack's must be from two developers not
>> affiliated with the same company.
>> (actually we might even want to take it further and saying that global
>> changes that affect everyone would require ACK's from developers from
>> more than one distro).
>>
>> Question to the TSC: should the SOC_FAMILY patch be reverted and put
>> on hold until there is agreement whether or not we want this?
>
> Frans,
>
> As the person who submitted this change I'd like to ask that it not be reverted.  I am using it currently so that when defining COMPATIBLE_MACHINE for recipes targeted at OMAP3 devices I do not have to keep a rolling list of all the various OMAP3 devices.  This can quickly become:
>
> COMPATIBLE_MACHINE = "dm37x-evm|am37x-evm|omap3evm|am3517-evm|beagleboard|......"
>
> Instead by allowing SOC_FAMILY to be used as a COMPATIBLE_MACHINE this can be handled cleanly with:
>
> COMPATIBLE_MACHINE = "omap3"
>
> Please consider this use case.  I would much prefer if your fix was put into the base.bbclass than if this were removed.
>

Chase,

I understand your use case.
The major concern I see is the overlap with MACHINE_CLASS.
Anyway, I have no real objections against SOC_FAMILY.
The only observation is that it has been introduced a little bit
hastily without the community reaching consensus.

Frans.

PS: thanks for the ack that arrived while I was typing this
PPS: wrt COMPATIBLE_MACHINE: I think it is possible to use wildcards,
so *if* the machines with omap3 get that as a prefix it could also be
used
(and yes: this is just an alternate option, and I see the con's of it)

Re: [PATCH] base.bbclass: fix soc-family test

[Denys Dmytriyenko]

On Thu, Sep 09, 2010 at 03:16:54PM +0200, Frans Meulenbroeks wrote:
> 2010/9/9 Maupin, Chase <chase.maupin@ti.com>:
> 
> >>
> >> I'm not too sure on the usefulness of it either, but there is some
> >> breakage so we should either revert the patch or fix it.
> >>
> >> Actually the SOC_FAMILY got pushed before the review was concluded.
> >> Technically it got the ack's but the discussion was still ongoing when
> >> this was pushed.
> >> What also makes it fishy is that all acks are from the same company
> >> (which is the same one as the person who submitted the patch):
> >>
> >> Signed-off-by: Chase Maupin <chase.maupin@ti.com>
> >> Acked-by: Denys Dmytriyenko <denys@ti.com>
> >> Acked-by: Koen Kooi <k-kooi@ti.com>
> >> Signed-off-by: Koen Kooi <koen@openembedded.org>
> >>
> >> I would suggest modifying the commit policy disallowing these kind of
> >> things, saying the two Ack's must be from two developers not
> >> affiliated with the same company.
> >> (actually we might even want to take it further and saying that global
> >> changes that affect everyone would require ACK's from developers from
> >> more than one distro).
> >>
> >> Question to the TSC: should the SOC_FAMILY patch be reverted and put
> >> on hold until there is agreement whether or not we want this?
> >
> > Frans,
> >
> > As the person who submitted this change I'd like to ask that it not be reverted.  I am using it currently so that when defining COMPATIBLE_MACHINE for recipes targeted at OMAP3 devices I do not have to keep a rolling list of all the various OMAP3 devices.  This can quickly become:
> >
> > COMPATIBLE_MACHINE = "dm37x-evm|am37x-evm|omap3evm|am3517-evm|beagleboard|......"
> >
> > Instead by allowing SOC_FAMILY to be used as a COMPATIBLE_MACHINE this can be handled cleanly with:
> >
> > COMPATIBLE_MACHINE = "omap3"
> >
> > Please consider this use case.  I would much prefer if your fix was put into the base.bbclass than if this were removed.
> >
> 
> Chase,
> 
> I understand your use case.
> The major concern I see is the overlap with MACHINE_CLASS.
> Anyway, I have no real objections against SOC_FAMILY.
> The only observation is that it has been introduced a little bit
> hastily without the community reaching consensus.
> 
> Frans.
> 
> PS: thanks for the ack that arrived while I was typing this
> PPS: wrt COMPATIBLE_MACHINE: I think it is possible to use wildcards,
> so *if* the machines with omap3 get that as a prefix it could also be
> used
> (and yes: this is just an alternate option, and I see the con's of it)

Frans,

I'd like us to find a compromise and a solution that satisfies everybody. 

Unfortunately, the original discussion about SOC_FAMILY vs. MACHINE_CLASS 
never came to any fruition - Graeme explained his motives behind MACHINE_CLASS 
and said that it is now deprecated.

It was also mentioned, while SOC_FAMILY is slightly newer than MACHINE_CLASS, 
the feature itself is over a year old and used quite extensively, although 
limited mainly to recipes/ti location...

And if technically SOC_FAMILY may be similar to MACHINE_CLASS, logically they 
try to solve grouping problem from different direction, which was also 
explained.

After that the original discussion was stalled and there were no strong 
opinions one way or another. Based on that, Chase's change was pushed.

I see and understand Phil's position - if that's strong enough, we can 
re-consider.

BTW, I was under impression, that the issue in COMPATIBLE_MACHINE was fixed 
soon after it was discovered. Sorry for not acknowledging your fix earlier.

Anyway, just wanted to let you know that we are not trying to undermine 
anybody in the community and we, as a company, want to work close with the 
community, and be a good citizen. Hopefully, this incident will not be 
considered as an indication of otherwise. Thank you for your understanding.

-- 
Denys

Re: [PATCH] base.bbclass: fix soc-family test

[Phil Blundell]

On Fri, 2010-09-10 at 14:37 -0400, Denys Dmytriyenko wrote:
> I'd like us to find a compromise and a solution that satisfies everybody. 
> 
> Unfortunately, the original discussion about SOC_FAMILY vs. MACHINE_CLASS 
> never came to any fruition - Graeme explained his motives behind MACHINE_CLASS 
> and said that it is now deprecated.
> 
> It was also mentioned, while SOC_FAMILY is slightly newer than MACHINE_CLASS, 
> the feature itself is over a year old and used quite extensively, although 
> limited mainly to recipes/ti location...
> 
> And if technically SOC_FAMILY may be similar to MACHINE_CLASS, logically they 
> try to solve grouping problem from different direction, which was also 
> explained.
> 
> After that the original discussion was stalled and there were no strong 
> opinions one way or another. Based on that, Chase's change was pushed.
> 
> I see and understand Phil's position - if that's strong enough, we can 
> re-consider.

I take the point about MACHINE_CLASS and SOC_FAMILY being different in
intent.  However, I do feel that these are just two out of a whole
universe of possible machine groupings and I remain somewhat uneasy
about adding this sort of thing to base.bbclass: if we admit SOC_FAMILY
(or even MACHINE_CLASS) there then it seems like it will set an
undesirable precedent for the next guy who wants his favourite machine
grouping to be given the same treatment.  (The same thing applies to the
OVERRIDES patch that was posted recently and which I am not very fond of
either.)

How many recipes are there for which this is a big deal?  It's worth
remembering that the whole COMPATIBLE_MACHINE thing in base.bbclass is,
essentially, just syntactic sugar and there is nothing to stop you from
implementing whatever compatibility logic you want in your own .bb files
(or in an .inc, or a custom class).  If there are only a handful of
recipes for which gating on SOC_FAMILY is required then I would suggest
that you simply put the appropriate Python bits in those recipes.

p.

Re: [PATCH] base.bbclass: fix soc-family test

[Maupin, Chase]

> -----Original Message-----
> From: openembedded-devel-bounces@lists.openembedded.org
> [mailto:openembedded-devel-bounces@lists.openembedded.org] On Behalf Of
> Phil Blundell
> Sent: Friday, September 10, 2010 4:20 PM
> To: openembedded-devel@lists.openembedded.org
> Subject: Re: [oe] [PATCH] base.bbclass: fix soc-family test
> 
> On Fri, 2010-09-10 at 14:37 -0400, Denys Dmytriyenko wrote:
> > I'd like us to find a compromise and a solution that satisfies everybody.
> >
> > Unfortunately, the original discussion about SOC_FAMILY vs.
> MACHINE_CLASS
> > never came to any fruition - Graeme explained his motives behind
> MACHINE_CLASS
> > and said that it is now deprecated.
> >
> > It was also mentioned, while SOC_FAMILY is slightly newer than
> MACHINE_CLASS,
> > the feature itself is over a year old and used quite extensively,
> although
> > limited mainly to recipes/ti location...
> >
> > And if technically SOC_FAMILY may be similar to MACHINE_CLASS, logically
> they
> > try to solve grouping problem from different direction, which was also
> > explained.
> >
> > After that the original discussion was stalled and there were no strong
> > opinions one way or another. Based on that, Chase's change was pushed.
> >
> > I see and understand Phil's position - if that's strong enough, we can
> > re-consider.
> 
> I take the point about MACHINE_CLASS and SOC_FAMILY being different in
> intent.  However, I do feel that these are just two out of a whole
> universe of possible machine groupings and I remain somewhat uneasy
> about adding this sort of thing to base.bbclass: if we admit SOC_FAMILY
> (or even MACHINE_CLASS) there then it seems like it will set an
> undesirable precedent for the next guy who wants his favourite machine
> grouping to be given the same treatment.  (The same thing applies to the
> OVERRIDES patch that was posted recently and which I am not very fond of
> either.)
> 
> How many recipes are there for which this is a big deal?  It's worth
> remembering that the whole COMPATIBLE_MACHINE thing in base.bbclass is,
> essentially, just syntactic sugar and there is nothing to stop you from
> implementing whatever compatibility logic you want in your own .bb files
> (or in an .inc, or a custom class).  If there are only a handful of
> recipes for which gating on SOC_FAMILY is required then I would suggest
> that you simply put the appropriate Python bits in those recipes.

Phil,

First thanks for the response.  There a quite a few recipes that use this or plan to use it.  Kernel recipes, several of the recipes in the "recipes/ti" directory.  I guess as a person who actually uses this I have a bias here.  I hope you can understand though that this is variable found usefulness as an override and that is why it was extended to also work for COMPATIBLE_MACHINE.

> 
> p.
> 
> 
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel

Re: [PATCH] base.bbclass: fix soc-family test

[Phil Blundell]

On Fri, 2010-09-10 at 15:27 -0500, Maupin, Chase wrote:
> First thanks for the response.  There a quite a few recipes that use this or plan to use it.  Kernel recipes, several of the recipes in the "recipes/ti" directory.  I guess as a person who actually uses this I have a bias here.  I hope you can understand though that this is variable found usefulness as an override and that is why it was extended to also work for COMPATIBLE_MACHINE.

I certainly don't doubt that SOC_FAMILY is useful to you and that the
code in base.bbclass serves a purpose.  There are three things, though,
that I'm not yet entirely convinced about:

a) whether SOC_FAMILY is sufficiently useful to other (i.e. non-TI)
folks to merit inclusion in base.bbclass, and is going to remain that
way; and

b) whether the high-level goals of SOC_FAMILY cannot be met in some more
generic fashion; and

c) if the answer to both (a) and (b) is yes, whether this functionality
genuinely needs to be in base.bbclass and can't be implemented somewhere
more specific.

The main reason for my uneasiness about (a) is that, although it might
appear that the concept of a "SoC family" is quite tightly defined
today, at least within the TI universe, experience suggests that the
boundaries of any given family tend to become a bit blurred over time.
Soft cores and IP blocks have a habit of being reused in new designs
which suddenly differ in some important way from the original parts for
which they were created and, unless you have a very clear definition
upfront for what constitutes the "family", you can suddenly end up with
machines which appear to be half in and half out, or which want to be
members of two families simultaneously.

As far as (b) goes, I was mostly wondering whether you could do
something with MACHINE_FEATURES.  I think I would be generally receptive
to a patch which added a variable along the general lines of
REQUIRED_MACHINE_FEATURES = "omap-core ti-rfbi ..." and which would
cause SkipPackage to be raised if all the required things didn't appear
in MACHINE_FEATURES at runtime.  Alternatively, I wonder whether you can
do some more free-form grouping of machines within the
COMPATIBLE_MACHINE syntax which doesn't involve introducing a new
variable to base.bbclass.

Finally, regarding (c), I think the question does still stand as to
whether the recipes which have SOC_FAMILY dependencies couldn't just
inherit some sort of ti-compatibility-check.bbclass which would perform
the appropriate tests.  That would allow you to do whatever you wanted
without needing to trouble base.bbclass, and you could bash it any way
you liked without needing to get code review :-)

The thing about OVERRIDES is another issue again.  Adding a new thing to
the global OVERRIDES is definitely not to be done lightly: it introduces
new namespace hazards (i.e. inadvertently triggering the override
because your MACHINE or DISTRO happens to be named the same), and it
also introduces an extra processing overhead in every parse operation.
Again, I am not keen to see this happen unless it is obvious that the
same thing can't be done better in another way.

p.

Re: [PATCH] base.bbclass: fix soc-family test

[Michael 'Mickey' Lauer]

Am Freitag, den 10.09.2010, 14:37 -0400 schrieb Denys Dmytriyenko:
> Unfortunately, the original discussion about SOC_FAMILY vs. MACHINE_CLASS 
> never came to any fruition - Graeme explained his motives behind MACHINE_CLASS 
> and said that it is now deprecated.

Which applies to the work he did while he worked for Openmoko. I brought
back MACHINE_CLASS for other models (EZX, HTC, etc.) and it is in use
and its use will be enhanced - so, not deprecated.

-- 
:M:

Re: [PATCH] base.bbclass: fix soc-family test

[Chris Larson]

On Thu, Sep 9, 2010 at 3:11 AM, Frans Meulenbroeks <
fransmeulenbroeks@gmail.com> wrote:

> Signed-off-by: Chase Maupin <chase.maupin@ti.com>
> Acked-by: Denys Dmytriyenko <denys@ti.com>
> Acked-by: Koen Kooi <k-kooi@ti.com>
> Signed-off-by: Koen Kooi <koen@openembedded.org>
>
> I would suggest modifying the commit policy disallowing these kind of
> things, saying the two Ack's must be from two developers not
> affiliated with the same company.
>

I'd agree with this quite strongly.  I'm generally pretty careful to let
others review the changes from my coworkers, to avoid any bias.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Re: [PATCH] base.bbclass: fix soc-family test

[Philip Balister]

On 09/09/2010 10:06 AM, Chris Larson wrote:
> On Thu, Sep 9, 2010 at 3:11 AM, Frans Meulenbroeks<
> fransmeulenbroeks@gmail.com>  wrote:
>
>> Signed-off-by: Chase Maupin<chase.maupin@ti.com>
>> Acked-by: Denys Dmytriyenko<denys@ti.com>
>> Acked-by: Koen Kooi<k-kooi@ti.com>
>> Signed-off-by: Koen Kooi<koen@openembedded.org>
>>
>> I would suggest modifying the commit policy disallowing these kind of
>> things, saying the two Ack's must be from two developers not
>> affiliated with the same company.
>>
>
> I'd agree with this quite strongly.  I'm generally pretty careful to let
> others review the changes from my coworkers, to avoid any bias.

This would be a good topic for OEDEM.

For the record, I am really happy to see the .com people actively 
participating in the the OE project. I am also glad that they are 
pushing stuff into .dev and not keeping it in private repositories. This 
is a very good thing.

Philip

Re: [PATCH] base.bbclass: fix soc-family test

[Denys Dmytriyenko]

On Thu, Sep 09, 2010 at 10:16:11AM -0400, Philip Balister wrote:
> On 09/09/2010 10:06 AM, Chris Larson wrote:
>> On Thu, Sep 9, 2010 at 3:11 AM, Frans Meulenbroeks<
>> fransmeulenbroeks@gmail.com>  wrote:
>>
>>> Signed-off-by: Chase Maupin<chase.maupin@ti.com>
>>> Acked-by: Denys Dmytriyenko<denys@ti.com>
>>> Acked-by: Koen Kooi<k-kooi@ti.com>
>>> Signed-off-by: Koen Kooi<koen@openembedded.org>
>>>
>>> I would suggest modifying the commit policy disallowing these kind of
>>> things, saying the two Ack's must be from two developers not
>>> affiliated with the same company.
>>>
>>
>> I'd agree with this quite strongly.  I'm generally pretty careful to let
>> others review the changes from my coworkers, to avoid any bias.

Chris,

Not to point any fingers, but mishaps happen and numerous reverts would be a 
living proof of that in the repository... :) Nobody's perfect.

> This would be a good topic for OEDEM.
>
> For the record, I am really happy to see the .com people actively 
> participating in the the OE project. I am also glad that they are pushing 
> stuff into .dev and not keeping it in private repositories. This is a very 
> good thing.

As I just replied to Frans' post in this thread - as a company, we are trying 
to follow review procedures closely and give enough time for feedback on the 
patches. I personally would like to apologize for the misunderstanding and 
promise to do a better job in the future.

As an active OE e.V. member on the other hand, I'm all in favor of this 
modification to the commit policy, as long as there is time limit on review 
period and if there are no strong objections, it can be pushed even if only 
acked by the same company. Let's discuss it further and vote at OEDEM.

-- 
Denys

Re: [PATCH] base.bbclass: fix soc-family test

[Chris Larson]

On Fri, Sep 10, 2010 at 11:50 AM, Denys Dmytriyenko <denis@denix.org> wrote:

> >> I'd agree with this quite strongly.  I'm generally pretty careful to let
> >> others review the changes from my coworkers, to avoid any bias.
>
> Not to point any fingers, but mishaps happen and numerous reverts would be
> a
> living proof of that in the repository... :) Nobody's perfect.


Mishaps have nothing to do with this, nor does being perfect, or reverts.
 You're missing the point entirely here.  The point is, if the only people
who acked a patch are from the same company as the person who wrote it, they
can't be considered to have been entirely unbiased in their review of it.
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Re: [PATCH] base.bbclass: fix soc-family test

[Denys Dmytriyenko]

On Fri, Sep 10, 2010 at 11:55:13AM -0700, Chris Larson wrote:
> On Fri, Sep 10, 2010 at 11:50 AM, Denys Dmytriyenko <denis@denix.org> wrote:
> 
> > >> I'd agree with this quite strongly.  I'm generally pretty careful to let
> > >> others review the changes from my coworkers, to avoid any bias.
> >
> > Not to point any fingers, but mishaps happen and numerous reverts would be
> > a
> > living proof of that in the repository... :) Nobody's perfect.
> 
> 
> Mishaps have nothing to do with this, nor does being perfect, or reverts.
>  You're missing the point entirely here.  The point is, if the only people
> who acked a patch are from the same company as the person who wrote it, they
> can't be considered to have been entirely unbiased in their review of it.

Chris,

Didn't mean to offend you by that joke, sorry about that. And I didn't mean 
you specifically, as we all had our share of revert...

But in the second, important :) part of my message, which you chose to ignore 
and remove from the reply, I did acknowledge the problem and agreed to the 
proposed changes. But, as I also mentioned, there should be a failsafe 
mechanism to eventually accept changes, if nobody cared to comment or 
strongly object, after the review period is over.

-- 
Denys

Re: [PATCH] base.bbclass: fix soc-family test

[Tom Rini]

Denys Dmytriyenko wrote:
> On Thu, Sep 09, 2010 at 10:16:11AM -0400, Philip Balister wrote:
>> On 09/09/2010 10:06 AM, Chris Larson wrote:
>>> On Thu, Sep 9, 2010 at 3:11 AM, Frans Meulenbroeks<
>>> fransmeulenbroeks@gmail.com>  wrote:
>>>
>>>> Signed-off-by: Chase Maupin<chase.maupin@ti.com>
>>>> Acked-by: Denys Dmytriyenko<denys@ti.com>
>>>> Acked-by: Koen Kooi<k-kooi@ti.com>
>>>> Signed-off-by: Koen Kooi<koen@openembedded.org>
>>>>
>>>> I would suggest modifying the commit policy disallowing these kind of
>>>> things, saying the two Ack's must be from two developers not
>>>> affiliated with the same company.
>>>>
>>> I'd agree with this quite strongly.  I'm generally pretty careful to let
>>> others review the changes from my coworkers, to avoid any bias.
> 
> Chris,
> 
> Not to point any fingers, but mishaps happen and numerous reverts would be a 
> living proof of that in the repository... :) Nobody's perfect.
> 
>> This would be a good topic for OEDEM.
>>
>> For the record, I am really happy to see the .com people actively 
>> participating in the the OE project. I am also glad that they are pushing 
>> stuff into .dev and not keeping it in private repositories. This is a very 
>> good thing.
> 
> As I just replied to Frans' post in this thread - as a company, we are trying 
> to follow review procedures closely and give enough time for feedback on the 
> patches. I personally would like to apologize for the misunderstanding and 
> promise to do a better job in the future.

That it seems that you're doing Ack/SOB in-house first is a good thing, 
I think.

> As an active OE e.V. member on the other hand, I'm all in favor of this 
> modification to the commit policy, as long as there is time limit on review 
> period and if there are no strong objections, it can be pushed even if only 
> acked by the same company. Let's discuss it further and vote at OEDEM.

So long as there's exceptions for limited area of expertise places. 
There's both things like packaged-staging where Chris and I and RP have 
dealt with it the most, and then there's also TI recipes that's really 
you guys.

-- 
Tom Rini
Mentor Graphics Corporation

Re: [PATCH] base.bbclass: fix soc-family test

[Phil Blundell]

Just on a note of administrivia, everyone, please don't cross-post
between oe@ and tsc@.  The tsc@ reflector is intended to be the place to
submit specific issues which require attention/action by the TSC and, if
it ends up in the Cc: field for a long mailing list thread, its
usefulness is much reduced.  

I think it is safe to assume that all TSC members are either subscribed
to the oe@ list or know how to operate the archives, so you shouldn't
feel that it is necessary to copy tsc@ purely for information.

thanks

p.

Re: [PATCH] base.bbclass: fix soc-family test

[Maupin, Chase]

> -----Original Message-----
> From: openembedded-devel-bounces@lists.openembedded.org
> [mailto:openembedded-devel-bounces@lists.openembedded.org] On Behalf Of
> Frans Meulenbroeks
> Sent: Thursday, September 09, 2010 2:11 AM
> To: openembedded-devel@lists.openembedded.org
> Subject: Re: [oe] [PATCH] base.bbclass: fix soc-family test
> 
> 2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
> > see http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-
> September/023680.html
> >
> > Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> > ---
> >  classes/base.bbclass |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/classes/base.bbclass b/classes/base.bbclass
> > index 299e875..2c9ad89 100644
> > --- a/classes/base.bbclass
> > +++ b/classes/base.bbclass
> > @@ -337,7 +337,7 @@ python () {
> >             this_machine = bb.data.getVar('MACHINE', d, 1)
> >             if this_machine and not re.match(need_machine,
> this_machine):
> >                 this_soc_family = bb.data.getVar('SOC_FAMILY', d, 1)
> > -                if this_soc_family and not re.match(need_machine,
> this_soc_family):
> > +                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" % this_machine)
> >
> >         need_target = bb.data.getVar('COMPATIBLE_TARGET_SYS', d, 1)
> > --
> > 1.6.4.2
> >
> 
> Bump.
> 
> This is out for a week now without any feedback, although the patch is
> quite trivial.

Frans,

I'm pretty sure I already gave my feedback on this fix.  If not then I think it looks fine to me and thank you for finding and fixing this issue.

> 
> Actually I am quite disappointed by the developers who created and
> pushed the SOC_FAMILY patch.
> They were very eager to push this change in a day, without waiting for
> the discussion on it to conclude, but they seem to be not-so-willing
> to review a fix for a problem they caused (let alone resolve the
> problem). What is even somewhat irritating is that one of the people
> involved is often in the front seat when it comes to criticizing
> others if they make a mistake.
> 
> Frans.
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel

Re: [PATCH] base.bbclass: fix soc-family test

[Maupin, Chase]

> -----Original Message-----
> From: openembedded-devel-bounces@lists.openembedded.org
> [mailto:openembedded-devel-bounces@lists.openembedded.org] On Behalf Of
> Frans Meulenbroeks
> Sent: Thursday, September 09, 2010 2:11 AM
> To: openembedded-devel@lists.openembedded.org
> Subject: Re: [oe] [PATCH] base.bbclass: fix soc-family test
> 
> 2010/9/2 Frans Meulenbroeks <fransmeulenbroeks@gmail.com>:
> > see http://lists.linuxtogo.org/pipermail/openembedded-devel/2010-
> September/023680.html
> >
> > Signed-off-by: Frans Meulenbroeks <fransmeulenbroeks@gmail.com>
> > ---
> >  classes/base.bbclass |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/classes/base.bbclass b/classes/base.bbclass
> > index 299e875..2c9ad89 100644
> > --- a/classes/base.bbclass
> > +++ b/classes/base.bbclass
> > @@ -337,7 +337,7 @@ python () {
> >             this_machine = bb.data.getVar('MACHINE', d, 1)
> >             if this_machine and not re.match(need_machine,
> this_machine):
> >                 this_soc_family = bb.data.getVar('SOC_FAMILY', d, 1)
> > -                if this_soc_family and not re.match(need_machine,
> this_soc_family):
> > +                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" % this_machine)
> >
> >         need_target = bb.data.getVar('COMPATIBLE_TARGET_SYS', d, 1)
> > --
> > 1.6.4.2
> >

Acked-by: Chase Maupin <chase.maupin@ti.com>

> 
> Bump.
> 
> This is out for a week now without any feedback, although the patch is
> quite trivial.
> 
> Actually I am quite disappointed by the developers who created and
> pushed the SOC_FAMILY patch.
> They were very eager to push this change in a day, without waiting for
> the discussion on it to conclude, but they seem to be not-so-willing
> to review a fix for a problem they caused (let alone resolve the
> problem). What is even somewhat irritating is that one of the people
> involved is often in the front seat when it comes to criticizing
> others if they make a mistake.
> 
> Frans.
> 
> _______________________________________________
> Openembedded-devel mailing list
> Openembedded-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-devel