[CONSOLIDATED,PULL,012/113] base.bbclass: Allow buildstats to be optionally supplied

Submitted by Saul Wold on Jan. 3, 2012, 6:18 a.m.

Details

Message ID 09b1dc8bd886c8cd2a5d4085d8bb4b73ece1f5b0.1325571069.git.sgw@linux.intel.com
State New
Headers show

Commit Message

Saul Wold Jan. 3, 2012, 6:18 a.m.
From: Mark Hatle <mark.hatle@windriver.com>

Buildstats should be allowed to be optionally enabled.  It's
recommended that it be enabled via the USER_CLASSES setting.

Alternatively it could be enabled via the INHERIT_DISTRO or
similar mechanism.

Signed-off-by: Mark Hatle <mark.hatle@windriver.com>
---
 meta/classes/base.bbclass   |    1 -
 meta/conf/local.conf.sample |    3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index fbcaefb..e65a722 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -7,7 +7,6 @@  inherit mirrors
 inherit utils
 inherit utility-tasks
 inherit metadata_scm
-inherit buildstats
 inherit logging
 
 OE_IMPORTS += "os sys time oe.path oe.utils oe.data oe.packagegroup"
diff --git a/meta/conf/local.conf.sample b/meta/conf/local.conf.sample
index 73ab9f8..7d52d0e 100644
--- a/meta/conf/local.conf.sample
+++ b/meta/conf/local.conf.sample
@@ -134,12 +134,13 @@  EXTRA_IMAGE_FEATURES = "debug-tweaks"
 # The following is a list of additional classes to use when building images which
 # enable extra features. Some available options which can be included in this variable 
 # are:
+#   - 'buildstats' collect build statistics
 #   - 'image-mklibs' to reduce shared library files size for an image
 #   - 'image-prelink' in order to prelink the filesystem image
 #   - 'image-swab' to perform host system intrusion detection
 # NOTE: if listing mklibs & prelink both, then make sure mklibs is before prelink
 # NOTE: mklibs also needs to be explicitly enabled for a given image, see local.conf.extended
-USER_CLASSES ?= "image-mklibs image-prelink"
+USER_CLASSES ?= "buildstats image-mklibs image-prelink"
 
 #
 # Runtime testing of images

Comments

Phil Blundell Jan. 3, 2012, 12:14 p.m.
On Mon, 2012-01-02 at 22:18 -0800, Saul Wold wrote:
> From: Mark Hatle <mark.hatle@windriver.com>
> 
> Buildstats should be allowed to be optionally enabled.  It's
> recommended that it be enabled via the USER_CLASSES setting.
> 
> Alternatively it could be enabled via the INHERIT_DISTRO or
> similar mechanism.
> 
> Signed-off-by: Mark Hatle <mark.hatle@windriver.com>

I don't think the short summary of this patch gives a very clear
indication of what it's doing.  The terminology "optionally supplied"
makes it sound as though you're talking about some sort of add-on data
file which can be enabled.  Whereas, what the patch seems actually to be
doing is removing the unconditional inherit of buildstats (i.e. turning
it off for almost everyone who has it on today) and adding a suggestion
in local.conf.sample as to how it might be turned on again.

(That said, I do think the intent of this patch is a good one; it's been
a long-standing source of irritation to me that disabling buildstats is
so awkward at present.)

p.
Mark Hatle Jan. 3, 2012, 6:06 p.m.
On 1/3/12 6:14 AM, Phil Blundell wrote:
> On Mon, 2012-01-02 at 22:18 -0800, Saul Wold wrote:
>> From: Mark Hatle<mark.hatle@windriver.com>
>>
>> Buildstats should be allowed to be optionally enabled.  It's
>> recommended that it be enabled via the USER_CLASSES setting.
>>
>> Alternatively it could be enabled via the INHERIT_DISTRO or
>> similar mechanism.
>>
>> Signed-off-by: Mark Hatle<mark.hatle@windriver.com>
>
> I don't think the short summary of this patch gives a very clear
> indication of what it's doing.  The terminology "optionally supplied"
> makes it sound as though you're talking about some sort of add-on data
> file which can be enabled.  Whereas, what the patch seems actually to be
> doing is removing the unconditional inherit of buildstats (i.e. turning
> it off for almost everyone who has it on today) and adding a suggestion
> in local.conf.sample as to how it might be turned on again.

Perhaps enabled is a better word then "supplied" in this case?

I didn't comment that it was removing the unconditional inherit as I thought 
that was obvious..

Saul -- can you change the short summary or should I resend it?

> (That said, I do think the intent of this patch is a good one; it's been
> a long-standing source of irritation to me that disabling buildstats is
> so awkward at present.)

This was causing a problem for me as well, thus the solution.  Seemed simply 
enough and beneficial.

--Mark

> p.
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Saul Wold Jan. 4, 2012, 12:45 a.m.
On 01/03/2012 10:06 AM, Mark Hatle wrote:
> On 1/3/12 6:14 AM, Phil Blundell wrote:
>> On Mon, 2012-01-02 at 22:18 -0800, Saul Wold wrote:
>>> From: Mark Hatle<mark.hatle@windriver.com>
>>>
>>> Buildstats should be allowed to be optionally enabled. It's
>>> recommended that it be enabled via the USER_CLASSES setting.
>>>
>>> Alternatively it could be enabled via the INHERIT_DISTRO or
>>> similar mechanism.
>>>
>>> Signed-off-by: Mark Hatle<mark.hatle@windriver.com>
>>
>> I don't think the short summary of this patch gives a very clear
>> indication of what it's doing. The terminology "optionally supplied"
>> makes it sound as though you're talking about some sort of add-on data
>> file which can be enabled. Whereas, what the patch seems actually to be
>> doing is removing the unconditional inherit of buildstats (i.e. turning
>> it off for almost everyone who has it on today) and adding a suggestion
>> in local.conf.sample as to how it might be turned on again.
>
> Perhaps enabled is a better word then "supplied" in this case?
>
> I didn't comment that it was removing the unconditional inherit as I
> thought that was obvious..
>
> Saul -- can you change the short summary or should I resend it?
>
Already merged by the time this email came in!  Sorry.

Sau!

>> (That said, I do think the intent of this patch is a good one; it's been
>> a long-standing source of irritation to me that disabling buildstats is
>> so awkward at present.)
>
> This was causing a problem for me as well, thus the solution. Seemed
> simply enough and beneficial.
>
> --Mark
>
>> p.
>>
>>
>>
>> _______________________________________________
>> Openembedded-core mailing list
>> Openembedded-core@lists.openembedded.org
>> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>