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

[Martin Jansa <martin.jansa@gmail.com>]

[-- 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 --]