[PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Phil Blundell]

There doesn't appear to be any compelling reason for these tasks to be nostamp
and having them re-run on every build can be irritating (for example, when the
image is an initramfs which your kernel image depends on).

Signed-off-by: Phil Blundell <philb@gnu.org>
---
 meta/classes/image.bbclass |    2 --
 1 file changed, 2 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index ffb372a..667d03d 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -168,11 +168,9 @@ LINGUAS_INSTALL ?= "${@" ".join(map(lambda s: "locale-base-%s" % s, d.getVar('IM
 
 PSEUDO_PASSWD = "${IMAGE_ROOTFS}"
 
-do_rootfs[nostamp] = "1"
 do_rootfs[dirs] = "${TOPDIR} ${WORKDIR}/intercept_scripts"
 do_rootfs[lockfiles] += "${IMAGE_ROOTFS}.lock"
 do_rootfs[cleandirs] += "${S} ${WORKDIR}/intercept_scripts"
-do_build[nostamp] = "1"
 
 # Must call real_do_rootfs() from inside here, rather than as a separate
 # task, so that we have a single fakeroot context for the whole process.
-- 
1.7.10.4

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Paul Eggleton]

On Friday 26 April 2013 12:30:38 Phil Blundell wrote:
> There doesn't appear to be any compelling reason for these tasks to be
> nostamp and having them re-run on every build can be irritating (for
> example, when the image is an initramfs which your kernel image depends
> on).
> 
> Signed-off-by: Phil Blundell <philb@gnu.org>
> ---
>  meta/classes/image.bbclass |    2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index ffb372a..667d03d 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -168,11 +168,9 @@ LINGUAS_INSTALL ?= "${@" ".join(map(lambda s:
> "locale-base-%s" % s, d.getVar('IM
> 
>  PSEUDO_PASSWD = "${IMAGE_ROOTFS}"
> 
> -do_rootfs[nostamp] = "1"
>  do_rootfs[dirs] = "${TOPDIR} ${WORKDIR}/intercept_scripts"
>  do_rootfs[lockfiles] += "${IMAGE_ROOTFS}.lock"
>  do_rootfs[cleandirs] += "${S} ${WORKDIR}/intercept_scripts"
> -do_build[nostamp] = "1"
> 
>  # Must call real_do_rootfs() from inside here, rather than as a separate
>  # task, so that we have a single fakeroot context for the whole process.

I have to say I'm not in favour of this. AFAIK these tasks have always been 
nostamp, and I'm not sure making do_build is going to help for the case you 
cite because the dependency on INITRD_IMAGE is on do_rootfs.

If you're concerned about your initramfs image rebuilding when building the 
main image, what happens if you specify do_rootfs[nostamp] = "0" ?

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Richard Purdie]

On Sat, 2013-04-27 at 09:34 +0100, Paul Eggleton wrote:
> On Friday 26 April 2013 12:30:38 Phil Blundell wrote:
> > There doesn't appear to be any compelling reason for these tasks to be
> > nostamp and having them re-run on every build can be irritating (for
> > example, when the image is an initramfs which your kernel image depends
> > on).
> > 
> > Signed-off-by: Phil Blundell <philb@gnu.org>
> > ---
> >  meta/classes/image.bbclass |    2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index ffb372a..667d03d 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -168,11 +168,9 @@ LINGUAS_INSTALL ?= "${@" ".join(map(lambda s:
> > "locale-base-%s" % s, d.getVar('IM
> > 
> >  PSEUDO_PASSWD = "${IMAGE_ROOTFS}"
> > 
> > -do_rootfs[nostamp] = "1"
> >  do_rootfs[dirs] = "${TOPDIR} ${WORKDIR}/intercept_scripts"
> >  do_rootfs[lockfiles] += "${IMAGE_ROOTFS}.lock"
> >  do_rootfs[cleandirs] += "${S} ${WORKDIR}/intercept_scripts"
> > -do_build[nostamp] = "1"
> > 
> >  # Must call real_do_rootfs() from inside here, rather than as a separate
> >  # task, so that we have a single fakeroot context for the whole process.
> 
> I have to say I'm not in favour of this. AFAIK these tasks have always been 
> nostamp, and I'm not sure making do_build is going to help for the case you 
> cite because the dependency on INITRD_IMAGE is on do_rootfs.
> 
> If you're concerned about your initramfs image rebuilding when building the 
> main image, what happens if you specify do_rootfs[nostamp] = "0" ?

I think this deserves some further thought. It is a pretty major change
however it is something I've wondered about for a while independently of
Phil, I've just never proposed a patch.

It might help to think about this in an alternative way. Before we has
the sstate checksums, it was near impossible to know when the inputs to
an image had changed and when they had not. Now with the sstate
checksums, we can know when any given input has changed and if it has
changed, the sstate checksum changes and the task re-runs.

You can therefore make the case that these "nostamp" tasks are a legacy
of the former world and that now, there is no compelling reason to have
them as nostamp. If you particularly want to force them to run, we do
have the -f option and its tainting works here just as well as it does
anywhere else.

If the rootfs nostamp is removed, there is no reason to have build as a
nostamp either, they were there as a pair.

Further thoughts?

Cheers,

Richard

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Koen Kooi]

