[PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Chris Larson]

Set IMAGE_KEEPROOTFS to something other than the empty string to keep
tmp/rootfs around after image creation, otherwise it will be removed, as it's
not a fully functional filesystem, and causes much confusion among users as a
result.

Signed-off-by: Chris Larson <clarson@mvista.com>
---
 classes/image.bbclass |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/classes/image.bbclass b/classes/image.bbclass
index f0cc3b3..974e734 100644
--- a/classes/image.bbclass
+++ b/classes/image.bbclass
@@ -133,6 +133,7 @@ fakeroot do_rootfs () {
 	${IMAGE_POSTPROCESS_COMMAND}
 	
 	${MACHINE_POSTPROCESS_COMMAND}
+	${@['rm -rf ${IMAGE_ROOTFS}', ''][bool(d.getVar("IMAGE_KEEPROOTFS", 1))]}
 }
 
 do_deploy_to[nostamp] = "1"
-- 
1.6.0

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Tom Rini]

On Wed, Jun 17, 2009 at 10:10:40AM -0700, Chris Larson wrote:

> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
> tmp/rootfs around after image creation, otherwise it will be removed, as it's
> not a fully functional filesystem, and causes much confusion among users as a
> result.
> 
> Signed-off-by: Chris Larson <clarson@mvista.com>

Acked-by: Tom Rini <trini@embeddedalley.com>

-- 
Tom Rini

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Marcin Juszkiewicz]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

Dnia środa, 17 czerwca 2009 o 19:10:40 Chris Larson napisał(a):
> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
> tmp/rootfs around after image creation, otherwise it will be removed,
> as it's not a fully functional filesystem, and causes much confusion
> among users as a result.
>
> Signed-off-by: Chris Larson <clarson@mvista.com>

Acked-by: Marcin Juszkiewicz <marcin@juszkiewicz.com.pl>

I am also tired of explaining to users why tmp/rootfs should not be used 
as nfsroot.

Regards, 
-- 
JID:      hrw@jabber.org
Website:  http://marcin.juszkiewicz.com.pl/
LinkedIn: http://www.linkedin.com/in/marcinjuszkiewicz

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 204 bytes --]

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Rolf Leggewie]

Chris Larson wrote:
> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
> tmp/rootfs around after image creation, otherwise it will be removed

We had something like this proposed a while ago by I think Lynn Lin. 
This is already a bit more evolved since it is conditional on a 
variable.  The proposal then was rejected on the grounds of breaking 
concurrent builds and I think this will likely be the case here, too.

comments?

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Rolf Leggewie]

Rolf Leggewie wrote:
> Chris Larson wrote:
>> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
>> tmp/rootfs around after image creation, otherwise it will be removed
> 
> We had something like this proposed a while ago by I think Lynn Lin. 

I revisited that thread and it was actually about something else.

http://thread.gmane.org/gmane.comp.handhelds.openembedded/24434

My earlier comment does not apply.

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Chris Larson]

On Wed, Jun 17, 2009 at 10:47 AM, Rolf
Leggewie<no2spam@nospam.arcornews.de> wrote:
> Rolf Leggewie wrote:
>>
>> Chris Larson wrote:
>>>
>>> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
>>> tmp/rootfs around after image creation, otherwise it will be removed
>>
>> We had something like this proposed a while ago by I think Lynn Lin.
>
> I revisited that thread and it was actually about something else.
>
> http://thread.gmane.org/gmane.comp.handhelds.openembedded/24434
>
> My earlier comment does not apply.
>


Pushed once it got acked a couple times :)  Feel free to revert if
there's a good reason to do s
o.
-- 
Chris Larson
clarson at kergoth dot com
clarson at mvista dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Software Engineer
MontaVista Software, Inc.

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Roman I Khimov]

On Среда 17 июня 2009 21:40:58 Rolf Leggewie wrote:
> Chris Larson wrote:
> > Set IMAGE_KEEPROOTFS to something other than the empty string to keep
> > tmp/rootfs around after image creation, otherwise it will be removed
>
> We had something like this proposed a while ago by I think Lynn Lin.
> This is already a bit more evolved since it is conditional on a
> variable.  The proposal then was rejected on the grounds of breaking
> concurrent builds and I think this will likely be the case here, too.
>
> comments?

