[OE-core][PATCH v2 0/2] introduce multi-state image deployment

[OE-core][PATCH v2 0/2] introduce multi-state image deployment

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Hi Richard et al,

This is another shot at multi-stage image deployment. This time around
I managed to fix the problem with changing task hashes and anonymous
python functions ordering by simply moving the code assigning pre- and
postfuncs for sstate into an even handler invoked right after all
anonymous python code is run. Seems to work fine and there's no need
to deltask do_package anymore.

Just like before: each image task becomes an sstate task and handles
the deployment of the artifacts it generates. Files created by tasks
other than IMAGE_CMD are still deployed by do_image_complete.

The dm-verity problem I'm trying to solve is described here:
    https://lists.openembedded.org/g/openembedded-core/message/136203

Bartosz Golaszewski (2):
  qemuboot.bbclass: don't redefine IMGDEPLOYDIR
  image.bbclass: deploy image artifacts in stages

 meta/classes/image.bbclass    | 17 +++++++++++++++++
 meta/classes/qemuboot.bbclass |  1 -
 meta/classes/sstate.bbclass   |  4 ++++
 3 files changed, 21 insertions(+), 1 deletion(-)

-- 
2.25.0

[OE-core][PATCH v2 1/2] qemuboot.bbclass: don't redefine IMGDEPLOYDIR

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

This variable is already defined in image.bbclass and there's not need
to redefine it here.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 meta/classes/qemuboot.bbclass | 1 -
 1 file changed, 1 deletion(-)

diff --git a/meta/classes/qemuboot.bbclass b/meta/classes/qemuboot.bbclass
index 54044c38da..68f7a03619 100644
--- a/meta/classes/qemuboot.bbclass
+++ b/meta/classes/qemuboot.bbclass
@@ -83,7 +83,6 @@ QB_DRIVE_TYPE ?= "/dev/sd"
 
 # Create qemuboot.conf
 addtask do_write_qemuboot_conf after do_rootfs before do_image