Op 27 apr. 2013, om 11:08 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:

> On Sat, 2013-04-27 at 09:34 +0100, Paul Eggleton wrote:
>> On Friday 26 April 2013 12:30:38 Phil Blundell wrote:
>>> There doesn't appear to be any compelling reason for these tasks to be
>>> nostamp and having them re-run on every build can be irritating (for
>>> example, when the image is an initramfs which your kernel image depends
>>> on).
>>> 
>>> Signed-off-by: Phil Blundell <philb@gnu.org>
>>> ---
>>> meta/classes/image.bbclass |    2 --
>>> 1 file changed, 2 deletions(-)
>>> 
>>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>>> index ffb372a..667d03d 100644
>>> --- a/meta/classes/image.bbclass
>>> +++ b/meta/classes/image.bbclass
>>> @@ -168,11 +168,9 @@ LINGUAS_INSTALL ?= "${@" ".join(map(lambda s:
>>> "locale-base-%s" % s, d.getVar('IM
>>> 
>>> PSEUDO_PASSWD = "${IMAGE_ROOTFS}"
>>> 
>>> -do_rootfs[nostamp] = "1"
>>> do_rootfs[dirs] = "${TOPDIR} ${WORKDIR}/intercept_scripts"
>>> do_rootfs[lockfiles] += "${IMAGE_ROOTFS}.lock"
>>> do_rootfs[cleandirs] += "${S} ${WORKDIR}/intercept_scripts"
>>> -do_build[nostamp] = "1"
>>> 
>>> # Must call real_do_rootfs() from inside here, rather than as a separate
>>> # task, so that we have a single fakeroot context for the whole process.
>> 
>> I have to say I'm not in favour of this. AFAIK these tasks have always been 
>> nostamp, and I'm not sure making do_build is going to help for the case you 
>> cite because the dependency on INITRD_IMAGE is on do_rootfs.
>> 
>> If you're concerned about your initramfs image rebuilding when building the 
>> main image, what happens if you specify do_rootfs[nostamp] = "0" ?
> 
> I think this deserves some further thought. It is a pretty major change
> however it is something I've wondered about for a while independently of
> Phil, I've just never proposed a patch.
> 
> It might help to think about this in an alternative way. Before we has
> the sstate checksums, it was near impossible to know when the inputs to
> an image had changed and when they had not. Now with the sstate
> checksums, we can know when any given input has changed and if it has
> changed, the sstate checksum changes and the task re-runs.
> 
> You can therefore make the case that these "nostamp" tasks are a legacy
> of the former world and that now, there is no compelling reason to have
> them as nostamp. If you particularly want to force them to run, we do
> have the -f option and its tainting works here just as well as it does
> anywhere else.
> 
> If the rootfs nostamp is removed, there is no reason to have build as a
> nostamp either, they were there as a pair.
> 
> Further thoughts?

You summed up my thoughts on this quite nicely :)

regards,

Koen

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Paul Eggleton]

On Saturday 27 April 2013 10:08:54 Richard Purdie wrote:
> On Sat, 2013-04-27 at 09:34 +0100, Paul Eggleton wrote:
> > On Friday 26 April 2013 12:30:38 Phil Blundell wrote:
> > > There doesn't appear to be any compelling reason for these tasks to be
> > > nostamp and having them re-run on every build can be irritating (for
> > > example, when the image is an initramfs which your kernel image depends
> > > on).
> > > 
> > > Signed-off-by: Phil Blundell <philb@gnu.org>
> > > ---
> > > 
> > >  meta/classes/image.bbclass |    2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > > index ffb372a..667d03d 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -168,11 +168,9 @@ LINGUAS_INSTALL ?= "${@" ".join(map(lambda s:
> > > "locale-base-%s" % s, d.getVar('IM
> > > 
> > >  PSEUDO_PASSWD = "${IMAGE_ROOTFS}"
> > > 
> > > -do_rootfs[nostamp] = "1"
> > > 
> > >  do_rootfs[dirs] = "${TOPDIR} ${WORKDIR}/intercept_scripts"
> > >  do_rootfs[lockfiles] += "${IMAGE_ROOTFS}.lock"
> > >  do_rootfs[cleandirs] += "${S} ${WORKDIR}/intercept_scripts"
> > > 
> > > -do_build[nostamp] = "1"
> > > 
> > >  # Must call real_do_rootfs() from inside here, rather than as a
> > >  separate
> > >  # task, so that we have a single fakeroot context for the whole
> > >  process.
> > 
> > I have to say I'm not in favour of this. AFAIK these tasks have always
> > been
> > nostamp, and I'm not sure making do_build is going to help for the case
> > you
> > cite because the dependency on INITRD_IMAGE is on do_rootfs.
> > 
> > If you're concerned about your initramfs image rebuilding when building
> > the
> > main image, what happens if you specify do_rootfs[nostamp] = "0" ?
> 
> I think this deserves some further thought. It is a pretty major change
> however it is something I've wondered about for a while independently of
> Phil, I've just never proposed a patch.
> 
> It might help to think about this in an alternative way. Before we has
> the sstate checksums, it was near impossible to know when the inputs to
> an image had changed and when they had not. Now with the sstate
> checksums, we can know when any given input has changed and if it has
> changed, the sstate checksum changes and the task re-runs.
> 
> You can therefore make the case that these "nostamp" tasks are a legacy
> of the former world and that now, there is no compelling reason to have
> them as nostamp. If you particularly want to force them to run, we do
> have the -f option and its tainting works here just as well as it does
> anywhere else.
> 
> If the rootfs nostamp is removed, there is no reason to have build as a
> nostamp either, they were there as a pair.

So upon further reflection, I'm guessing my objections were only really about 
changing the status quo, as well as a lot of my own testing involving re-
running the image creation step without really changing anything other than 
the code that contributes to do_rootfs; it could be argued that my use case 
would be equally served by me setting nostamp locally or using -f, and 
everyone else having the benefit of not rebuilding the image when not needed.

Given that this is a departure from previously established behaviour, I do 
think we need to give a more of an explanation in the commit message than the 
proposed patch however; something that summarises Richard's explanation above 
would be informative.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Richard Purdie]

