[PATCH] meta: add quality check helper to image class

[PATCH] meta: add quality check helper to image class

[Henning Schild]

From: Henning Schild <henning.schild@siemens.com>

Content of rootfs should ideally all be coming from packages and their
hooks. POSTPROCESSing and custom tasks should be handled with a lot of
care and avoided where possible.

This commit introduces a quality check task to help devs not
accidentially abuse POSTPROCESS, while allowing them to add files to an
ignore list.

Signed-off-by: Henning Schild <henning.schild@siemens.com>
---
 meta/classes/image.bbclass | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index d44298bcdd7c..a0d44489d5bb 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -231,5 +231,33 @@ EOSUDO
 }
 addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
 
+ROOTFS_QA_FIND_ARGS ?= ""
+
+do_rootfs_quality_check() {
+    rootfs_install_stamp=$( ls -1 "${STAMP}".do_rootfs_install.* | head -1 )
+    test -f $rootfs_install_stamp
+
+    args="$ROOTFS_QA_FIND_ARGS"
+    # rootfs_finalize chroot-setup.sh
+    args="${args} ! -path ${ROOTFSDIR}/var/lib/dpkg/diversions"
+    for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
+        case "${cmd}" in
+	    image_postprocess_mark)
+	        args="${args} ! -path ${ROOTFSDIR}/etc/os-release";;
+	    image_postprocess_machine_id)
+	        args="${args} ! -path ${ROOTFSDIR}/etc/machine-id";;
+	esac
+    done
+    found=$( sudo find ${ROOTFSDIR} -type f -newer $rootfs_install_stamp $args )
+    if [ -n "$found" ]; then
+        bbwarn "Files changed after package install. The following files seem"
+	bbwarn "to have changed where they probably should not have."
+	bbwarn "You might have a custom task or writing POSTPROCESS function."
+	bbwarn "$found"
+    fi
+}
+
+addtask rootfs_quality_check after do_rootfs_finalize before do_rootfs
+
 # Last so that the image type can overwrite tasks if needed
 inherit ${IMAGE_FSTYPES}
-- 
2.34.1

Re: [PATCH] meta: add quality check helper to image class

[Henning Schild]

Hi all,

this is inspired by the discussion around "[RFC PATCH] image: Reorder
do_copy_boot_files task".

Early tests suggest that this works well. Everybody might want to try
this on their most hacky layer.

I could envision future QA checks like ... do we find any files that do
not belong to packages but live in package manager locations.

regards,
Henning

Am Mon, 28 Feb 2022 15:59:18 +0100
schrieb Henning Schild <work@hennsch.de>:

> From: Henning Schild <henning.schild@siemens.com>
> 
> Content of rootfs should ideally all be coming from packages and their
> hooks. POSTPROCESSing and custom tasks should be handled with a lot of
> care and avoided where possible.
> 
> This commit introduces a quality check task to help devs not
> accidentially abuse POSTPROCESS, while allowing them to add files to
> an ignore list.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/classes/image.bbclass | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index d44298bcdd7c..a0d44489d5bb 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -231,5 +231,33 @@ EOSUDO
>  }
>  addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
>  
> +ROOTFS_QA_FIND_ARGS ?= ""
> +
> +do_rootfs_quality_check() {
> +    rootfs_install_stamp=$( ls -1 "${STAMP}".do_rootfs_install.* |
> head -1 )
> +    test -f $rootfs_install_stamp
> +
> +    args="$ROOTFS_QA_FIND_ARGS"
> +    # rootfs_finalize chroot-setup.sh
> +    args="${args} ! -path ${ROOTFSDIR}/var/lib/dpkg/diversions"
> +    for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
> +        case "${cmd}" in
> +	    image_postprocess_mark)
> +	        args="${args} ! -path ${ROOTFSDIR}/etc/os-release";;
> +	    image_postprocess_machine_id)
> +	        args="${args} ! -path ${ROOTFSDIR}/etc/machine-id";;
> +	esac
> +    done
> +    found=$( sudo find ${ROOTFSDIR} -type f -newer
> $rootfs_install_stamp $args )
> +    if [ -n "$found" ]; then
> +        bbwarn "Files changed after package install. The following
> files seem"
> +	bbwarn "to have changed where they probably should not have."
> +	bbwarn "You might have a custom task or writing POSTPROCESS
> function."
> +	bbwarn "$found"
> +    fi
> +}
> +
> +addtask rootfs_quality_check after do_rootfs_finalize before
> do_rootfs +
>  # Last so that the image type can overwrite tasks if needed
>  inherit ${IMAGE_FSTYPES}

