[PATCH] rm_work: keep do_fetch, do_write_srcrev stamps

[PATCH] rm_work: keep do_fetch, do_write_srcrev stamps

[Martin Jansa]

* otherwise do_fetch/do_write_srcrev/do_rmwork is reexecuted for each recipe included in image
  on every build

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/classes/rm_work.bbclass | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
index 1642af7..2d0eb1b 100644
--- a/meta/classes/rm_work.bbclass
+++ b/meta/classes/rm_work.bbclass
@@ -60,6 +60,14 @@ do_rm_work () {
                 i=dummy
                 break
                 ;;
+            *do_fetch*)
+                i=dummy
+                break
+                ;;
+            *do_write_srcrev*)
+                i=dummy
+                break
+                ;;
             *do_build*)
                 i=dummy
                 break
-- 
1.8.1.5

Re: [PATCH] rm_work: keep do_fetch, do_write_srcrev stamps

[Richard Purdie]

On Wed, 2013-04-03 at 18:35 +0200, Martin Jansa wrote:
> * otherwise do_fetch/do_write_srcrev/do_rmwork is reexecuted for each recipe included in image
>   on every build
> 
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/classes/rm_work.bbclass | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
> index 1642af7..2d0eb1b 100644
> --- a/meta/classes/rm_work.bbclass
> +++ b/meta/classes/rm_work.bbclass
> @@ -60,6 +60,14 @@ do_rm_work () {
>                  i=dummy
>                  break
>                  ;;
> +            *do_fetch*)
> +                i=dummy
> +                break
> +                ;;
> +            *do_write_srcrev*)
> +                i=dummy
> +                break
> +                ;;
>              *do_build*)
>                  i=dummy
>                  break

We cannot do this, it is not in keeping with the rest of the system and
will break things and set a dangerous precedent.

If something subsequently tries to do some operation assuming do_fetch
already ran, it would run again assuming the do_fetch data is there and
it may not be. Right now the way things are setup with do_fetch will
happen to work out ok in most cases with the above but its the wrong
message to send out. For the do_write_srcrev case, it also means that in
some cases the revision will be written out, in other cases it will not
be and I don't think the inconsistency between the two is correct.

The other ways I can see to solve this are:

a) put do_write_srcrev under sstate
b) change do_write_srcrev to a postfunc of do_fetch
c) change the "addtask write_srcrev after do_fetch before do_build" to
be before do_install.

The trouble with c) is that it will cause the sstate checksums to change
when enabling buildhistory (which happens anyway but is rather annoying
and we should be trying to fix that, not make it worse). b) is therefore
looking the more attractive option at this point.

Cheers,

Richard

Re: [PATCH] rm_work: keep do_fetch, do_write_srcrev stamps

[Martin Jansa]

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

On Thu, Apr 04, 2013 at 01:31:57PM +0100, Richard Purdie wrote:
> On Wed, 2013-04-03 at 18:35 +0200, Martin Jansa wrote:
> > * otherwise do_fetch/do_write_srcrev/do_rmwork is reexecuted for each recipe included in image
> >   on every build
> > 
> > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > ---
> >  meta/classes/rm_work.bbclass | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
> > index 1642af7..2d0eb1b 100644
> > --- a/meta/classes/rm_work.bbclass
> > +++ b/meta/classes/rm_work.bbclass
> > @@ -60,6 +60,14 @@ do_rm_work () {
> >                  i=dummy
> >                  break
> >                  ;;
> > +            *do_fetch*)
> > +                i=dummy
> > +                break
> > +                ;;
> > +            *do_write_srcrev*)
> > +                i=dummy
> > +                break
> > +                ;;
> >              *do_build*)
> >                  i=dummy
> >                  break
> 
> We cannot do this, it is not in keeping with the rest of the system and
> will break things and set a dangerous precedent.
> 
> If something subsequently tries to do some operation assuming do_fetch
> already ran, it would run again assuming the do_fetch data is there and
> it may not be. Right now the way things are setup with do_fetch will
> happen to work out ok in most cases with the above but its the wrong
> message to send out. For the do_write_srcrev case, it also means that in
> some cases the revision will be written out, in other cases it will not
> be and I don't think the inconsistency between the two is correct.

OK, I haven't seen any issues with current state (only do_unpack was
touching WORKDIR afaik, so do_fetch stamp wasn't "invalidated" by
WORKDIR removal) but maybe I just haven't found the right corner case.

> The other ways I can see to solve this are:
> 
> a) put do_write_srcrev under sstate
> b) change do_write_srcrev to a postfunc of do_fetch
> c) change the "addtask write_srcrev after do_fetch before do_build" to
> be before do_install.
> 
> The trouble with c) is that it will cause the sstate checksums to change
> when enabling buildhistory (which happens anyway but is rather annoying
> and we should be trying to fix that, not make it worse). b) is therefore
> looking the more attractive option at this point.

I like b) too, because I don't care about srcrev change if foo wasn't
fetched here (if all sstate archives for foo are reused from SSTATE_MIRROR)

But maybe some people will have different use-case for write_srcrev and
will expect to get complete set of SRCREVs from
openembedded-core/scripts/buildhistory-collect-srcrevs
when executed on any builder. But that's something which can be improved
in 1.5, fixing do_fetch reexecution with rm_work or with other tasks
covered from SSTATE_MIRROR would be nice in 1.4 (Or at least add option
to disable do_write_srcrev task completely).

Cheers,
-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]