Patchwork [1/1] rm_work.bbclass: inhibit rm_work per recipe

login
mail settings
Submitter Qi.Chen@windriver.com
Date March 13, 2013, 7:01 a.m.
Message ID <ab888a7abe849f3170d5386b577f8a92e8ff5385.1363156578.git.Qi.Chen@windriver.com>
Download mbox | patch
Permalink /patch/46115/
State Accepted
Commit 6c930c3c06f2e698540626c87bd7f7f571df35ef
Headers show

Comments

Qi.Chen@windriver.com - March 13, 2013, 7:01 a.m.
From: Chen Qi <Qi.Chen@windriver.com>

Use RM_WORK_WHITELIST to inhibit rm_work per recipe. In this way,
one can use rm_work for the most of the recipes but still keep the
work area for the recipe(s) one is working on.

As an example, the following settings in local.conf will inhibit
rm_work for icu-native, icu and busybox.
    INHERIT += "rm_work"
    RM_WORK_WHITELIST += "icu-native icu busybox"

If we comment out the RM_WORK_WHITELIST line and do a rebuild, the
working area of these recipes will be cleaned up.

[YOCTO #3675]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/rm_work.bbclass |   13 +++++++++++++
 1 file changed, 13 insertions(+)
Paul Eggleton - March 26, 2013, 5:12 p.m.
On Wednesday 13 March 2013 15:01:33 Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> Use RM_WORK_WHITELIST to inhibit rm_work per recipe. In this way,
> one can use rm_work for the most of the recipes but still keep the
> work area for the recipe(s) one is working on.
> 
> As an example, the following settings in local.conf will inhibit
> rm_work for icu-native, icu and busybox.
>     INHERIT += "rm_work"
>     RM_WORK_WHITELIST += "icu-native icu busybox"
> 
> If we comment out the RM_WORK_WHITELIST line and do a rebuild, the
> working area of these recipes will be cleaned up.

This is a great feature, but I just looked at it and realised that the term 
"whitelist" isn't really correct - this is more of a blacklist.

The question is does it matter? If so we should probably change it now before 
it becomes too hard to change...

Cheers,
Paul
Ross Burton - March 26, 2013, 5:15 p.m.
On 26 March 2013 17:12, Paul Eggleton <paul.eggleton@linux.intel.com> wrote:
> This is a great feature, but I just looked at it and realised that the term
> "whitelist" isn't really correct - this is more of a blacklist.
>
> The question is does it matter? If so we should probably change it now before
> it becomes too hard to change...

A variable called WHITELIST that is actually a blacklist should be
changed asap before we make a release, imho.

Ross
Phil Blundell - March 26, 2013, 5:25 p.m.
On Tue, 2013-03-26 at 17:12 +0000, Paul Eggleton wrote:
> On Wednesday 13 March 2013 15:01:33 Qi.Chen@windriver.com wrote:
> > From: Chen Qi <Qi.Chen@windriver.com>
> > 
> > Use RM_WORK_WHITELIST to inhibit rm_work per recipe. In this way,
> > one can use rm_work for the most of the recipes but still keep the
> > work area for the recipe(s) one is working on.
> > 
> > As an example, the following settings in local.conf will inhibit
> > rm_work for icu-native, icu and busybox.
> >     INHERIT += "rm_work"
> >     RM_WORK_WHITELIST += "icu-native icu busybox"
> > 
> > If we comment out the RM_WORK_WHITELIST line and do a rebuild, the
> > working area of these recipes will be cleaned up.
> 
> This is a great feature, but I just looked at it and realised that the term 
> "whitelist" isn't really correct - this is more of a blacklist.

This doesn't seem (at the risk of invoking an unintended metaphor)
entirely black or white.  Maybe it should just be "RM_WORK_EXCEPTIONS"
or something.

Of course, you can get the same effect in your distro configuration by
saying:

RM_WORK = "rm_work"
RM_WORK_pn-icu-native = ""
INHERIT += "${RM_WORK}"

so I must admit to being slightly ambivalent about whether the extra
syntactic sugar is all that valuable.  And then again you can always use
rm_old_work instead. :-)

p.
Paul Eggleton - March 26, 2013, 5:52 p.m.
On Tuesday 26 March 2013 17:25:52 Phil Blundell wrote:
> This doesn't seem (at the risk of invoking an unintended metaphor)
> entirely black or white.  Maybe it should just be "RM_WORK_EXCEPTIONS"
> or something.

"exception" has another meaning to my mind. I've sent out a patch to change it 
to RM_WORK_EXCLUDE.

> Of course, you can get the same effect in your distro configuration by
> saying:
> 
> RM_WORK = "rm_work"
> RM_WORK_pn-icu-native = ""
> INHERIT += "${RM_WORK}"
> 
> so I must admit to being slightly ambivalent about whether the extra
> syntactic sugar is all that valuable.  

True, that works; I think having an explicit variable makes it easier to 
understand what's going on though and is a little harder to typo and have your 
work removed when you didn't want it to be ;)

> And then again you can always use rm_old_work instead. :-)

I'm sure this has come up before, but is rm_old_work something we ought to 
have in OE-Core?