-IMGDEPLOYDIR ?= "${WORKDIR}/deploy-${PN}-image-complete"
 
 def qemuboot_vars(d):
     build_vars = ['MACHINE', 'TUNE_ARCH', 'DEPLOY_DIR_IMAGE',
-- 
2.25.0

[OE-core][PATCH v2 2/2] image.bbclass: deploy image artifacts in stages

[Bartosz Golaszewski]

From: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Make each IMAGE_CMD task an sstate task with its own IMGDEPLOYDIR
override. This way each generated set of artifacts is deployed as soon
as it's ready instead of the do_image_complete task handling the entire
deployement. This allows us to better fine-tune dependencies e.g. we
can make do_image_wic depend on fitImage task which can in turn depend
on do_image_ext4.

We need delete the IMGDEPLOYDIR variable from the data object passed
to each image task so that it gets expanded with the correct override.

In order to make sure that tasks added to SSTATETASKS in anonymous python
functions are correctly setup, move the code that assigns pre- and
postfuncs to an event handler invoked on bb.event.RecipeTaskPreProcess
in sstate.bbclass. This event is fired right after the anonymous
functions.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
---
 meta/classes/image.bbclass  | 17 +++++++++++++++++
 meta/classes/sstate.bbclass |  4 ++++
 2 files changed, 21 insertions(+)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 07aa1f1fa5..ffd63aa82a 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -428,10 +428,12 @@ python () {
         # date/time values. It will get expanded at execution time.
         # Similarly TMPDIR since otherwise we see QA stamp comparision problems
         # Expand PV else it can trigger get_srcrev which can fail due to these variables being unset
+        # Delete IMGDEPLOYDIR so that each task gets its own, overriden value
         localdata.setVar('PV', d.getVar('PV'))
         localdata.delVar('DATETIME')
         localdata.delVar('DATE')
         localdata.delVar('TMPDIR')
+        localdata.delVar('IMGDEPLOYDIR')
         vardepsexclude = (d.getVarFlag('IMAGE_CMD_' + realt, 'vardepsexclude', True) or '').split()
         for dep in vardepsexclude:
             localdata.delVar(dep)
@@ -502,6 +504,21 @@ python () {
 
         bb.debug(2, "Adding task %s before %s, after %s" % (task, 'do_image_complete', after))
         bb.build.addtask(task, 'do_image_complete', after, d)
+
+        imgdeploydir = d.getVar('IMGDEPLOYDIR')
+        task_override = task[3:].replace('_', '-') # 'do_image_ext4' becomes 'image-ext4'
+        taskdeploydir = '{}/deploy-{}-{}'.format(os.path.dirname(imgdeploydir),
+                                                 d.getVar('PN'), task_override)
+
+        # Each image task gets its own IMGDEPLOYDIR directory and is added to
+        # SSTATETASKS. This way every set of artifacts gets deployed right after
+        # the do_image_foo task completes.
+        d.setVar('IMGDEPLOYDIR_task-{}'.format(task_override), taskdeploydir)
+        d.appendVar('SSTATETASKS', ' {}'.format(task))
+        d.setVarFlag(task, 'sstate-inputdirs', taskdeploydir)
+        d.setVarFlag(task, 'sstate-outputdirs', d.getVar('DEPLOY_DIR_IMAGE'))
+        d.setVarFlag(task, 'cleandirs', taskdeploydir)
+        d.setVar('SSTATE_SKIP_CREATION_task-{}'.format(task_override), '1')
 }
 
 #
diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index c73c3b42a7..3c71312010 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -138,13 +138,17 @@ python () {
         d.setVar('SSTATE_EXTRAPATH', "${NATIVELSBSTRING}/")
         d.setVar('BB_HASHFILENAME', "True ${SSTATE_PKGSPEC} ${SSTATE_SWSPEC}")
         d.setVar('SSTATE_EXTRAPATHWILDCARD', "${NATIVELSBSTRING}/")
+}
 
+python sstate_setup_tasks() {
     unique_tasks = sorted(set((d.getVar('SSTATETASKS') or "").split()))
     d.setVar('SSTATETASKS', " ".join(unique_tasks))
     for task in unique_tasks:
         d.prependVarFlag(task, 'prefuncs', "sstate_task_prefunc ")
         d.appendVarFlag(task, 'postfuncs', " sstate_task_postfunc")
 }
+addhandler sstate_setup_tasks
+sstate_setup_tasks[eventmask] = "bb.event.RecipeTaskPreProcess"
 
 def sstate_init(task, d):
     ss = {}
-- 
2.25.0

Re: [OE-core][PATCH v2 2/2] image.bbclass: deploy image artifacts in stages

[Bartosz Golaszewski]

pon., 6 kwi 2020 o 19:02 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Make each IMAGE_CMD task an sstate task with its own IMGDEPLOYDIR
> override. This way each generated set of artifacts is deployed as soon
> as it's ready instead of the do_image_complete task handling the entire
> deployement. This allows us to better fine-tune dependencies e.g. we
> can make do_image_wic depend on fitImage task which can in turn depend
> on do_image_ext4.
>
> We need delete the IMGDEPLOYDIR variable from the data object passed
> to each image task so that it gets expanded with the correct override.
>
> In order to make sure that tasks added to SSTATETASKS in anonymous python
> functions are correctly setup, move the code that assigns pre- and
> postfuncs to an event handler invoked on bb.event.RecipeTaskPreProcess
> in sstate.bbclass. This event is fired right after the anonymous
> functions.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>  meta/classes/image.bbclass  | 17 +++++++++++++++++
>  meta/classes/sstate.bbclass |  4 ++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 07aa1f1fa5..ffd63aa82a 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -428,10 +428,12 @@ python () {
>          # date/time values. It will get expanded at execution time.
>          # Similarly TMPDIR since otherwise we see QA stamp comparision problems
>          # Expand PV else it can trigger get_srcrev which can fail due to these variables being unset
> +        # Delete IMGDEPLOYDIR so that each task gets its own, overriden value
>          localdata.setVar('PV', d.getVar('PV'))
>          localdata.delVar('DATETIME')
>          localdata.delVar('DATE')
>          localdata.delVar('TMPDIR')
> +        localdata.delVar('IMGDEPLOYDIR')
>          vardepsexclude = (d.getVarFlag('IMAGE_CMD_' + realt, 'vardepsexclude', True) or '').split()
>          for dep in vardepsexclude:
>              localdata.delVar(dep)
> @@ -502,6 +504,21 @@ python () {
>
>          bb.debug(2, "Adding task %s before %s, after %s" % (task, 'do_image_complete', after))
>          bb.build.addtask(task, 'do_image_complete', after, d)
> +
> +        imgdeploydir = d.getVar('IMGDEPLOYDIR')
> +        task_override = task[3:].replace('_', '-') # 'do_image_ext4' becomes 'image-ext4'
> +        taskdeploydir = '{}/deploy-{}-{}'.format(os.path.dirname(imgdeploydir),
> +                                                 d.getVar('PN'), task_override)
> +
> +        # Each image task gets its own IMGDEPLOYDIR directory and is added to
> +        # SSTATETASKS. This way every set of artifacts gets deployed right after
> +        # the do_image_foo task completes.
> +        d.setVar('IMGDEPLOYDIR_task-{}'.format(task_override), taskdeploydir)
> +        d.appendVar('SSTATETASKS', ' {}'.format(task))
> +        d.setVarFlag(task, 'sstate-inputdirs', taskdeploydir)
> +        d.setVarFlag(task, 'sstate-outputdirs', d.getVar('DEPLOY_DIR_IMAGE'))
> +        d.setVarFlag(task, 'cleandirs', taskdeploydir)
> +        d.setVar('SSTATE_SKIP_CREATION_task-{}'.format(task_override), '1')
>  }
>
>  #
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index c73c3b42a7..3c71312010 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -138,13 +138,17 @@ python () {
>          d.setVar('SSTATE_EXTRAPATH', "${NATIVELSBSTRING}/")
>          d.setVar('BB_HASHFILENAME', "True ${SSTATE_PKGSPEC} ${SSTATE_SWSPEC}")
>          d.setVar('SSTATE_EXTRAPATHWILDCARD', "${NATIVELSBSTRING}/")
> +}
>
> +python sstate_setup_tasks() {
>      unique_tasks = sorted(set((d.getVar('SSTATETASKS') or "").split()))
>      d.setVar('SSTATETASKS', " ".join(unique_tasks))
>      for task in unique_tasks:
>          d.prependVarFlag(task, 'prefuncs', "sstate_task_prefunc ")
>          d.appendVarFlag(task, 'postfuncs', " sstate_task_postfunc")
>  }
> +addhandler sstate_setup_tasks
> +sstate_setup_tasks[eventmask] = "bb.event.RecipeTaskPreProcess"
>
>  def sstate_init(task, d):
>      ss = {}
> --
> 2.25.0
>

Gentle ping.

Bart

Re: [OE-core][PATCH v2 2/2] image.bbclass: deploy image artifacts in stages

[Richard Purdie]

On Wed, 2020-04-15 at 12:20 +0200, Bartosz Golaszewski wrote:
> pon., 6 kwi 2020 o 19:02 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > 
> > Make each IMAGE_CMD task an sstate task with its own IMGDEPLOYDIR
> > override. This way each generated set of artifacts is deployed as soon
> > as it's ready instead of the do_image_complete task handling the entire
> > deployement. This allows us to better fine-tune dependencies e.g. we
> > can make do_image_wic depend on fitImage task which can in turn depend
> > on do_image_ext4.
> > 
> > We need delete the IMGDEPLOYDIR variable from the data object passed
> > to each image task so that it gets expanded with the correct override.
> > 
> > In order to make sure that tasks added to SSTATETASKS in anonymous python
> > functions are correctly setup, move the code that assigns pre- and
> > postfuncs to an event handler invoked on bb.event.RecipeTaskPreProcess
> > in sstate.bbclass. This event is fired right after the anonymous
> > functions.
> > 
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  meta/classes/image.bbclass  | 17 +++++++++++++++++
> >  meta/classes/sstate.bbclass |  4 ++++
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index 07aa1f1fa5..ffd63aa82a 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -428,10 +428,12 @@ python () {
> >          # date/time values. It will get expanded at execution time.
> >          # Similarly TMPDIR since otherwise we see QA stamp comparision problems
> >          # Expand PV else it can trigger get_srcrev which can fail due to these variables being unset
> > +        # Delete IMGDEPLOYDIR so that each task gets its own, overriden value
> >          localdata.setVar('PV', d.getVar('PV'))
> >          localdata.delVar('DATETIME')
> >          localdata.delVar('DATE')
> >          localdata.delVar('TMPDIR')
> > +        localdata.delVar('IMGDEPLOYDIR')
> >          vardepsexclude = (d.getVarFlag('IMAGE_CMD_' + realt, 'vardepsexclude', True) or '').split()
> >          for dep in vardepsexclude:
> >              localdata.delVar(dep)
> > @@ -502,6 +504,21 @@ python () {
> > 
> >          bb.debug(2, "Adding task %s before %s, after %s" % (task, 'do_image_complete', after))
> >          bb.build.addtask(task, 'do_image_complete', after, d)
> > +
> > +        imgdeploydir = d.getVar('IMGDEPLOYDIR')
> > +        task_override = task[3:].replace('_', '-') # 'do_image_ext4' becomes 'image-ext4'
> > +        taskdeploydir = '{}/deploy-{}-{}'.format(os.path.dirname(imgdeploydir),
> > +                                                 d.getVar('PN'), task_override)
> > +
> > +        # Each image task gets its own IMGDEPLOYDIR directory and is added to
> > +        # SSTATETASKS. This way every set of artifacts gets deployed right after
> > +        # the do_image_foo task completes.
> > +        d.setVar('IMGDEPLOYDIR_task-{}'.format(task_override), taskdeploydir)
> > +        d.appendVar('SSTATETASKS', ' {}'.format(task))
> > +        d.setVarFlag(task, 'sstate-inputdirs', taskdeploydir)
> > +        d.setVarFlag(task, 'sstate-outputdirs', d.getVar('DEPLOY_DIR_IMAGE'))
> > +        d.setVarFlag(task, 'cleandirs', taskdeploydir)
> > +        d.setVar('SSTATE_SKIP_CREATION_task-{}'.format(task_override), '1')
> >  }
> > 
> >  #
> > diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> > index c73c3b42a7..3c71312010 100644
> > --- a/meta/classes/sstate.bbclass
> > +++ b/meta/classes/sstate.bbclass
> > @@ -138,13 +138,17 @@ python () {
> >          d.setVar('SSTATE_EXTRAPATH', "${NATIVELSBSTRING}/")
> >          d.setVar('BB_HASHFILENAME', "True ${SSTATE_PKGSPEC} ${SSTATE_SWSPEC}")
> >          d.setVar('SSTATE_EXTRAPATHWILDCARD', "${NATIVELSBSTRING}/")
> > +}
> > 
> > +python sstate_setup_tasks() {
> >      unique_tasks = sorted(set((d.getVar('SSTATETASKS') or "").split()))
> >      d.setVar('SSTATETASKS', " ".join(unique_tasks))
> >      for task in unique_tasks:
> >          d.prependVarFlag(task, 'prefuncs', "sstate_task_prefunc ")
> >          d.appendVarFlag(task, 'postfuncs', " sstate_task_postfunc")
> >  }
> > +addhandler sstate_setup_tasks
> > +sstate_setup_tasks[eventmask] = "bb.event.RecipeTaskPreProcess"
> > 
> >  def sstate_init(task, d):
> >      ss = {}
> > --
> > 2.25.0
> > 
> 
> Gentle ping.

Gentle reminder that we're in the middle of sorting out a release. This
will not merge until after the release and I don't have the cycles for
review of this kind of change right now.

FWIW I still don't like the way this integrates into sstate and I want
to spend some time trying to find a better way. I don't have any
constructive suggestion as to what that would look like so I haven't
replied, I simply don't know and that isn't a useful answer to you.

You've also ignored my request to follow the coding style used in the 
rest of the class code :(.

Cheers,

Richard

Re: [OE-core][PATCH v2 2/2] image.bbclass: deploy image artifacts in stages

[Bartosz Golaszewski]

śr., 15 kwi 2020 o 12:32 Richard Purdie
<richard.purdie@linuxfoundation.org> napisał(a):
>
> > >
> >
> > Gentle ping.
>
> Gentle reminder that we're in the middle of sorting out a release. This
> will not merge until after the release and I don't have the cycles for
> review of this kind of change right now.
>

Got it, I'll get back to you after the release then.

> FWIW I still don't like the way this integrates into sstate and I want
> to spend some time trying to find a better way. I don't have any
> constructive suggestion as to what that would look like so I haven't
> replied, I simply don't know and that isn't a useful answer to you.
>

Sure, thanks. Despite my best efforts I couldn't figure out any
solution without touching sstate.

> You've also ignored my request to follow the coding style used in the
> rest of the class code :(.
>

Indeed, although "forgot" would describe it better. :)

Bart