Re: [PATCH] meta: add quality check helper to image class

[vijai kumar]

Hi Henning,

On Mon, Feb 28, 2022 at 8:32 PM Henning Schild
<henning.schild@siemens.com> wrote:
>
> Hi all,
>
> this is inspired by the discussion around "[RFC PATCH] image: Reorder
> do_copy_boot_files task".
>
> Early tests suggest that this works well. Everybody might want to try
> this on their most hacky layer.

Even in plain ISAR the rpi target complains. We made some changes to
fstab and other files as part of DISTRO_CONFIG_SCRIPT[1].

As the name implies I would not be surprised if a downstream user
creates a simple distro config script to reconfigure plymouth to have
a different theme for that distro.
If everything except certain modifications should come from a package,
then obviously this has to go away.


WARNING: Files changed after package install. The following files seem
WARNING: to have changed where they probably should not have.
WARNING: You might have a custom task or writing POSTPROCESS function.
WARNING: /home/gomti/vijai_workspace/isar-upstream/isar/build/tmp/work/raspios-bullseye-arm64/isar-image-base-rpi-arm64-v8-rpi-sdimg/1.0-r0/rootfs/boot/cmdline.txt
/home/gomti/vijai_workspace/isar-upstream/isar/build/tmp/work/raspios-bullseye-arm64/isar-image-base-rpi-arm64-v8-rpi-sdimg/1.0-r0/rootfs/boot/config.txt
/home/gomti/vijai_workspace/isar-upstream/isar/build/tmp/work/raspios-bullseye-arm64/isar-image-base-rpi-arm64-v8-rpi-sdimg/1.0-r0/rootfs/etc/fstab

[1] https://github.com/ilbers/isar/blob/next/meta-isar/conf/distro/raspios-configscript.sh

Thanks,
VIjai Kumar K

>
> I could envision future QA checks like ... do we find any files that do
> not belong to packages but live in package manager locations.
>
> regards,
> Henning
>
> Am Mon, 28 Feb 2022 15:59:18 +0100
> schrieb Henning Schild <work@hennsch.de>:
>
> > From: Henning Schild <henning.schild@siemens.com>
> >
> > Content of rootfs should ideally all be coming from packages and their
> > hooks. POSTPROCESSing and custom tasks should be handled with a lot of
> > care and avoided where possible.
> >
> > This commit introduces a quality check task to help devs not
> > accidentially abuse POSTPROCESS, while allowing them to add files to
> > an ignore list.
> >
> > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > ---
> >  meta/classes/image.bbclass | 28 ++++++++++++++++++++++++++++
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> > index d44298bcdd7c..a0d44489d5bb 100644
> > --- a/meta/classes/image.bbclass
> > +++ b/meta/classes/image.bbclass
> > @@ -231,5 +231,33 @@ EOSUDO
> >  }
> >  addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
> >
> > +ROOTFS_QA_FIND_ARGS ?= ""
> > +
> > +do_rootfs_quality_check() {
> > +    rootfs_install_stamp=$( ls -1 "${STAMP}".do_rootfs_install.* |
> > head -1 )
> > +    test -f $rootfs_install_stamp
> > +
> > +    args="$ROOTFS_QA_FIND_ARGS"
> > +    # rootfs_finalize chroot-setup.sh
> > +    args="${args} ! -path ${ROOTFSDIR}/var/lib/dpkg/diversions"
> > +    for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
> > +        case "${cmd}" in
> > +         image_postprocess_mark)
> > +             args="${args} ! -path ${ROOTFSDIR}/etc/os-release";;
> > +         image_postprocess_machine_id)
> > +             args="${args} ! -path ${ROOTFSDIR}/etc/machine-id";;
> > +     esac
> > +    done
> > +    found=$( sudo find ${ROOTFSDIR} -type f -newer
> > $rootfs_install_stamp $args )
> > +    if [ -n "$found" ]; then
> > +        bbwarn "Files changed after package install. The following
> > files seem"
> > +     bbwarn "to have changed where they probably should not have."
> > +     bbwarn "You might have a custom task or writing POSTPROCESS
> > function."
> > +     bbwarn "$found"
> > +    fi
> > +}
> > +
> > +addtask rootfs_quality_check after do_rootfs_finalize before
> > do_rootfs +
> >  # Last so that the image type can overwrite tasks if needed
> >  inherit ${IMAGE_FSTYPES}
>
> --
> You received this message because you are subscribed to the Google Groups "isar-users" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/20220228160210.6ca63409%40md1za8fc.ad001.siemens.net.

