Re: [PATCH] image: Run copy_boot_files after rootfs postprocessing

[Harald Seiler <hws@denx.de>]

Hello Henning,

On Thu, 2020-06-25 at 19:02 +0200, Jan Kiszka wrote:
> On 25.06.20 18:48, [ext] Henning Schild wrote:
> > Hi Harald,
> > 
> > can you elaborate on those cases? The postprocessing is hacky, if the
> > problem is coming from your layer you should probably keep this patch
> > in you layer.
> 
> Basically do_generate_image_uuid from 
> https://lore.kernel.org/cip-dev/20200625141015.31719-4-Quirin.Gylstorff@siemens.com/T/#u, 
> just modeled as post-processing hook, rather than a task.

For reference, this is the exact code:

    ROOTFS_POSTPROCESS_COMMAND =+ "image_postprocess_generate_uuid"
    image_postprocess_generate_uuid() {
        sudo sed -i '/^IMAGE_UUID=.*/d' '${IMAGE_ROOTFS}/etc/os-release'
        echo "IMAGE_UUID=\"${IMAGE_UUID}\"" | \
            sudo tee -a '${IMAGE_ROOTFS}/etc/os-release'

        sudo -E chroot '${ROOTFSDIR}' \
            update-initramfs -u
    }

> Jan
> 
> > Maybe you can point out an issue in isar itself, or explain how you got
> > into this situation? We can then see if your change is generic enough
> > for upstream. You could also provide the error-case from your layer as
> > an upstream feature, if that is generic enough.

I think this patch addresses an issue in isar itself.  There is no reason
for copy_boot_files() to run before the postprocessing does.  I've checked
through the git history and the reason this relationship was introduced
was a bigger refactor of the task dependency chain.  It does not seem to
be intentionally this way from what I can tell.

The other way around makes more sense, in my opinion.  As stated in the
commit message, postprocessing might do an update to the initramfs (as
seen above) and this change needs to be reflected in the deployed
initramfs as well, instead of silently only living in the version that is
part of the rootfs.

I also checked all existing postprocessing commands and did not see any
that assume to be run after the boot files have been deployed.

-- 
Harald

> > Henning
> > 
> > Am Thu, 25 Jun 2020 17:33:51 +0200
> > schrieb Harald Seiler <hws@denx.de>:
> > 
> > > In some cases, postprocessing might trigger an update of the initramfs
> > > which would not appear in the deployed version.  Fix this by running
> > > copy_boot_files only after all postprocessing completed.
> > > 
> > > Signed-off-by: Harald Seiler <hws@denx.de>
> > > ---
> > >   meta/classes/image.bbclass | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > > index 0150f2718573..21d634a8f34f 100644
> > > --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -153,7 +153,7 @@ do_copy_boot_files() {
> > >           cp -f "$dtb" "${DEPLOY_DIR_IMAGE}/"
> > >       done
> > >   }
> > > -addtask copy_boot_files before do_rootfs_postprocess after
> > > do_rootfs_install +addtask copy_boot_files before do_rootfs after
> > > do_rootfs_postprocess
> > >   python do_image_tools() {
> > >       """Virtual task"""