Skip to contentSkip to conversation

rm_work: keep do_fetch, do_write_srcrev stamps

PATCH In review Martin Jansa 2 participants 3 messages Apr 3, 2013 – Apr 4, 2013

lore
Share & tools
lore

Martin Jansa

author
#0 lore raw

PATCH 1/1 rm_work: keep do_fetch, do_write_srcrev stamps

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

Signed-off-by Martin Jansa <[email protected]>
---
 meta/classes/rm_work.bbclass | 8 ++++++++
 1 file changed, 8 insertions(+)
meta/classes/rm_work.bbclass +8 -0
index 1642af7..2d0eb1b 100644
@@ -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
Signature · 1 line
1.8.1.5
#1 lore raw
Quoted 17 lines · 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 <[email protected]>
---
 meta/classes/rm_work.bbclass | 8 ++++++++
 1 file changed, 8 insertions(+)
meta/classes/rm_work.bbclass +1 -0
index 1642af7..2d0eb1b 100644
@@ -60,6 +60,14 @@ do_rm_work () {
                 i=dummy
                 break
                 ;;
+            *do_fetch*)

meta/classes/rm_work.bbclass do_rm_work () { · line 73

Quoted 10 lines
meta/classes/rm_work.bbclass +7 -0
+                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

author
#2 lore raw
Quoted 32 lines · Richard Purdie
Quoted 17 lines · 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 <[email protected]>
---
 meta/classes/rm_work.bbclass | 8 ++++++++
 1 file changed, 8 insertions(+)
meta/classes/rm_work.bbclass +1 -0
index 1642af7..2d0eb1b 100644
@@ -60,6 +60,14 @@ do_rm_work () {
                 i=dummy
                 break
                 ;;
+            *do_fetch*)

meta/classes/rm_work.bbclass do_rm_work () { · line 73

Quoted 10 lines
meta/classes/rm_work.bbclass +7 -0
+                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.

Quoted 7 lines

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.

Quoted 11 lines

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,

Signature · 1 line
Martin 'JaMa' Jansa     jabber: [email protected]