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

[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?

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
>