Re: [PATCH v2] rootfs: clean package log files that are not owned by packages

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

Am Mon, 4 Oct 2021 12:51:43 +0000
schrieb <Venkata.Pyla@toshiba-tsip.com>:

> >-----Original Message-----
> >From: Henning Schild <henning.schild@siemens.com>
> >Sent: 04 October 2021 17:35
> >To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>
> >Cc: isar-users@googlegroups.com; jan.kiszka@siemens.com
> >Subject: Re: [PATCH v2] rootfs: clean package log files that are not
> >owned by packages
> >
> >Am Fri, 1 Oct 2021 20:07:48 +0530
> >schrieb <venkata.pyla@toshiba-tsip.com>:
> >  
> >> From: venkata pyla <venkata.pyla@toshiba-tsip.com>
> >>
> >>  /var/log/* files that are created during build stage and not owned
> >> by any package are not neccessary to be present in rootfs image, as
> >> these log files adds additional size to rootfs image, and also it
> >> create problems for reproducible build functionality.
> >>
> >>  so this ROOTFS feature 'clean-log-files' should help to clean the
> >> log files when it is enalbed, disable it if we need the log files
> >> for debugging purpose.
> >>
> >>  ROOTFS_FEATURE += clean-log-files  
> >
> >The two leading spaces of the commit message look a little weird.
> >Like you are not using git-format-patch or git-send-email correctly.
> >Or like you are using a funny editor for writing your commit
> >messages.  
> 
> I will correct it, thanks.
> 
> >
> >I think that adds value but i do not see why we should model it as a
> >feature. It should probably be the "new normal" and we introduce an
> >option once there is a need for a switch.  
> 
> I added it as feature because if we delete the logs file by default
> from the image, if any user want those log files for debugging
> purpose then there will be provision to disable this feature and get
> the log files as earlier
> 
> >  
> >> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> >> ---
> >>  meta/classes/rootfs.bbclass | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)
> >>
> >> diff --git a/meta/classes/rootfs.bbclass
> >> b/meta/classes/rootfs.bbclass index f9151c5..ff0ecad 100644
> >> --- a/meta/classes/rootfs.bbclass
> >> +++ b/meta/classes/rootfs.bbclass
> >> @@ -12,6 +12,7 @@ ROOTFS_PACKAGES ?= ""
> >>  # 'clean-package-cache' - delete package cache from rootfs  #
> >> 'generate-manifest' - generate a package manifest of the rootfs
> >> into ${ROOTFS_MANIFEST_DEPLOY_DIR} # 'export-dpkg-status' - exports
> >> /var/lib/dpkg/status file to ${ROOTFS_DPKGSTATUS_DEPLOY_DIR} +#
> >> 'clean-log-files' - delete log files that are not owned by packages
> >> ROOTFS_FEATURES ?= ""
> >>  ROOTFS_APT_ARGS="install --yes -o Debug::pkgProblemResolver=yes"
> >> @@ -213,6 +214,15 @@ rootfs_postprocess_clean_package_cache() {
> >>      sudo rm -rf "${ROOTFSDIR}/var/lib/apt/lists/"*
> >>  }
> >>
> >> +ROOTFS_POSTPROCESS_COMMAND +=
> >> "${@bb.utils.contains('ROOTFS_FEATURES', 'clean-log-files',
> >> 'rootfs_postprocess_clean_log_files', '', d)}"
> >> +rootfs_postprocess_clean_log_files() {
> >> +    # Delete log files that are not owned by packages
> >> +    sudo -E chroot '${ROOTFSDIR}' \
> >> +        /usr/bin/find /var/log/ \
> >> +        -exec sh -c '! dpkg -S {} > /dev/null 2>&1' ';' \
> >> +        -exec rm -rf {} ';'  
> >
> >I think we should restrict that to "-type f" and switch over to "rm
> >-f" (not -r)
> >  
> 
> If we delete only files then there may be some residue left over as
> empty folders.

Which is exactly what i was aiming for! An empty folder belonging to a
package should be kept ... i think your code keeps it as well. But
empty folders do not hurt too much ... and them not being there might
have an impact on the applications expecting them to exist ... they
might not log or might not even start in the worst case.

I think a proper debian package will contain required empty folders, i
am not sure we should remove empty folders that are seemingly not
needed.

Henning

> >Henning
> >  
> >> +}
> >> +
> >>  ROOTFS_POSTPROCESS_COMMAND +=
> >> "${@bb.utils.contains('ROOTFS_FEATURES', 'generate-manifest',
> >> 'rootfs_generate_manifest', '', d)}" rootfs_generate_manifest () {
> >> mkdir -p ${ROOTFS_MANIFEST_DEPLOY_DIR}  
>