[PATCH v2 0/3] rm_work enhancements

[PATCH v2 0/3] rm_work enhancements

[Patrick Ohly]

This is the OE-core side of the rm_work.bbclass enhancements. Depends
on the corresponding bitbake patch series. See the OE-core "rm_work +
pybootchart enhancements" mail thread for further information.

Changes since v1:
  - now based on the (tenative!) bb.event.RecipeTaskPreProcess instead
    of prioritized anonymous functions
  - no need to change the scheduler, the "completion" scheduler now
    has the enhanced implementation

Patrick Ohly (3):
  gcc-source.inc: cleanly disable do_rm_work
  rm_work_and_downloads.bbclass: more aggressively minimize disk usage
  rm_work.bbclass: clean up sooner

 meta/classes/rm_work.bbclass               | 31 ++++++++++++++--------
 meta/classes/rm_work_and_downloads.bbclass | 33 +++++++++++++++++++++++-
 meta/recipes-devtools/gcc/gcc-source.inc   |  2 +-
 3 files changed, 54 insertions(+), 12 deletions(-)
 create mode 100644 meta/classes/rm_work_and_downloads.bbclass

base-commit: acce512a0b85853b5acf2ef07e4163a3b4f33a98
-- 
git-series 0.9.1

[PATCH v2 1/3] gcc-source.inc: cleanly disable do_rm_work

[Patrick Ohly]

Using "deltask" assumes that do_rm_work has been added already, which
won't be the case anymore in the upcoming improved rm_work.bbclass,
because then an anonymous python method will add do_rm_work.

Setting RM_WORK_EXCLUDE works with the current and upcoming
rm_work.bbclass and is the API that is meant to be used for excluding
recipes from cleaning, so use that.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/recipes-devtools/gcc/gcc-source.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/recipes-devtools/gcc/gcc-source.inc b/meta/recipes-devtools/gcc/gcc-source.inc
index 49bde92..0d0edb5 100644
--- a/meta/recipes-devtools/gcc/gcc-source.inc
+++ b/meta/recipes-devtools/gcc/gcc-source.inc
@@ -3,7 +3,7 @@ deltask do_compile
 deltask do_install 
 deltask do_populate_sysroot
 deltask do_populate_lic 
-deltask do_rm_work
+RM_WORK_EXCLUDE += "${PN}"
 
 inherit nopackages
 
-- 
git-series 0.9.1

[PATCH v2 2/3] rm_work_and_downloads.bbclass: more aggressively minimize disk usage

[Patrick Ohly]

rm_work.bbclass never deletes downloaded files, even if they are not
going to be needed again during the
build. rm_work_and_downloads.bbclass is more aggressive in minimizing
the used disk space during a build, but has other disadvantages:
- sources required by different recipes need to be fetched once per
  recipe, not once per build
- incremental builds do not work reliably because sources get
  removed without ensuring that sources gets fetched again

That makes rm_work_and_downloads.bbclass useful for one-time builds in
a constrained environment (like a CI system), but not for general use.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/rm_work_and_downloads.bbclass | 33 +++++++++++++++++++++++-
 1 file changed, 33 insertions(+)
 create mode 100644 meta/classes/rm_work_and_downloads.bbclass

diff --git a/meta/classes/rm_work_and_downloads.bbclass b/meta/classes/rm_work_and_downloads.bbclass
new file mode 100644
index 0000000..7c00bea
--- /dev/null
+++ b/meta/classes/rm_work_and_downloads.bbclass
@@ -0,0 +1,33 @@
+# Author:       Patrick Ohly <patrick.ohly@intel.com>
+# Copyright:    Copyright (C) 2015 Intel Corporation
+#
+# This file is licensed under the MIT license, see COPYING.MIT in
+# this source distribution for the terms.
+
+# This class is used like rm_work:
+# INHERIT += "rm_work_and_downloads"
+#
+# In addition to removing local build directories of a recipe, it also
+# removes the downloaded source. This is achieved by making the DL_DIR
+# recipe-specific. While reducing disk usage, it increases network usage (for
+# example, compiling the same source for target and host implies downloading
+# the source twice).
+#
+# Because the "do_fetch" task does not get re-run after removing the downloaded
+# sources, this class is also not suitable for incremental builds.
+#
+# Where it works well is in well-connected build environments with limited
+# disk space (like TravisCI).
+
+inherit rm_work
+
+# This would ensure that the existing do_rm_work() removes the downloads,
+# but does not work because some recipes have a circular dependency between
+# WORKDIR and DL_DIR (via ${SRCPV}?).
+# DL_DIR = "${WORKDIR}/downloads"
+
+# Instead go up one level and remove ourself.
+DL_DIR = "${BASE_WORKDIR}/${MULTIMACH_TARGET_SYS}/${PN}/downloads"
+do_rm_work_append () {
+    rm -rf ${DL_DIR}
+}
-- 
git-series 0.9.1

[PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Patrick Ohly]

Having do_rm_work depend on do_build had one major disadvantage:
do_build depends on the do_build of other recipes, to ensure that
runtime dependencies also get built. The effect is that when work on a
recipe is complete and it could get cleaned up, do_rm_work still
doesn't run because it waits for those other recipes, thus leading to
more temporary disk space usage than really needed.

The right solution is to inject do_rm_work before do_build and after
all tasks of the recipe. Achieving that depends on the new bitbake
bb.event.RecipeTaskPreProcess and bb.build.preceedtask().

It can't just run in an anonymous function, because other anonymous
functions that run later may add more tasks. There's still such a
potential conflict when some future RecipeTaskPreProcess event handler
also wants to change task dependencies, but that's not a problem
now. Should it ever occur, the two handlers will have to know about
each other and cooperate to resolve the conflict.

Benchmarking (see "rm_work + pybootchart enhancements" on the OE-core
mailing list) showed that builds with the modified rm_work.bbclass
were both faster (albeit not by much) and required considerably less
disk space (14230MiB instead of 18740MiB for core-image-sato).
Interestingly enough, builds with rm_work.bbclass were also faster
than those without.

Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
---
 meta/classes/rm_work.bbclass | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
index 3516c7e..fda7bd6 100644
--- a/meta/classes/rm_work.bbclass
+++ b/meta/classes/rm_work.bbclass
@@ -18,9 +18,6 @@ BB_SCHEDULER ?= "completion"
 # Run the rm_work task in the idle scheduling class
 BB_TASK_IONICE_LEVEL_task-rm_work = "3.0"
 
