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

[Henning Schild <henning.schild@siemens.com>]

Am Thu, 25 Jun 2020 21:23:54 +0200
schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:

> Am Thu, 25 Jun 2020 20:43:07 +0200
> schrieb "[ext] Henning Schild" <henning.schild@siemens.com>:
> 
> > Am Thu, 25 Jun 2020 19:24:30 +0200
> > schrieb 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
> > >     }    
> > 
> > If /etc/os-release goes into the initrd we have the issue with
> > image_postprocess_mark in isar, a valid reason for a merge.  
> 
> But that is not the case. You re-create the initrd in your layer so it
> might just be your problem! How about we discuss the IMAGE_UUID
> upstream together with the reordering?

Or do we need the "update-initramfs -u" for image_postprocess_mark?
If so that should be done as a patch in this queue, you will get your
reordering for IMAGE_UUID in your layer if you decide to keep that
downstream.

Henning

> Henning
> 
> > > > 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.    
> > 
> > Ok, sounds fair.
> >    
> > > 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.    
> > 
> > I just worried about people abusing postprocess for changes that
> > should be packages instead. Thanks for going into detail!
> > 
> > ACK.
> > 
> > Henning
> >   
>