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

["Richard Purdie" <richard.purdie@linuxfoundation.org>]

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