| Message ID | 20211001143748.8711-1-venkata.pyla@toshiba-tsip.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] rootfs: clean package log files that are not owned by packages | expand |
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
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
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}
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
>-----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}
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} >
>-----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} >>
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} > >>
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}