I think that was on 'tmp.gz' deletion and this one actually removes 
IMAGE_ROOTFS, not tmp/rootfs, so it should be fine.

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Richard Purdie]

On Wed, 2009-06-17 at 10:10 -0700, Chris Larson wrote:
> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
> tmp/rootfs around after image creation, otherwise it will be removed, as it's
> not a fully functional filesystem, and causes much confusion among users as a
> result.
> 
> Signed-off-by: Chris Larson <clarson@mvista.com>
> ---
>  classes/image.bbclass |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/classes/image.bbclass b/classes/image.bbclass
> index f0cc3b3..974e734 100644
> --- a/classes/image.bbclass
> +++ b/classes/image.bbclass
> @@ -133,6 +133,7 @@ fakeroot do_rootfs () {
>  	${IMAGE_POSTPROCESS_COMMAND}
>  	
>  	${MACHINE_POSTPROCESS_COMMAND}
> +	${@['rm -rf ${IMAGE_ROOTFS}', ''][bool(d.getVar("IMAGE_KEEPROOTFS", 1))]}
>  }
>  
>  do_deploy_to[nostamp] = "1"

Something I've been thinking about for a while is moving rootfs into
${WORKDIR}/. That way you can have concurrent image generation for a
start and rm_work could be taught to clean it up...

Cheers,

Richard

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Chris Larson]

On Wed, Jun 17, 2009 at 11:01 AM, Richard Purdie<rpurdie@rpsys.net> wrote:
> On Wed, 2009-06-17 at 10:10 -0700, Chris Larson wrote:
>> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
>> tmp/rootfs around after image creation, otherwise it will be removed, as it's
>> not a fully functional filesystem, and causes much confusion among users as a
>> result.
>>
>> Signed-off-by: Chris Larson <clarson@mvista.com>
>> ---
>>  classes/image.bbclass |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/classes/image.bbclass b/classes/image.bbclass
>> index f0cc3b3..974e734 100644
>> --- a/classes/image.bbclass
>> +++ b/classes/image.bbclass
>> @@ -133,6 +133,7 @@ fakeroot do_rootfs () {
>>       ${IMAGE_POSTPROCESS_COMMAND}
>>
>>       ${MACHINE_POSTPROCESS_COMMAND}
>> +     ${@['rm -rf ${IMAGE_ROOTFS}', ''][bool(d.getVar("IMAGE_KEEPROOTFS", 1))]}
>>  }
>>
>>  do_deploy_to[nostamp] = "1"
>
> Something I've been thinking about for a while is moving rootfs into
> ${WORKDIR}/. That way you can have concurrent image generation for a
> start and rm_work could be taught to clean it up...

That's a better way of doing it, indeed.  In fact, someone suggested
it on irc, and I promptly went and forgot about it  :)
-- 
Chris Larson
clarson at kergoth dot com
clarson at mvista dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Software Engineer
MontaVista Software, Inc.

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Tom Rini]

On Wed, Jun 17, 2009 at 07:01:56PM +0100, Richard Purdie wrote:
> On Wed, 2009-06-17 at 10:10 -0700, Chris Larson wrote:
> > Set IMAGE_KEEPROOTFS to something other than the empty string to keep
> > tmp/rootfs around after image creation, otherwise it will be removed, as it's
> > not a fully functional filesystem, and causes much confusion among users as a
> > result.
> > 
> > Signed-off-by: Chris Larson <clarson@mvista.com>
> > ---
> >  classes/image.bbclass |    1 +
> >  1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/classes/image.bbclass b/classes/image.bbclass
> > index f0cc3b3..974e734 100644
> > --- a/classes/image.bbclass
> > +++ b/classes/image.bbclass
> > @@ -133,6 +133,7 @@ fakeroot do_rootfs () {
> >  	${IMAGE_POSTPROCESS_COMMAND}
> >  	
> >  	${MACHINE_POSTPROCESS_COMMAND}
> > +	${@['rm -rf ${IMAGE_ROOTFS}', ''][bool(d.getVar("IMAGE_KEEPROOTFS", 1))]}
> >  }
> >  
> >  do_deploy_to[nostamp] = "1"
> 
> Something I've been thinking about for a while is moving rootfs into
> ${WORKDIR}/. That way you can have concurrent image generation for a
> start and rm_work could be taught to clean it up...