-RMWORK_ORIG_TASK := "${BB_DEFAULT_TASK}"
-BB_DEFAULT_TASK = "rm_work_all"
-
 do_rm_work () {
     # If the recipe name is in the RM_WORK_EXCLUDE, skip the recipe.
     for p in ${RM_WORK_EXCLUDE}; do
@@ -97,13 +94,6 @@ do_rm_work () {
         rm -f $i
     done
 }
-addtask rm_work after do_${RMWORK_ORIG_TASK}
-
-do_rm_work_all () {
-    :
-}
-do_rm_work_all[recrdeptask] = "do_rm_work"
-addtask rm_work_all after do_rm_work
 
 do_populate_sdk[postfuncs] += "rm_work_populatesdk"
 rm_work_populatesdk () {
@@ -117,7 +107,13 @@ rm_work_rootfs () {
 }
 rm_work_rootfs[cleandirs] = "${WORKDIR}/rootfs"
 
-python () {
+# We have to add the do_rmwork task already now, because all tasks are
+# meant to be defined before the RecipeTaskPreProcess event triggers.
+# The inject_rm_work event handler then merely changes task dependencies.
+addtask do_rm_work
+addhandler inject_rm_work
+inject_rm_work[eventmask] = "bb.event.RecipeTaskPreProcess"
+python inject_rm_work() {
     if bb.data.inherits_class('kernel', d):
         d.appendVar("RM_WORK_EXCLUDE", ' ' + d.getVar("PN"))
     # If the recipe name is in the RM_WORK_EXCLUDE, skip the recipe.
@@ -126,4 +122,17 @@ python () {
     if pn in excludes:
         d.delVarFlag('rm_work_rootfs', 'cleandirs')
         d.delVarFlag('rm_work_populatesdk', 'cleandirs')
+    else:
+        # Inject do_rm_work into the tasks of the current recipe such that do_build
+        # depends on it and that it runs after all other tasks that block do_build,
+        # i.e. after all work on the current recipe is done. The reason for taking
+        # this approach instead of making do_rm_work depend on do_build is that
+        # do_build inherits additional runtime dependencies on
+        # other recipes and thus will typically run much later than completion of
+        # work in the recipe itself.
+        deps = bb.build.preceedtask('do_build', True, d)
+        if 'do_build' in deps:
+            deps.remove('do_build')
+        # In practice, addtask() here merely updates the dependencies.
+        bb.build.addtask('do_rm_work', 'do_build', ' '.join(deps), d)
 }
-- 
git-series 0.9.1

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Mike Crowe]

On Friday 13 January 2017 at 15:52:33 +0100, Patrick Ohly wrote:
> Having do_rm_work depend on do_build had one major disadvantage:
> do_build depends on the do_build of other recipes, to ensure that
> runtime dependencies also get built. The effect is that when work on a
> recipe is complete and it could get cleaned up, do_rm_work still
> doesn't run because it waits for those other recipes, thus leading to
> more temporary disk space usage than really needed.
> 
> The right solution is to inject do_rm_work before do_build and after
> all tasks of the recipe. Achieving that depends on the new bitbake
> bb.event.RecipeTaskPreProcess and bb.build.preceedtask().

We've run into trouble with this change. We have a number of custom
ancillary tasks that are used to generate source release files and run
package tests. No other tasks (including do_build) depend on these tasks
since they are run explicitly when required using bitbake -c; either
directly or via a recrdeptask.

Running a single task continues to work correctly - presumably this is
because the do_build task is not being run, so its dependencies (including
rm_work) aren't run either.

Running via the recrdeptask fails. This is because for any particular
recipe we end up depending on both do_build and the source release tasks.
There's nothing to stop do_rm_work running before (or even during!) one of
the source release tasks.

> diff --git a/meta/classes/rm_work.bbclass b/meta/classes/rm_work.bbclass
> index 3516c7e..fda7bd6 100644
> --- a/meta/classes/rm_work.bbclass
> +++ b/meta/classes/rm_work.bbclass
> @@ -117,7 +107,13 @@ rm_work_rootfs () {
>  }
>  rm_work_rootfs[cleandirs] = "${WORKDIR}/rootfs"
>  
> -python () {
> +# We have to add the do_rmwork task already now, because all tasks are
> +# meant to be defined before the RecipeTaskPreProcess event triggers.
> +# The inject_rm_work event handler then merely changes task dependencies.
> +addtask do_rm_work
> +addhandler inject_rm_work
> +inject_rm_work[eventmask] = "bb.event.RecipeTaskPreProcess"
> +python inject_rm_work() {
>      if bb.data.inherits_class('kernel', d):
>          d.appendVar("RM_WORK_EXCLUDE", ' ' + d.getVar("PN"))
>      # If the recipe name is in the RM_WORK_EXCLUDE, skip the recipe.
> @@ -126,4 +122,17 @@ python () {
>      if pn in excludes:
>          d.delVarFlag('rm_work_rootfs', 'cleandirs')
>          d.delVarFlag('rm_work_populatesdk', 'cleandirs')
> +    else:
> +        # Inject do_rm_work into the tasks of the current recipe such that do_build
> +        # depends on it and that it runs after all other tasks that block do_build,
> +        # i.e. after all work on the current recipe is done. The reason for taking
> +        # this approach instead of making do_rm_work depend on do_build is that
> +        # do_build inherits additional runtime dependencies on
> +        # other recipes and thus will typically run much later than completion of
> +        # work in the recipe itself.
> +        deps = bb.build.preceedtask('do_build', True, d)
> +        if 'do_build' in deps:
> +            deps.remove('do_build')
> +        # In practice, addtask() here merely updates the dependencies.
> +        bb.build.addtask('do_rm_work', 'do_build', ' '.join(deps), d)
>  }

It seems that we need to ensure that do_rm_work also needs to depend on our
ancillary tasks too, but only if they are being built. I'm unsure how this
can be done though. :(

For the time being, I've reverted this patch in our tree and it seems to
have resolved the problem. I'd be very interested in knowing what the
correct solution would be.

Mike.

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Patrick Ohly]

On Wed, 2017-02-08 at 11:50 +0000, Mike Crowe wrote:
> On Friday 13 January 2017 at 15:52:33 +0100, Patrick Ohly wrote:
> > Having do_rm_work depend on do_build had one major disadvantage:
> > do_build depends on the do_build of other recipes, to ensure that
> > runtime dependencies also get built. The effect is that when work on a
> > recipe is complete and it could get cleaned up, do_rm_work still
> > doesn't run because it waits for those other recipes, thus leading to
> > more temporary disk space usage than really needed.
> > 
> > The right solution is to inject do_rm_work before do_build and after
> > all tasks of the recipe. Achieving that depends on the new bitbake
> > bb.event.RecipeTaskPreProcess and bb.build.preceedtask().
> 
> We've run into trouble with this change. We have a number of custom
> ancillary tasks that are used to generate source release files and run
> package tests. No other tasks (including do_build) depend on these tasks
> since they are run explicitly when required using bitbake -c; either
> directly or via a recrdeptask.
> 
> Running a single task continues to work correctly - presumably this is
> because the do_build task is not being run, so its dependencies (including
> rm_work) aren't run either.
> 
> Running via the recrdeptask fails. This is because for any particular
> recipe we end up depending on both do_build and the source release tasks.
> There's nothing to stop do_rm_work running before (or even during!) one of
> the source release tasks.

Can you show how you use recrdeptask and how you call bitbake to trigger
those extra tasks, just for my understanding?

I suppose it worked before because your tasks could depend on do_build
without triggering do_rm_work, while now that is included.

> It seems that we need to ensure that do_rm_work also needs to depend on our
> ancillary tasks too, but only if they are being built. I'm unsure how this
> can be done though. :(

How do you determine whether the tasks need to run? Does it depend on
how bitbake is invoked or does it depend on specific properties of the
recipe?

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Mike Crowe]

On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> On Wed, 2017-02-08 at 11:50 +0000, Mike Crowe wrote:
> > On Friday 13 January 2017 at 15:52:33 +0100, Patrick Ohly wrote:
> > > The right solution is to inject do_rm_work before do_build and after
> > > all tasks of the recipe. Achieving that depends on the new bitbake
> > > bb.event.RecipeTaskPreProcess and bb.build.preceedtask().
> > 
> > We've run into trouble with this change. We have a number of custom
> > ancillary tasks that are used to generate source release files and run
> > package tests. No other tasks (including do_build) depend on these tasks
> > since they are run explicitly when required using bitbake -c; either
> > directly or via a recrdeptask.
> > 
> > Running a single task continues to work correctly - presumably this is
> > because the do_build task is not being run, so its dependencies (including
> > rm_work) aren't run either.
> > 
> > Running via the recrdeptask fails. This is because for any particular
> > recipe we end up depending on both do_build and the source release tasks.
> > There's nothing to stop do_rm_work running before (or even during!) one of
> > the source release tasks.
> 
> Can you show how you use recrdeptask and how you call bitbake to trigger
> those extra tasks, just for my understanding?

Certainly, we have a bbclass globally in INHERIT that contains:

 addtask source_release # potential fix here
 do_source_release() {
     :
 }

 addtask all_source_releases
 xx_do_all_source_releases() {
     :
 }

 do_all_source_releases[nostamp] = "1"
 do_all_source_releases[recrdeptask] += "do_all_source_releases do_source_release"

 addtask husk_recipe before do_source_release
 python xx_do_husk_recipe() {
    ...
 }

 (there's also another task similar to do_husk_recipe)

and in the particular recipe that has trouble with racing against rm_work:

 do_husk_recipe() {
    # do stuff in ${WORKDIR}
 }
 addtask husk_recipe after do_populate_sysroot before do_source_release

there's also a source-release-world recipe that contains:

 DEPENDS = "our-image"

and we run:

 bitbake -c all_source_releases source-release-world

> I suppose it worked before because your tasks could depend on do_build
> without triggering do_rm_work, while now that is included.

I believe so.

> > It seems that we need to ensure that do_rm_work also needs to depend on our
> > ancillary tasks too, but only if they are being built. I'm unsure how this
> > can be done though. :(
> 
> How do you determine whether the tasks need to run? Does it depend on
> how bitbake is invoked or does it depend on specific properties of the
> recipe?

Running bitbake as above.

I think that I could fix this by changing:

 addtask source_release

to

 addtask source_release before do_rm_work

(and also fixing any other tasks that suffer from this problem)

but I was hoping that there was a better fix to rm_work.bbclass itself.

Thanks.

Mike.

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Patrick Ohly]

On Wed, 2017-02-08 at 13:48 +0000, Mike Crowe wrote:
> On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> > On Wed, 2017-02-08 at 11:50 +0000, Mike Crowe wrote:
> > > On Friday 13 January 2017 at 15:52:33 +0100, Patrick Ohly wrote:
> > > > The right solution is to inject do_rm_work before do_build and after
> > > > all tasks of the recipe. Achieving that depends on the new bitbake
> > > > bb.event.RecipeTaskPreProcess and bb.build.preceedtask().
> > > 
> > > We've run into trouble with this change. We have a number of custom
> > > ancillary tasks that are used to generate source release files and run
> > > package tests. No other tasks (including do_build) depend on these tasks
> > > since they are run explicitly when required using bitbake -c; either
> > > directly or via a recrdeptask.
> > > 
> > > Running a single task continues to work correctly - presumably this is
> > > because the do_build task is not being run, so its dependencies (including
> > > rm_work) aren't run either.
> > > 
> > > Running via the recrdeptask fails. This is because for any particular
> > > recipe we end up depending on both do_build and the source release tasks.
> > > There's nothing to stop do_rm_work running before (or even during!) one of
> > > the source release tasks.
> > 
> > Can you show how you use recrdeptask and how you call bitbake to trigger
> > those extra tasks, just for my understanding?
> 
> Certainly, we have a bbclass globally in INHERIT that contains:
> 
>  addtask source_release # potential fix here
>  do_source_release() {
>      :
>  }
> 
>  addtask all_source_releases
>  xx_do_all_source_releases() {
>      :
>  }
> 
>  do_all_source_releases[nostamp] = "1"
>  do_all_source_releases[recrdeptask] += "do_all_source_releases do_source_release"
> 
>  addtask husk_recipe before do_source_release
>  python xx_do_husk_recipe() {
>     ...
>  }
> 
>  (there's also another task similar to do_husk_recipe)
> 
> and in the particular recipe that has trouble with racing against rm_work:
> 
>  do_husk_recipe() {
>     # do stuff in ${WORKDIR}
>  }
>  addtask husk_recipe after do_populate_sysroot before do_source_release
> 
> there's also a source-release-world recipe that contains:
> 
>  DEPENDS = "our-image"
> 
> and we run:
> 
>  bitbake -c all_source_releases source-release-world

I tried to replicate that with Poky master (= 226a508da), but without
luck:

/work/poky$ cat meta/classes/release-source.bbclass 
addtask source_release # potential fix here
do_source_release() {
    :
}

addtask all_source_releases
do_all_source_releases() {
    :
}

do_all_source_releases[nostamp] = "1"
do_all_source_releases[recrdeptask] += "do_all_source_releases do_source_release"

addtask husk_recipe before do_source_release
python do_husk_recipe() {
    pass
}

/work/poky$ cat meta/recipes-core/husk/husk.bb 
LICENSE = "custom"

do_husk_recipe() {
    # do stuff in ${WORKDIR}
    :
}
addtask husk_recipe after do_populate_sysroot before do_source_release

/work/poky$ cat meta/recipes-core/husk/source-release-world.bb
LICENSE = "custom"
DEPENDS = "core-image-sato"

/work/poky/build$ tail -1 conf/local.conf
INHERIT += "rm_work release-source"

/work/poky/build$ bitbake --dry-run -c all_source_releases source-release-world
...

That last command does not trigger the do_rm_work task and thus there is
also no race.

I had a hunch that it is related to populating the sysroot and how it
might depend on do_build in the dependencies, but that's not it. I added
DEPENDS="m4" to the husk.bb above and it has an effect:

/work/poky/build$ bitbake -g -c all_source_releases husk
Loading cache: 100% |
#################################################################################| Time: 0:00:00
Loaded 1324 entries from dependency cache.
NOTE: Resolving any missing task queue dependencies
NOTE: PN build list saved to 'pn-buildlist'
NOTE: PN dependencies saved to 'pn-depends.dot'
NOTE: Package dependencies saved to 'package-depends.dot'
NOTE: Task dependencies saved to 'task-depends.dot'
/work/poky/build$ grep -e 'husk.*->.*m4' task-depends.dot 
"husk.do_all_source_releases" -> "m4.do_source_release"
"husk.do_all_source_releases" -> "m4-native.do_source_release"
"husk.do_prepare_recipe_sysroot" -> "m4.do_populate_sysroot"

But it does not cause m4.do_build and thus m4.do_rm_work to run.

> > > It seems that we need to ensure that do_rm_work also needs to depend on our
> > > ancillary tasks too, but only if they are being built. I'm unsure how this
> > > can be done though. :(
> > 
> > How do you determine whether the tasks need to run? Does it depend on
> > how bitbake is invoked or does it depend on specific properties of the
> > recipe?
> 
> Running bitbake as above.
> 
> I think that I could fix this by changing:
> 
>  addtask source_release
> 
> to
> 
>  addtask source_release before do_rm_work
> 
> (and also fixing any other tasks that suffer from this problem)
> 
> but I was hoping that there was a better fix to rm_work.bbclass itself.

It seems unsafe to have tasks that are not properly ordered and just
rely on not activating them in the same build, but without understanding
the problem better it is too early to look for a solution.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Mike Crowe]

On Thursday 09 February 2017 at 17:24:39 +0100, Patrick Ohly wrote:
> On Wed, 2017-02-08 at 13:48 +0000, Mike Crowe wrote:
> > On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> > > On Wed, 2017-02-08 at 11:50 +0000, Mike Crowe wrote:
> > > > On Friday 13 January 2017 at 15:52:33 +0100, Patrick Ohly wrote:
> > > > > The right solution is to inject do_rm_work before do_build and after
> > > > > all tasks of the recipe. Achieving that depends on the new bitbake
> > > > > bb.event.RecipeTaskPreProcess and bb.build.preceedtask().
> > > > 
> > > > We've run into trouble with this change. We have a number of custom
> > > > ancillary tasks that are used to generate source release files and run
> > > > package tests. No other tasks (including do_build) depend on these tasks
> > > > since they are run explicitly when required using bitbake -c; either
> > > > directly or via a recrdeptask.
> > > > 
> > > > Running a single task continues to work correctly - presumably this is
> > > > because the do_build task is not being run, so its dependencies (including
> > > > rm_work) aren't run either.
> > > > 
> > > > Running via the recrdeptask fails. This is because for any particular
> > > > recipe we end up depending on both do_build and the source release tasks.
> > > > There's nothing to stop do_rm_work running before (or even during!) one of
> > > > the source release tasks.
> > > 
> > > Can you show how you use recrdeptask and how you call bitbake to trigger
> > > those extra tasks, just for my understanding?
> > 
> > Certainly, we have a bbclass globally in INHERIT that contains:
> > 
> >  addtask source_release # potential fix here
> >  do_source_release() {
> >      :
> >  }
> > 
> >  addtask all_source_releases
> >  xx_do_all_source_releases() {
> >      :
> >  }
> > 
> >  do_all_source_releases[nostamp] = "1"
> >  do_all_source_releases[recrdeptask] += "do_all_source_releases do_source_release"
> > 
> >  addtask husk_recipe before do_source_release
> >  python xx_do_husk_recipe() {
> >     ...
> >  }
> > 
> >  (there's also another task similar to do_husk_recipe)
> > 
> > and in the particular recipe that has trouble with racing against rm_work:
> > 
> >  do_husk_recipe() {
> >     # do stuff in ${WORKDIR}
> >  }
> >  addtask husk_recipe after do_populate_sysroot before do_source_release
> > 
> > there's also a source-release-world recipe that contains:
> > 
> >  DEPENDS = "our-image"
> > 
> > and we run:
> > 
> >  bitbake -c all_source_releases source-release-world
> 
> I tried to replicate that with Poky master (= 226a508da), but without
> luck:
> 
> /work/poky$ cat meta/classes/release-source.bbclass 
> addtask source_release # potential fix here
> do_source_release() {
>     :
> }
> 
> addtask all_source_releases
> do_all_source_releases() {
>     :
> }
> 
> do_all_source_releases[nostamp] = "1"
> do_all_source_releases[recrdeptask] += "do_all_source_releases do_source_release"
> 
> addtask husk_recipe before do_source_release
> python do_husk_recipe() {
>     pass
> }
> 
> /work/poky$ cat meta/recipes-core/husk/husk.bb 
> LICENSE = "custom"
> 
> do_husk_recipe() {
>     # do stuff in ${WORKDIR}
>     :
> }
> addtask husk_recipe after do_populate_sysroot before do_source_release
> 
> /work/poky$ cat meta/recipes-core/husk/source-release-world.bb
> LICENSE = "custom"
> DEPENDS = "core-image-sato"
> 
> /work/poky/build$ tail -1 conf/local.conf
> INHERIT += "rm_work release-source"

The part I'd missed is the all-important line in source-release-world.bb:

 do_source_release[depends] += "core-image-sato:do_build"

We have this to ensure that the source release includes everything that is
required to build the rootfs itself. (For example, elfutils-native is only
included if this line is present.)

> /work/poky/build$ bitbake --dry-run -c all_source_releases source-release-world
> ...
> 
> That last command does not trigger the do_rm_work task and thus there is
> also no race.

That was my experience without the magic extra line too. Sorry I omitted
that.

> It seems unsafe to have tasks that are not properly ordered and just
> rely on not activating them in the same build, but without understanding
> the problem better it is too early to look for a solution.

Thanks for investigating. If you're still having trouble then I have a
single patch on top of current oe-core master that reproduces it for me
that I can send.

Mike.

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Patrick Ohly]

On Fri, 2017-02-10 at 18:32 +0000, Mike Crowe wrote:
> On Thursday 09 February 2017 at 17:24:39 +0100, Patrick Ohly wrote:
> > On Wed, 2017-02-08 at 13:48 +0000, Mike Crowe wrote:
> > > On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> The part I'd missed is the all-important line in source-release-world.bb:
> 
>  do_source_release[depends] += "core-image-sato:do_build"

Okay, that explains it.

IMHO this do_build dependency should trigger do_rm_work. Your "bitbake
-c all_source_releases source-release-world" intentionally includes a
real world build, not just executing the source release tasks. Cleaning
up while building is the goal of rm_work.bbclass. It's arguably a
deficiency in the previous rm_work.bbclass that it wasn't active in your
case.

Now we just need to find a way to combine these without breaking the
extra tasks.

> > It seems unsafe to have tasks that are not properly ordered and just
> > rely on not activating them in the same build, but without understanding
> > the problem better it is too early to look for a solution.
> 
> Thanks for investigating. If you're still having trouble then I have a
> single patch on top of current oe-core master that reproduces it for me
> that I can send.

I can reproduce it.

As you said earlier, adding "before do_rm_work" solves the problem:
addtask source_release before do_rm_work

That's okay, even when rm_work.bbclass does not get inherited. However,
when rm_work.bbclass, a "normal" build like "bitbake menu-cache" ends up
triggering the source_release task:

$ bitbake -g menu-cache
...
$ grep do_rm_work task-depends.dot  | grep do_source_release | grep menu-cache
"menu-cache.do_rm_work" -> "menu-cache.do_source_release"

Is that acceptable for you?

To me it seems like the right solution. Inheriting
release-source.bbclass could be limited to builds which produce
releases, for example in your CI setup, then normal developers will not
be affected.

With that in mind, you could also just do:
addtask source_release before do_build

And then build with:
bitbake world (or your image)

That seems simpler than the construct with do_all_source_releases and
the special source-release-world.bb.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Mike Crowe]

On Monday 13 February 2017 at 11:54:32 +0100, Patrick Ohly wrote:
> On Fri, 2017-02-10 at 18:32 +0000, Mike Crowe wrote:
> > On Thursday 09 February 2017 at 17:24:39 +0100, Patrick Ohly wrote:
> > > On Wed, 2017-02-08 at 13:48 +0000, Mike Crowe wrote:
> > > > On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> > The part I'd missed is the all-important line in source-release-world.bb:
> > 
> >  do_source_release[depends] += "core-image-sato:do_build"
> 
> Okay, that explains it.
> 
> IMHO this do_build dependency should trigger do_rm_work. Your "bitbake
> -c all_source_releases source-release-world" intentionally includes a
> real world build, not just executing the source release tasks. Cleaning
> up while building is the goal of rm_work.bbclass. It's arguably a
> deficiency in the previous rm_work.bbclass that it wasn't active in your
> case.
> 
> Now we just need to find a way to combine these without breaking the
> extra tasks.
> 
> > > It seems unsafe to have tasks that are not properly ordered and just
> > > rely on not activating them in the same build, but without understanding
> > > the problem better it is too early to look for a solution.
> > 
> > Thanks for investigating. If you're still having trouble then I have a
> > single patch on top of current oe-core master that reproduces it for me
> > that I can send.
> 
> I can reproduce it.
> 
> As you said earlier, adding "before do_rm_work" solves the problem:
> addtask source_release before do_rm_work

I'd temporarily forgotten that this would cause every build to include the
source release when I wrote that. :(

> That's okay, even when rm_work.bbclass does not get inherited. However,
> when rm_work.bbclass, a "normal" build like "bitbake menu-cache" ends up
> triggering the source_release task:
> 
> $ bitbake -g menu-cache
> ...
> $ grep do_rm_work task-depends.dot  | grep do_source_release | grep menu-cache
> "menu-cache.do_rm_work" -> "menu-cache.do_source_release"
> 
> Is that acceptable for you?

Not really.

> To me it seems like the right solution. Inheriting
> release-source.bbclass could be limited to builds which produce
> releases, for example in your CI setup, then normal developers will not
> be affected.

At the moment it is straightforward to build the source release with a
simple bitbake invocation.

Your solution would work, but it would be necessary to meddle with
local.conf or similar in order to generate the source release.

Is there any way we can get our tasks in as predecessors of rm_work only if
they would have run anyway? Rather like the way make(1) supports order-only
prerequisites[1]?

Thanks.

Mike.

[1] https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html

Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner

[Patrick Ohly]

On Mon, 2017-02-13 at 12:19 +0000, Mike Crowe wrote:
> On Monday 13 February 2017 at 11:54:32 +0100, Patrick Ohly wrote:
> > To me it seems like the right solution. Inheriting
> > release-source.bbclass could be limited to builds which produce
> > releases, for example in your CI setup, then normal developers will not
> > be affected.
> 
> At the moment it is straightforward to build the source release with a
> simple bitbake invocation.
> 
> Your solution would work, but it would be necessary to meddle with
> local.conf or similar in order to generate the source release.

There is conf/auto.conf for that. local.conf can stay unmodified.

Alternatively, you could also introduce an environment variable which
controls whether release-source.bbclass gets inherited. Add that
variable to the BB_ENV_EXTRAWHITE in your local.conf.sample and builds
with or without the class could be as simple as:
RELEASE_SOURCE=1 bitbake ...

> Is there any way we can get our tasks in as predecessors of rm_work only if
> they would have run anyway? Rather like the way make(1) supports order-only
> prerequisites[1]?

No, I don't think there is such a dependency in bitbake, and I'm not
convinced that the usecase justifies the extra complexity.

It might "feel" natural to use the "addtask before" as such a weaker
ordering relationship and the "addtask after" as the stronger "must run
dependency" relationship, but that's just not the current semantic and
changing it probably would break too much.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Running task for all recipes required by an image (was Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner)

[Mike Crowe]

On Monday 13 February 2017 at 11:54:32 +0100, Patrick Ohly wrote:
> On Fri, 2017-02-10 at 18:32 +0000, Mike Crowe wrote:
> > On Thursday 09 February 2017 at 17:24:39 +0100, Patrick Ohly wrote:
> > > On Wed, 2017-02-08 at 13:48 +0000, Mike Crowe wrote:
> > > > On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> > The part I'd missed is the all-important line in source-release-world.bb:
> > 
> >  do_source_release[depends] += "core-image-sato:do_build"
> 
> Okay, that explains it.
> 
> IMHO this do_build dependency should trigger do_rm_work. Your "bitbake
> -c all_source_releases source-release-world" intentionally includes a
> real world build, not just executing the source release tasks. Cleaning
> up while building is the goal of rm_work.bbclass. It's arguably a
> deficiency in the previous rm_work.bbclass that it wasn't active in your
> case.
> 
> Now we just need to find a way to combine these without breaking the
> extra tasks.

Now I think about this further, we're only depending on do_build in order
to ensure that we get all the dependencies included in the source release
via the recrdeps task. If there were a better way to do that then perhaps
rm_work wouldn't cause any problems, and we also wouldn't waste time
building stuff that we aren't going to use.

I've tried:

 ALL_DEPENDENCIES = "${RDEPENDS} ${DEPENDS}"
 do_source_release[depends] = "${@' '.join([s+':do_source_release ' for s in d.getVar('ALL_DEPENDENCIES', True).split()])}"

in the bbclass (which is still inherited by all recipes) and then just
running the source_release task on the final images.

Unfortunately this fails when IMAGE_INSTALL contains package names that are
generated using PACKAGES_DYNAMIC such as gstreamer plugins. :(

ERROR: Nothing PROVIDES 'gstreamer1.0-plugins-good-audioparsers'. Close matches:
  gstreamer1.0-plugins-good
  gstreamer1.0-plugins-base
  gstreamer1.0-plugins-bad
  gstreamer1.0-plugins-good RPROVIDES gstreamer1.0-plugins-good-audioparsers

Is there a correct way to get a task to run for every recipe that is
required by an image?

Thanks.

Mike.

Re: Running task for all recipes required by an image (was Re: [PATCH v2 3/3] rm_work.bbclass: clean up sooner)

[Patrick Ohly]

On Fri, 2017-02-17 at 15:21 +0000, Mike Crowe wrote:
> On Monday 13 February 2017 at 11:54:32 +0100, Patrick Ohly wrote:
> > On Fri, 2017-02-10 at 18:32 +0000, Mike Crowe wrote:
> > > On Thursday 09 February 2017 at 17:24:39 +0100, Patrick Ohly wrote:
> > > > On Wed, 2017-02-08 at 13:48 +0000, Mike Crowe wrote:
> > > > > On Wednesday 08 February 2017 at 14:04:42 +0100, Patrick Ohly wrote:
> > > The part I'd missed is the all-important line in source-release-world.bb:
> > > 
> > >  do_source_release[depends] += "core-image-sato:do_build"
> > 
> > Okay, that explains it.
> > 
> > IMHO this do_build dependency should trigger do_rm_work. Your "bitbake
> > -c all_source_releases source-release-world" intentionally includes a
> > real world build, not just executing the source release tasks. Cleaning
> > up while building is the goal of rm_work.bbclass. It's arguably a
> > deficiency in the previous rm_work.bbclass that it wasn't active in your
> > case.
> > 
> > Now we just need to find a way to combine these without breaking the
> > extra tasks.
> 
> Now I think about this further, we're only depending on do_build in order
> to ensure that we get all the dependencies included in the source release
> via the recrdeps task. If there were a better way to do that then perhaps
> rm_work wouldn't cause any problems, and we also wouldn't waste time
> building stuff that we aren't going to use.

Isn't that exactly what do_fetchall does? I thought you wanted to build
things as part of your source-release-world. If that isn't needed, then
this in release-source.bbclass might work:

do_all_source_releases[recrdeptask] = "do_all_source_releases do_source_release"
do_all_source_releases[recideptask] = "do_build"

And used like this:
bitbake -c all_source_releases core-image-sato

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.