[isar] rootfs: Add new ROOTFS_FEATURE 'slimfy' to minimize the footprint

[isar] rootfs: Add new ROOTFS_FEATURE 'slimfy' to minimize the footprint

[venkata.pyla]

From: venkata pyla <venkata.pyla@toshiba-tsip.com>

 This Adds new ROOTFS_FEATURE 'slimify' that deletes unnecessary files
 in the rootfs and contributes to minimal footprint in the rootfs and
 also avoids the reproducible failures due to non-deterministic data in
 the log files and temporary files.

 It deletes the following files
  - /var/log/*
  - /tmp/*

 To enable this feature
   ROOTFS_FEATURE += slimify

Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
---
 meta/classes/rootfs.bbclass | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index f9151c5..d01a9d1 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}
+# 'slimify'            - delete unnecessary files in rootfs like /var/log/*, /tmp/*
 ROOTFS_FEATURES ?= ""
 
 ROOTFS_APT_ARGS="install --yes -o Debug::pkgProblemResolver=yes"
@@ -229,6 +230,12 @@ rootfs_export_dpkg_status() {
        '${ROOTFS_DPKGSTATUS_DEPLOY_DIR}'/'${PF}'.dpkg_status
 }
 
+ROOTFS_POSTPROCESS_COMMAND += "${@bb.utils.contains('ROOTFS_FEATURES', 'slimify', 'rootfs_slimify', '', d)}"
+rootfs_slimify() {
+    sudo rm -rf "${ROOTFSDIR}/var/log/"*
+    sudo rm -rf "${ROOTFSDIR}/tmp/"*
+}
+
 do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
 python do_rootfs_postprocess() {
     # Take care that its correctly mounted:
-- 
2.20.1

Re: [isar] rootfs: Add new ROOTFS_FEATURE 'slimfy' to minimize the footprint

[Jan Kiszka]

On 06.09.21 11:45, venkata.pyla@toshiba-tsip.com wrote:
> From: venkata pyla <venkata.pyla@toshiba-tsip.com>
> 
>  This Adds new ROOTFS_FEATURE 'slimify' that deletes unnecessary files
>  in the rootfs and contributes to minimal footprint in the rootfs and
>  also avoids the reproducible failures due to non-deterministic data in
>  the log files and temporary files.
> 
>  It deletes the following files
>   - /var/log/*
>   - /tmp/*
> 
>  To enable this feature
>    ROOTFS_FEATURE += slimify
> 
> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> ---
>  meta/classes/rootfs.bbclass | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index f9151c5..d01a9d1 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}
> +# 'slimify'            - delete unnecessary files in rootfs like /var/log/*, /tmp/*

/tmp should be emptied unconditionally.

I'm also wondering if we should rather permit to preserving logs for
debugging purposes and make deletion the default as well.

Jan

>  ROOTFS_FEATURES ?= ""
>  
>  ROOTFS_APT_ARGS="install --yes -o Debug::pkgProblemResolver=yes"
> @@ -229,6 +230,12 @@ rootfs_export_dpkg_status() {
>         '${ROOTFS_DPKGSTATUS_DEPLOY_DIR}'/'${PF}'.dpkg_status
>  }
>  
> +ROOTFS_POSTPROCESS_COMMAND += "${@bb.utils.contains('ROOTFS_FEATURES', 'slimify', 'rootfs_slimify', '', d)}"
> +rootfs_slimify() {
> +    sudo rm -rf "${ROOTFSDIR}/var/log/"*
> +    sudo rm -rf "${ROOTFSDIR}/tmp/"*
> +}
> +
>  do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
>  python do_rootfs_postprocess() {
>      # Take care that its correctly mounted:
> 

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

Re: [isar] rootfs: Add new ROOTFS_FEATURE 'slimfy' to minimize the footprint

[Henning Schild]

IMHO no need to model that as a feature. Removing "temporary" files
that do not belong to packages is something everybody would expect
anyways.

Am Mon, 6 Sep 2021 15:15:10 +0530
schrieb <venkata.pyla@toshiba-tsip.com>:

> From: venkata pyla <venkata.pyla@toshiba-tsip.com>
> 
>  This Adds new ROOTFS_FEATURE 'slimify' that deletes unnecessary files
>  in the rootfs and contributes to minimal footprint in the rootfs and
>  also avoids the reproducible failures due to non-deterministic data
> in the log files and temporary files.
> 
>  It deletes the following files
>   - /var/log/*
>   - /tmp/*
> 
>  To enable this feature
>    ROOTFS_FEATURE += slimify
> 
> Signed-off-by: venkata pyla <venkata.pyla@toshiba-tsip.com>
> ---
>  meta/classes/rootfs.bbclass | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
> index f9151c5..d01a9d1 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} +#
> 'slimify'            - delete unnecessary files in rootfs like
> /var/log/*, /tmp/* ROOTFS_FEATURES ?= "" 
>  ROOTFS_APT_ARGS="install --yes -o Debug::pkgProblemResolver=yes"
> @@ -229,6 +230,12 @@ rootfs_export_dpkg_status() {
>         '${ROOTFS_DPKGSTATUS_DEPLOY_DIR}'/'${PF}'.dpkg_status
>  }
>  
> +ROOTFS_POSTPROCESS_COMMAND +=
> "${@bb.utils.contains('ROOTFS_FEATURES', 'slimify', 'rootfs_slimify',
> '', d)}" +rootfs_slimify() {
> +    sudo rm -rf "${ROOTFSDIR}/var/log/"*

This is going too far because you are removing package content i.e.
  dpkg -S /var/log/apt
  apt: /var/log/apt

If packages ship directories or maybe even files we can not simply
delete those.

And overall the change is not going far enough. Because i.e. /var/lib
and /var/cache remain as problems.
Instead of listing a bunch of directories we should really go and ask
the package manager which files are without owner.

Henning


> +    sudo rm -rf "${ROOTFSDIR}/tmp/"*
> +}
> +
>  do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
>  python do_rootfs_postprocess() {
>      # Take care that its correctly mounted:

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

[venkata.pyla]

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

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 {} ';'
+}
+
 ROOTFS_POSTPROCESS_COMMAND += "${@bb.utils.contains('ROOTFS_FEATURES', 'generate-manifest', 'rootfs_generate_manifest', '', d)}"
 rootfs_generate_manifest () {
     mkdir -p ${ROOTFS_MANIFEST_DEPLOY_DIR}
-- 
2.20.1

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

[Jan Kiszka]

On 01.10.21 16:37, venkata.pyla@toshiba-tsip.com wrote:
> 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
> 

Style: Do not indent the commit body. I think this will survive "git am"
and will make the result look inconsistent.

> 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 {} ';'
> +}
> +
>  ROOTFS_POSTPROCESS_COMMAND += "${@bb.utils.contains('ROOTFS_FEATURES', 'generate-manifest', 'rootfs_generate_manifest', '', d)}"
>  rootfs_generate_manifest () {
>      mkdir -p ${ROOTFS_MANIFEST_DEPLOY_DIR}
> 

Is there any reason why this feature should be default-off? If not, I
would also add it to image.bbclass, ROOTFS_FEATURES.

Jan

-- 
Siemens AG, T RDA IOT
Corporate Competence Center Embedded Linux

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

[Venkata.Pyla]

Hi Jan,

Thanks for the review, please find my inline comments.

>-----Original Message-----
>From: Jan Kiszka <jan.kiszka@siemens.com>
>Sent: 04 October 2021 15:26
>To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>; isar-
>users@googlegroups.com
>Cc: henning.schild@siemens.com
>Subject: Re: [PATCH v2] rootfs: clean package log files that are not owned by
>packages
>
>On 01.10.21 16:37, venkata.pyla@toshiba-tsip.com wrote:
>> 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
>>
>
>Style: Do not indent the commit body. I think this will survive "git am"
>and will make the result look inconsistent.

Thanks for correcting me, I will send the patch again with corrected comments.  

>
>> 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 {} ';'
>> +}
>> +
>>  ROOTFS_POSTPROCESS_COMMAND +=
>"${@bb.utils.contains('ROOTFS_FEATURES', 'generate-manifest',
>'rootfs_generate_manifest', '', d)}"
>>  rootfs_generate_manifest () {
>>      mkdir -p ${ROOTFS_MANIFEST_DEPLOY_DIR}
>>
>
>Is there any reason why this feature should be default-off? If not, I would also
>add it to image.bbclass, ROOTFS_FEATURES.

No reasons to set it to off, I taught to enable this feature where it is required (isar-cip-core),
But, It is good idea to enable in upstream also.

If no objection I can send the patch with enabling this feature in image.bbclass

>
>Jan
>
>--
>Siemens AG, T RDA IOT
>Corporate Competence Center Embedded Linux

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

[Henning Schild]

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 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.

> 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)

Henning

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

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

[Henning Schild]

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

> Hi Jan,
> 
> Thanks for the review, please find my inline comments.
> 
> >-----Original Message-----
> >From: Jan Kiszka <jan.kiszka@siemens.com>
> >Sent: 04 October 2021 15:26
> >To: pyla venkata(ＴＳＩＰ) <Venkata.Pyla@toshiba-tsip.com>; isar-
> >users@googlegroups.com
> >Cc: henning.schild@siemens.com
> >Subject: Re: [PATCH v2] rootfs: clean package log files that are not
> >owned by packages
> >
> >On 01.10.21 16:37, venkata.pyla@toshiba-tsip.com wrote:  
> >> 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
> >>  
> >
> >Style: Do not indent the commit body. I think this will survive "git
> >am" and will make the result look inconsistent.  
> 
> Thanks for correcting me, I will send the patch again with corrected
> comments.  
> 
> >  
> >> 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 {} ';'
> >> +}
> >> +
> >>  ROOTFS_POSTPROCESS_COMMAND +=  
> >"${@bb.utils.contains('ROOTFS_FEATURES', 'generate-manifest',
> >'rootfs_generate_manifest', '', d)}"  
> >>  rootfs_generate_manifest () {
> >>      mkdir -p ${ROOTFS_MANIFEST_DEPLOY_DIR}
> >>  
> >
> >Is there any reason why this feature should be default-off? If not,
> >I would also add it to image.bbclass, ROOTFS_FEATURES.  
> 
> No reasons to set it to off, I taught to enable this feature where it
> is required (isar-cip-core), But, It is good idea to enable in
> upstream also.
> 
> If no objection I can send the patch with enabling this feature in
> image.bbclass

Seems to boil down to what i suggested ... even not making it optional
at all as long as we do not know of any reason to not clean up.
Saving space is someone we can assume anyone will want. Reproducability
is just a nice side effect ;)

Henning

> >
> >Jan
> >
> >--
> >Siemens AG, T RDA IOT
> >Corporate Competence Center Embedded Linux  

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

[Venkata.Pyla]

>-----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.

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

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

[Henning Schild]

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}  
> 

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

[Venkata.Pyla]

>-----Original Message-----
>From: Henning Schild <henning.schild@siemens.com>
>Sent: 04 October 2021 19:19
>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 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.

The dpkg -S should search even if it is empty folder and owned by packages, 
Also if the applications are expecting them even if it is not part of package list contents, I think it should be an application bug.
Anyway keeping them doesn’t give any difference, so If no objections I will modify it.

I will resend the patch with the below changes, kindly let me know if there are more review comments.
* correct the commit message by removing extra leading space characters
* Use "type -f" to delete only files in the log folder
* Set this feature to on as default in image.bbclass

Thanks.

>
>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}
>>

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

[Henning Schild]

Am Mon, 4 Oct 2021 15:07:04 +0000
schrieb <Venkata.Pyla@toshiba-tsip.com>:

> >-----Original Message-----
> >From: Henning Schild <henning.schild@siemens.com>
> >Sent: 04 October 2021 19:19
> >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 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.  
> 
> The dpkg -S should search even if it is empty folder and owned by
> packages, Also if the applications are expecting them even if it is
> not part of package list contents, I think it should be an
> application bug. Anyway keeping them doesn’t give any difference, so
> If no objections I will modify it.
> 
> I will resend the patch with the below changes, kindly let me know if
> there are more review comments.
> * correct the commit message by removing extra leading space
> characters
> * Use "type -f" to delete only files in the log folder

I am also not sure that is needed. I first thought a debian package was
not allowed to contain an empty folder ... like git or a gentoo
package. I do not feel strong about the "type f" and the "rm -f".

Henning

> * Set this feature to on as default in image.bbclass
> 
> Thanks.
> 
> >
> >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}  
> >>