Re: [PATCH] meta: add quality check helper to image class

[Henning Schild]

Am Tue, 1 Mar 2022 15:58:53 +0530
schrieb vijai kumar <vijaikumar.kanagarajan@gmail.com>:

> Hi Henning,
> 
> On Mon, Feb 28, 2022 at 8:32 PM Henning Schild
> <henning.schild@siemens.com> wrote:
> >
> > Hi all,
> >
> > this is inspired by the discussion around "[RFC PATCH] image:
> > Reorder do_copy_boot_files task".
> >
> > Early tests suggest that this works well. Everybody might want to
> > try this on their most hacky layer.  
> 
> Even in plain ISAR the rpi target complains. We made some changes to
> fstab and other files as part of DISTRO_CONFIG_SCRIPT[1].

I will look into that. In fact i never really understood why we do not
use wic, and why we do not use debian ... i guess the cleanest way
would be to move to wic.

But hey for the given target the three files are well understood, so we
add them to the ignore list and our warning worked. Before doing so we
can carefully think if we want to ignore or find a better solution.

In the concrete example the configure script steps is something that
smells a lot like "imaging" and not rootfs building.

> As the name implies I would not be surprised if a downstream user
> creates a simple distro config script to reconfigure plymouth to have
> a different theme for that distro.
> If everything except certain modifications should come from a package,
> then obviously this has to go away.

Yes, the warnings should go away in Isar. But i would conclude the
patch already worked well showing "bad style" in isar itself.

But i would suggest to merge this if people like it, no matter if that
will cause warning in isar itself. Those can be fixed later.

Henning

> 
> WARNING: Files changed after package install. The following files seem
> WARNING: to have changed where they probably should not have.
> WARNING: You might have a custom task or writing POSTPROCESS function.
> WARNING:
> /home/gomti/vijai_workspace/isar-upstream/isar/build/tmp/work/raspios-bullseye-arm64/isar-image-base-rpi-arm64-v8-rpi-sdimg/1.0-r0/rootfs/boot/cmdline.txt
> /home/gomti/vijai_workspace/isar-upstream/isar/build/tmp/work/raspios-bullseye-arm64/isar-image-base-rpi-arm64-v8-rpi-sdimg/1.0-r0/rootfs/boot/config.txt
> /home/gomti/vijai_workspace/isar-upstream/isar/build/tmp/work/raspios-bullseye-arm64/isar-image-base-rpi-arm64-v8-rpi-sdimg/1.0-r0/rootfs/etc/fstab
> 
> [1]
> https://github.com/ilbers/isar/blob/next/meta-isar/conf/distro/raspios-configscript.sh
> 
> Thanks,
> VIjai Kumar K
> 
> >
> > I could envision future QA checks like ... do we find any files
> > that do not belong to packages but live in package manager
> > locations.
> >
> > regards,
> > Henning
> >
> > Am Mon, 28 Feb 2022 15:59:18 +0100
> > schrieb Henning Schild <work@hennsch.de>:
> >  
> > > From: Henning Schild <henning.schild@siemens.com>
> > >
> > > Content of rootfs should ideally all be coming from packages and
> > > their hooks. POSTPROCESSing and custom tasks should be handled
> > > with a lot of care and avoided where possible.
> > >
> > > This commit introduces a quality check task to help devs not
> > > accidentially abuse POSTPROCESS, while allowing them to add files
> > > to an ignore list.
> > >
> > > Signed-off-by: Henning Schild <henning.schild@siemens.com>
> > > ---
> > >  meta/classes/image.bbclass | 28 ++++++++++++++++++++++++++++
> > >  1 file changed, 28 insertions(+)
> > >
> > > diff --git a/meta/classes/image.bbclass
> > > b/meta/classes/image.bbclass index d44298bcdd7c..a0d44489d5bb
> > > 100644 --- a/meta/classes/image.bbclass
> > > +++ b/meta/classes/image.bbclass
> > > @@ -231,5 +231,33 @@ EOSUDO
> > >  }
> > >  addtask rootfs_finalize before do_rootfs after
> > > do_rootfs_postprocess
> > >
> > > +ROOTFS_QA_FIND_ARGS ?= ""
> > > +
> > > +do_rootfs_quality_check() {
> > > +    rootfs_install_stamp=$( ls -1 "${STAMP}".do_rootfs_install.*
> > > | head -1 )
> > > +    test -f $rootfs_install_stamp
> > > +
> > > +    args="$ROOTFS_QA_FIND_ARGS"
> > > +    # rootfs_finalize chroot-setup.sh
> > > +    args="${args} ! -path ${ROOTFSDIR}/var/lib/dpkg/diversions"
> > > +    for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
> > > +        case "${cmd}" in
> > > +         image_postprocess_mark)
> > > +             args="${args} ! -path ${ROOTFSDIR}/etc/os-release";;
> > > +         image_postprocess_machine_id)
> > > +             args="${args} ! -path ${ROOTFSDIR}/etc/machine-id";;
> > > +     esac
> > > +    done
> > > +    found=$( sudo find ${ROOTFSDIR} -type f -newer
> > > $rootfs_install_stamp $args )
> > > +    if [ -n "$found" ]; then
> > > +        bbwarn "Files changed after package install. The
> > > following files seem"
> > > +     bbwarn "to have changed where they probably should not
> > > have."
> > > +     bbwarn "You might have a custom task or writing POSTPROCESS
> > > function."
> > > +     bbwarn "$found"
> > > +    fi
> > > +}
> > > +
> > > +addtask rootfs_quality_check after do_rootfs_finalize before
> > > do_rootfs +
> > >  # Last so that the image type can overwrite tasks if needed
> > >  inherit ${IMAGE_FSTYPES}  
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "isar-users" group. To unsubscribe from this group and stop
> > receiving emails from it, send an email to
> > isar-users+unsubscribe@googlegroups.com. To view this discussion on
> > the web visit
> > https://groups.google.com/d/msgid/isar-users/20220228160210.6ca63409%40md1za8fc.ad001.siemens.net.
> >  
> 