On Mon, 2013-04-29 at 15:45 +0100, Paul Eggleton wrote:
> So upon further reflection, I'm guessing my objections were only really about 
> changing the status quo, as well as a lot of my own testing involving re-
> running the image creation step without really changing anything other than 
> the code that contributes to do_rootfs; it could be argued that my use case 
> would be equally served by me setting nostamp locally or using -f, and 
> everyone else having the benefit of not rebuilding the image when not needed.

Remember as well that if you change the rootfs code, that will change
the rootfs stamp and the image will get regenerated. So it should even
work for your use case, if it doesn't, we have bigger dependencies
issues which we should fix which would be the real problems.

> Given that this is a departure from previously established behaviour, I do 
> think we need to give a more of an explanation in the commit message than the 
> proposed patch however; something that summarises Richard's explanation above 
> would be informative.

Agreed, Phil's commit message missed out a few important points.

Cheers,

Richard

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Phil Blundell]

On Mon, 2013-04-29 at 15:54 +0100, Richard Purdie wrote:
> On Mon, 2013-04-29 at 15:45 +0100, Paul Eggleton wrote:
> > Given that this is a departure from previously established behaviour, I do 
> > think we need to give a more of an explanation in the commit message than the 
> > proposed patch however; something that summarises Richard's explanation above 
> > would be informative.
> 
> Agreed, Phil's commit message missed out a few important points.

Do you want me to re-send it with a different commit message?

p.

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Saul Wold]

On 04/29/2013 11:33 AM, Phil Blundell wrote:
> On Mon, 2013-04-29 at 15:54 +0100, Richard Purdie wrote:
>> On Mon, 2013-04-29 at 15:45 +0100, Paul Eggleton wrote:
>>> Given that this is a departure from previously established behaviour, I do
>>> think we need to give a more of an explanation in the commit message than the
>>> proposed patch however; something that summarises Richard's explanation above
>>> would be informative.
>>
>> Agreed, Phil's commit message missed out a few important points.
>
> Do you want me to re-send it with a different commit message?
>
Yes, please resend with updated commit message.

Sau!

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

Re: [PATCH] image.bbclass: Don't mark do_rootfs and do_build as nostamp

[Phil Blundell]

On Sat, 2013-04-27 at 09:34 +0100, Paul Eggleton wrote:
> On Friday 26 April 2013 12:30:38 Phil Blundell wrote:
> > -do_rootfs[nostamp] = "1"
> >  do_rootfs[dirs] = "${TOPDIR} ${WORKDIR}/intercept_scripts"
> >  do_rootfs[lockfiles] += "${IMAGE_ROOTFS}.lock"
> >  do_rootfs[cleandirs] += "${S} ${WORKDIR}/intercept_scripts"
> > -do_build[nostamp] = "1"
>
> I have to say I'm not in favour of this. AFAIK these tasks have always been 
> nostamp, and I'm not sure making do_build is going to help for the case you 
> cite because the dependency on INITRD_IMAGE is on do_rootfs.

You're right that they've always been this way but that alone doesn't
seem like a convincing argument for keeping it so.  I think the original
reason they were nostamp is that bitbake didn't originally have enough
dependency tracking to work out when the rootfs did actually need to be
regenerated, and the only choices were "always" or "never".  Nowadays I
don't think there are any situations where you'd want the rootfs to
rebuild that won't already be detected by the task dependency logic.

I've had this change in my tree for at least six months or so and I've
certainly never noticed any failure to rebuild the rootfs when I would
have wanted it to.  What's the specific scenario you're concerned about?

> If you're concerned about your initramfs image rebuilding when building the 
> main image, what happens if you specify do_rootfs[nostamp] = "0" ?

That probably would work, yeah.

p.