Cheers,
Paul
Martin Jansa - March 26, 2013, 5:55 p.m.
On Tue, Mar 26, 2013 at 05:12:16PM +0000, Paul Eggleton wrote:
> On Wednesday 13 March 2013 15:01:33 Qi.Chen@windriver.com wrote:
> > From: Chen Qi <Qi.Chen@windriver.com>
> > 
> > Use RM_WORK_WHITELIST to inhibit rm_work per recipe. In this way,
> > one can use rm_work for the most of the recipes but still keep the
> > work area for the recipe(s) one is working on.
> > 
> > As an example, the following settings in local.conf will inhibit
> > rm_work for icu-native, icu and busybox.
> >     INHERIT += "rm_work"
> >     RM_WORK_WHITELIST += "icu-native icu busybox"
> > 
> > If we comment out the RM_WORK_WHITELIST line and do a rebuild, the
> > working area of these recipes will be cleaned up.
> 
> This is a great feature, but I just looked at it and realised that the term 
> "whitelist" isn't really correct - this is more of a blacklist.
> 
> The question is does it matter? If so we should probably change it now before 
> it becomes too hard to change...

I got similar question yesterday about BB_HASHBASE_WHITELIST:

'And why is it called "WHITELIST"? Shouldn't things that are excluded be
in a "BLACKLIST"?'

Maybe term WHITELIST isn't correct in both of them, at least they are
consistent as it is now :).
Paul Eggleton - March 26, 2013, 6:02 p.m.
On Tuesday 26 March 2013 18:55:14 Martin Jansa wrote:
> On Tue, Mar 26, 2013 at 05:12:16PM +0000, Paul Eggleton wrote:
> > On Wednesday 13 March 2013 15:01:33 Qi.Chen@windriver.com wrote:
> > > From: Chen Qi <Qi.Chen@windriver.com>
> > > 
> > > Use RM_WORK_WHITELIST to inhibit rm_work per recipe. In this way,
> > > one can use rm_work for the most of the recipes but still keep the
> > > work area for the recipe(s) one is working on.
> > > 
> > > As an example, the following settings in local.conf will inhibit
> > > rm_work for icu-native, icu and busybox.
> > > 
> > >     INHERIT += "rm_work"
> > >     RM_WORK_WHITELIST += "icu-native icu busybox"
> > > 
> > > If we comment out the RM_WORK_WHITELIST line and do a rebuild, the
> > > working area of these recipes will be cleaned up.
> > 
> > This is a great feature, but I just looked at it and realised that the
> > term
> > "whitelist" isn't really correct - this is more of a blacklist.
> > 
> > The question is does it matter? If so we should probably change it now
> > before it becomes too hard to change...
> 
> I got similar question yesterday about BB_HASHBASE_WHITELIST:
> 
> 'And why is it called "WHITELIST"? Shouldn't things that are excluded be
> in a "BLACKLIST"?'
> 
> Maybe term WHITELIST isn't correct in both of them, at least they are
> consistent as it is now :)

You may well be right... BB_HASHBASE_WHITELIST has been around for so long 
though that I don't think we could consider changing it.

Cheers,
Paul
Phil Blundell - April 9, 2013, 9:01 p.m.
On Tue, 2013-03-26 at 17:52 +0000, Paul Eggleton wrote:
> I'm sure this has come up before, but is rm_old_work something we ought to 
> have in OE-Core?

Obviously it's not for me to say, but I suspect probably not.  It
requires ${WORKDIR} to follow a particular pattern, which not everybody
will necessarily want, and I don't think you could really argue that it
represents "core" functionality either.

p.
Martin Jansa - April 9, 2013, 10:55 p.m.
On Tue, Apr 09, 2013 at 10:01:05PM +0100, Phil Blundell wrote:
> On Tue, 2013-03-26 at 17:52 +0000, Paul Eggleton wrote:
> > I'm sure this has come up before, but is rm_old_work something we ought to 
> > have in OE-Core?
> 
> Obviously it's not for me to say, but I suspect probably not.  It
> requires ${WORKDIR} to follow a particular pattern, which not everybody

That pattern is now default in oe-core:
commit 05075cf3138d1c61f5cf4fe0e1a4587acc00c692
Author: Richard Purdie <richard.purdie@linuxfoundation.org>
Date:   Fri Nov 16 15:35:53 2012 +0000

    bitbake.conf/sanity: Separate versions and PN stamp components into
separate directories for WORKDIR and STAMP


> will necessarily want, and I don't think you could really argue that it
> represents "core" functionality either.

I think that in some cases it's more useful then rm_work.

IMHO only missing part is good documentation (or better name) describing
rm_work/rm_work_old difference:

1) rm_work is much better for builds in tmpfs
2) rm_work_old is better then rm_work if you have enough space to keep
1 WORKDIR for each component you're building

Patch

diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
index d3be0be..54287bf 100644
--- a/meta/classes/rm_work.bbclass
+++ b/meta/classes/rm_work.bbclass
@@ -5,6 +5,11 @@ 
 #
 # INHERIT += "rm_work"
 #
+# To inhibit rm_work for some recipes, specify them in RM_WORK_WHITELIST.
+# For example, in conf/local.conf:
+#
+# RM_WORK_WHITELIST += "icu-native icu busybox"
+#
 
 # Use the completion scheduler by default when rm_work is active
 # to try and reduce disk usage
@@ -14,6 +19,14 @@  RMWORK_ORIG_TASK := "${BB_DEFAULT_TASK}"
 BB_DEFAULT_TASK = "rm_work_all"
 
 do_rm_work () {
+    # If the recipe name is in the RM_WORK_WHITELIST, skip the recipe.
+    for p in ${RM_WORK_WHITELIST}; do
+        if [ "$p" = "${PN}" ]; then
+            bbnote "rm_work: Skipping ${PN} since it is in RM_WORK_WHITELIST"
+            exit 0
+        fi
+    done
+
     cd ${WORKDIR}
     for dir in *
     do