Isn't the other part of that the need to serialize making the package
index files?

-- 
Tom Rini

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Richard Purdie]

On Wed, 2009-06-17 at 12:08 -0700, Tom Rini wrote:
> On Wed, Jun 17, 2009 at 07:01:56PM +0100, Richard Purdie wrote:
> > Something I've been thinking about for a while is moving rootfs into
> > ${WORKDIR}/. That way you can have concurrent image generation for a
> > start and rm_work could be taught to clean it up...
> 
> Isn't the other part of that the need to serialize making the package
> index files?

I thought we'd done that? If not we should, its not difficult as we have
locking functions in bitbake.

-- 
RP

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Leon Woestenberg]

Hello,

On Wed, Jun 17, 2009 at 7:10 PM, Chris Larson<clarson@mvista.com> wrote:
> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
>
+1 for the idea, but "something other than the empty string" is a very
bad user interface.


If users see:

IMAGE_KEEPROOTFS = "1"

in most cases they'll do:

IMAGE_KEEPROOTFS = "0"

instead of

#IMAGE_KEEPROOTFS = "1"


Do we have more of these?? It never occured to me before.

Regards
-- 
Leon

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Christopher Larson]

On Jun 17, 2009, at 12:07 PM, Leon Woestenberg wrote:

> Hello,
>
> On Wed, Jun 17, 2009 at 7:10 PM, Chris Larson<clarson@mvista.com>  
> wrote:
>> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
>>
> +1 for the idea, but "something other than the empty string" is a very
> bad user interface.
>
>
> If users see:
>
> IMAGE_KEEPROOTFS = "1"
>
> in most cases they'll do:
>
> IMAGE_KEEPROOTFS = "0"
>
> instead of
>
> #IMAGE_KEEPROOTFS = "1"
>
>
> Do we have more of these?? It never occured to me before.


We have this sort of thing all over the place.  It needs an audit.   
Some vars expect set/empty, some expect 0/1, some expect yes/no, some  
just have a *convention* of yes/no.. afaik, anyway.
-- 
Chris Larson
Software Engineer
MontaVista Software, Inc.
Email: clarson AT mvista DOT com
Email: clarson AT kergoth DOT com

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Phil Blundell]

On Wed, 2009-06-17 at 21:07 +0200, Leon Woestenberg wrote:
> Hello,
> If users see:
> 
> IMAGE_KEEPROOTFS = "1"
> 
> in most cases they'll do:
> 
> IMAGE_KEEPROOTFS = "0"
> 
> instead of
> 
> #IMAGE_KEEPROOTFS = "1"
> 
> 
> Do we have more of these?? It never occured to me before.

Yeah, I made exactly this mistake with OE_STRICT_CHECKSUMS last week.
Luckily, Mickey was on hand to put me straight, otherwise it would have
been a frustrating experience.

p.

Re: [PATCH] image.bbclass: remove tmp/rootfs after image creation unless you set a variable

[Chris Larson]

On Wed, Jun 17, 2009 at 12:07 PM, Leon
Woestenberg<leon.woestenberg@gmail.com> wrote:
> Hello,
>
> On Wed, Jun 17, 2009 at 7:10 PM, Chris Larson<clarson@mvista.com> wrote:
>> Set IMAGE_KEEPROOTFS to something other than the empty string to keep
>>
> +1 for the idea, but "something other than the empty string" is a very
> bad user interface.
>
>
> If users see:
>
> IMAGE_KEEPROOTFS = "1"
>
> in most cases they'll do:
>
> IMAGE_KEEPROOTFS = "0"
>
> instead of
>
> #IMAGE_KEEPROOTFS = "1"
>
>
> Do we have more of these?? It never occured to me before.

Over the past day or so I've been thinking about putting together a
proof of concept variable type checking .inc.  Something like:

IMAGE_KEEPROOTFS ?= "0"
IMAGE_KEEPROOTFS[type] = "bool"

Then it would freak if it's not set to something valid for that type.
It wouldn't be hard to throw such a thing together.
-- 
Chris Larson
clarson at kergoth dot com
clarson at mvista dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Software Engineer
MontaVista Software, Inc.