Re: [isar-cip-core][PATCH v4 2/8] refactor: use imagetypes for swu generation

["Moessbauer, Felix" <felix.moessbauer@siemens.com>]

On Mon, 2023-02-13 at 16:28 +0100, Jan Kiszka wrote:
> On 12.02.23 09:27, Felix Moessbauer wrote:
> > This patch reworks the implementation of the swupdate type.
> > All generic aspects are moved from the swupdate.inc file into the
> > swupdate class and made conditional on the swu type.
> > The sw-description file is now referenced using the image-type
> > infrastructure, which avoids manual additions to FILESEXTRAPATHS
> > and
> > accidental overwrites of SRC_URI. The templating logic is moved
> > into the
> > generic one provided by imagetypes.
> > 
> > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > ---
> >  classes/swupdate.bbclass                      | 26 ++++++++++++---
> > ----
> >  kas/opt/swupdate.yml                          |  5 +---
> >  .../images/{files => swu}/sw-description.tmpl |  0
> >  recipes-core/images/swupdate.inc              | 15 -----------
> >  4 files changed, 18 insertions(+), 28 deletions(-)
> >  rename recipes-core/images/{files => swu}/sw-description.tmpl
> > (100%)
> > 
> > diff --git a/classes/swupdate.bbclass b/classes/swupdate.bbclass
> > index 5eb4936..966f7c0 100644
> > --- a/classes/swupdate.bbclass
> > +++ b/classes/swupdate.bbclass
> > @@ -8,23 +8,33 @@
> >  #  Quirin Gylstorff <quirin.gylstorff@siemens.com>
> >  #
> >  # SPDX-License-Identifier: MIT
> > +ROOTFS_PARTITION_NAME ?= "${IMAGE_FULLNAME}.wic.p4.gz"
> >  
> >  SWU_IMAGE_FILE ?= "${DEPLOY_DIR_IMAGE}/${PN}-${DISTRO}-
> > ${MACHINE}.swu"
> >  SWU_DESCRIPTION_FILE ?= "sw-description"
> > -SWU_ADDITIONAL_FILES ?= ""
> > +SWU_ADDITIONAL_FILES ?= "linux.efi ${ROOTFS_PARTITION_NAME}"
> >  SWU_SIGNED ?= ""
> >  SWU_SIGNATURE_EXT ?= "sig"
> >  SWU_SIGNATURE_TYPE ?= "rsa"
> >  
> >  BUILDCHROOT_IMAGE_FILE ?=
> > "${PP_DEPLOY}/${@os.path.basename(d.getVar('SWU_IMAGE_FILE'))}"
> >  
> > -IMAGER_INSTALL += "cpio"
> > -IMAGER_INSTALL += "${@'openssl' if
> > bb.utils.to_boolean(d.getVar('SWU_SIGNED')) else ''}"
> > +IMAGE_TYPEDEP:wic += "squashfs"
> > +IMAGE_TYPEDEP:swu = "wic"
> > +IMAGER_INSTALL:swu += "cpio ${@'openssl' if
> > bb.utils.to_boolean(d.getVar('SWU_SIGNED')) else ''}"
> >  
> > -do_swupdate_binary[stamp-extra-info] = "${DISTRO}-${MACHINE}"
> > -do_swupdate_binary[cleandirs] += "${WORKDIR}/swu"
> > -do_swupdate_binary[network] = "${TASK_USE_SUDO}"
> > -do_swupdate_binary() {
> > +IMAGE_SRC_URI:swu = "file://${SWU_DESCRIPTION_FILE}.tmpl"
> > +IMAGE_TEMPLATE_FILES:swu = "${SWU_DESCRIPTION_FILE}.tmpl"
> > +IMAGE_TEMPLATE_VARS:swu = "ROOTFS_PARTITION_NAME TARGET_IMAGE_UUID
> > ABROOTFS_PART_UUID_A ABROOTFS_PART_UUID_B"
> > +
> > +# This imagetype is neither machine nor distro specific. Hence, we
> > cannot
> > +# use paths in FILESOVERRIDES. Manual modifications of this
> > variable are
> > +# discouradged and hard to implement. Instead, we register this
> > path explicitly
> > +FILESEXTRAPATHS:prepend = "${LAYERDIR_cip-core}/recipes-
> > core/images/swu:"
> 
> This means downstream layers wanting to overload the sw-
> description.tmpl
> now need to register their own FILESEXTRAPATHS? That at least
> requires
> documentation. Or can we also drop LAYERDIR_cip-core here, and it
> would
> still work?

That is exactly as it always has been. But downstream layer have to add
it only if they override the SWU_DESCRIPTION_FILE. IMHO this is
reasonable. I highly vote for keeping it like that, as these kind of
things bite us again and again with each bitbake update.

For the OVA part which I recently re-modeled in ISAR, this is not
required as both vmware and virtualbox are machines, and both
/${MACHINE} and /${DISTRO} is appended to any search paths.

> 
> > +
> > +do_image_swu[stamp-extra-info] = "${DISTRO}-${MACHINE}"
> > +do_image_swu[cleandirs] += "${WORKDIR}/swu"
> > +IMAGE_CMD:swu() {
> >      rm -f '${SWU_IMAGE_FILE}'
> >      cp '${WORKDIR}/${SWU_DESCRIPTION_FILE}'
> > '${WORKDIR}/swu/${SWU_DESCRIPTION_FILE}'
> >  
> > @@ -91,5 +101,3 @@ do_swupdate_binary() {
> >             fi
> >          done | cpio -ovL -H crc > "${BUILDCHROOT_IMAGE_FILE}"'
> >  }
> > -
> > -addtask swupdate_binary before do_build after do_deploy
> > do_copy_boot_files do_install_imager_deps do_transform_template
> > diff --git a/kas/opt/swupdate.yml b/kas/opt/swupdate.yml
> > index ae5e3a1..80cd86e 100644
> > --- a/kas/opt/swupdate.yml
> > +++ b/kas/opt/swupdate.yml
> > @@ -19,11 +19,8 @@ local_conf_header:
> >      CIP_IMAGE_OPTIONS:append = " swupdate.inc"
> >  
> >    wic-swu: |
> > -    IMAGE_CLASSES += "squashfs"
> > -    IMAGE_TYPEDEP:wic += "squashfs"
> > -    IMAGE_FSTYPES = "wic"
> > +    IMAGE_FSTYPES += "swu"
> >      WKS_FILE ?= "${MACHINE}-${SWUPDATE_BOOTLOADER}.wks.in"
> >      INITRAMFS_INSTALL:append = " initramfs-squashfs-hook"
> > -    WIC_DEPLOY_PARTITIONS = "1"
> 
> Why can we already drop this at this point?

Good catch. That has to move into the next commit.

Felix

> 
> >      ABROOTFS_PART_UUID_A ?= "fedcba98-7654-3210-cafe-5e0710000001"
> >      ABROOTFS_PART_UUID_B ?= "fedcba98-7654-3210-cafe-5e0710000002"
> > diff --git a/recipes-core/images/files/sw-description.tmpl
> > b/recipes-core/images/swu/sw-description.tmpl
> > similarity index 100%
> > rename from recipes-core/images/files/sw-description.tmpl
> > rename to recipes-core/images/swu/sw-description.tmpl
> > diff --git a/recipes-core/images/swupdate.inc b/recipes-
> > core/images/swupdate.inc
> > index ee893dd..f4f5c42 100644
> > --- a/recipes-core/images/swupdate.inc
> > +++ b/recipes-core/images/swupdate.inc
> > @@ -10,26 +10,11 @@
> >  #
> >  
> >  inherit image_uuid
> > -inherit swupdate
> >  inherit read-only-rootfs
> >  
> >  IMAGE_INSTALL += " swupdate"
> >  IMAGE_INSTALL += " swupdate-handler-roundrobin"
> >  
> > -ROOTFS_PARTITION_NAME = "${IMAGE_FULLNAME}.wic.p4.gz"
> > -
> > -FILESEXTRAPATHS:prepend := "${THISDIR}/files:"
> > -
> > -SRC_URI += "file://sw-description.tmpl"
> > -TEMPLATE_FILES += "sw-description.tmpl"
> > -
> > -do_transform_template[vardeps] += "TARGET_IMAGE_UUID"
> > -addtask do_transform_template before do_swupdate_binary after
> > do_generate_image_uuid
> > -
> > -TEMPLATE_VARS += "ROOTFS_PARTITION_NAME TARGET_IMAGE_UUID
> > ABROOTFS_PART_UUID_A ABROOTFS_PART_UUID_B"
> > -
> > -SWU_ADDITIONAL_FILES += "linux.efi ${ROOTFS_PARTITION_NAME}"
> > -
> >  python() {
> >      for u in ['A', 'B']:
> >          if not d.getVar('ABROOTFS_PART_UUID_' + u):
> 
> Jan
>