Re: [PATCH] meta: add quality check helper to image class

[Henning Schild]

Am Mon, 28 Feb 2022 15:57:25 +0100
schrieb Henning Schild <work@hennsch.de>:

> From: Henning Schild <henning.schild@siemens.com>
> 
> Content of rootfs should ideally all be coming from packages and their
> hooks. POSTPROCESSing and custom tasks should be handled with a lot of
> care and avoided where possible.
> 
> This commit introduces a quality check task to help devs not
> accidentially abuse POSTPROCESS, while allowing them to add files to
> an ignore list.
> 
> Signed-off-by: Henning Schild <henning.schild@siemens.com>
> ---
>  meta/classes/image.bbclass | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index d44298bcdd7c..a0d44489d5bb 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -231,5 +231,33 @@ EOSUDO
>  }
>  addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
>  
> +ROOTFS_QA_FIND_ARGS ?= ""
> +
> +do_rootfs_quality_check() {
> +    rootfs_install_stamp=$( ls -1 "${STAMP}".do_rootfs_install.* |
> head -1 )

Will need to take setscene/sstate into account.

do_rootfs_install.* -> do_rootfs_install*

> +    test -f $rootfs_install_stamp

needs quoting because "test -f" exits 0 while 'test -f ""' does not

Henning

> +
> +    args="$ROOTFS_QA_FIND_ARGS"
> +    # rootfs_finalize chroot-setup.sh
> +    args="${args} ! -path ${ROOTFSDIR}/var/lib/dpkg/diversions"
> +    for cmd in ${ROOTFS_POSTPROCESS_COMMAND}; do
> +        case "${cmd}" in
> +	    image_postprocess_mark)
> +	        args="${args} ! -path ${ROOTFSDIR}/etc/os-release";;
> +	    image_postprocess_machine_id)
> +	        args="${args} ! -path ${ROOTFSDIR}/etc/machine-id";;
> +	esac
> +    done
> +    found=$( sudo find ${ROOTFSDIR} -type f -newer
> $rootfs_install_stamp $args )
> +    if [ -n "$found" ]; then
> +        bbwarn "Files changed after package install. The following
> files seem"
> +	bbwarn "to have changed where they probably should not have."
> +	bbwarn "You might have a custom task or writing POSTPROCESS
> function."
> +	bbwarn "$found"
> +    fi
> +}
> +
> +addtask rootfs_quality_check after do_rootfs_finalize before
> do_rootfs +
>  # Last so that the image type can overwrite tasks if needed
>  inherit ${IMAGE_FSTYPES}