[PATCH 0/4] Postinst logging reimplementation

[PATCH 0/4] Postinst logging reimplementation

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

Goal:
1. Enable postinst logging if 'debug-tweaks' is in IMAGE_FEATURES
2. Make rpm, opkg and dpkg not depend on IMAGE_FEATURES and POSTLOG

Implementation:
1. The run-postinst scripts will log outputs and errors if /etc/.enable_postinst_logging is present.
2. The log location is always /var/log/postinstall.log. Do not allow configuration, because this is
   mainly for debugging purpose and /var/log/postinstall.log is a good place. Making it configurable is not necessary.

The following changes since commit a9f5bf0ed398bf9cb861feaa8b6fefd8645b1d09:

  sanity.bbclass: Attach the missing value to a format string. (2013-05-13 21:54:40 +0300)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib ChenQi/postinst-logging-reimplementation
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/postinst-logging-reimplementation

Chen Qi (4):
  image.bbclass: add postinst_enable_logging
  dpkg: modify the run-postinst script to enable postinst logging
  opkg: modify the run-postinst script to enable postinst logging
  rpm-postinsts.bb: enable postinst logging

 meta/classes/image.bbclass                 |    7 +++++++
 meta/recipes-devtools/dpkg/dpkg.inc        |   10 +++++-----
 meta/recipes-devtools/opkg/opkg.inc        |   10 +++++-----
 meta/recipes-devtools/rpm/rpm-postinsts.bb |   11 ++++++-----
 4 files changed, 23 insertions(+), 15 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] image.bbclass: add postinst_enable_logging

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

Add a function postinst_enable_logging, so that when 'debug-tweaks'
is in IMAGE_FEATURES, we create .enable_postinst_logging under /etc,
which is used by run-postinst scripts to determine whether to log or
not.

[YOCTO #4262]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/image.bbclass |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 2b42e12..fc5e23c 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -175,6 +175,8 @@ MACHINE_POSTPROCESS_COMMAND ?= ""
 ROOTFS_POSTPROCESS_COMMAND_prepend = "run_intercept_scriptlets; "
 # Allow dropbear/openssh to accept logins from accounts with an empty password string if debug-tweaks is enabled
 ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "ssh_allow_empty_password; ", "",d)}'
+# Enable postinst logging if debug-tweaks is enabled
+ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "postinst_enable_logging; ", "",d)}'
 
 # some default locales
 IMAGE_LINGUAS ?= "de-de fr-fr en-gb"
@@ -484,6 +486,11 @@ ssh_allow_empty_password () {
 	fi
 }
 
+# Enable postinst logging if debug-tweaks is enabled
+postinst_enable_logging () {
+	touch ${IMAGE_ROOTFS}/etc/.enable_postinst_logging
+}
+
 # Turn any symbolic /sbin/init link into a file
 remove_init_link () {
 	if [ -h ${IMAGE_ROOTFS}/sbin/init ]; then
-- 
1.7.9.5

Re: [PATCH 1/4] image.bbclass: add postinst_enable_logging

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2030 bytes --]

On Thu, May 16, 2013 at 03:56:14PM +0800, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
> 
> Add a function postinst_enable_logging, so that when 'debug-tweaks'
> is in IMAGE_FEATURES, we create .enable_postinst_logging under /etc,
> which is used by run-postinst scripts to determine whether to log or
> not.
> 
> [YOCTO #4262]
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  meta/classes/image.bbclass |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 2b42e12..fc5e23c 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -175,6 +175,8 @@ MACHINE_POSTPROCESS_COMMAND ?= ""
>  ROOTFS_POSTPROCESS_COMMAND_prepend = "run_intercept_scriptlets; "
>  # Allow dropbear/openssh to accept logins from accounts with an empty password string if debug-tweaks is enabled
>  ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "ssh_allow_empty_password; ", "",d)}'
> +# Enable postinst logging if debug-tweaks is enabled
> +ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "postinst_enable_logging; ", "",d)}'
>  
>  # some default locales
>  IMAGE_LINGUAS ?= "de-de fr-fr en-gb"
> @@ -484,6 +486,11 @@ ssh_allow_empty_password () {
>  	fi
>  }
>  
> +# Enable postinst logging if debug-tweaks is enabled
> +postinst_enable_logging () {

cannot we use better file for this? it will be first hidden file in /etc

> +	touch ${IMAGE_ROOTFS}/etc/.enable_postinst_logging
> +}
> +
>  # Turn any symbolic /sbin/init link into a file
>  remove_init_link () {
>  	if [ -h ${IMAGE_ROOTFS}/sbin/init ]; then
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: [PATCH 1/4] image.bbclass: add postinst_enable_logging

[Saul Wold]

On 05/16/2013 10:56 AM, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
>
> Add a function postinst_enable_logging, so that when 'debug-tweaks'
> is in IMAGE_FEATURES, we create .enable_postinst_logging under /etc,
> which is used by run-postinst scripts to determine whether to log or
> not.
>
> [YOCTO #4262]
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>   meta/classes/image.bbclass |    7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index 2b42e12..fc5e23c 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -175,6 +175,8 @@ MACHINE_POSTPROCESS_COMMAND ?= ""
>   ROOTFS_POSTPROCESS_COMMAND_prepend = "run_intercept_scriptlets; "
>   # Allow dropbear/openssh to accept logins from accounts with an empty password string if debug-tweaks is enabled
>   ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "ssh_allow_empty_password; ", "",d)}'
> +# Enable postinst logging if debug-tweaks is enabled
> +ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", "debug-tweaks", "postinst_enable_logging; ", "",d)}'
>
>   # some default locales
>   IMAGE_LINGUAS ?= "de-de fr-fr en-gb"
> @@ -484,6 +486,11 @@ ssh_allow_empty_password () {
>   	fi
>   }
>
> +# Enable postinst logging if debug-tweaks is enabled
> +postinst_enable_logging () {
> +	touch ${IMAGE_ROOTFS}/etc/.enable_postinst_logging

I do not think this should go here, maybe better to have it in the 
/etc/default dir and named postinst, since that matches what other init
related scripts do and source the file in each postinst script, it 
should also have a standard guard to check existance of the file and 
checks something like a "ENABLED" variable instead?  This could also be 
extended for getting the log file location also.

Sau!


> +}
> +
>   # Turn any symbolic /sbin/init link into a file
>   remove_init_link () {
>   	if [ -h ${IMAGE_ROOTFS}/sbin/init ]; then
>

Re: [PATCH 1/4] image.bbclass: add postinst_enable_logging

[ChenQi]

On 05/17/2013 02:32 AM, Saul Wold wrote:
> On 05/16/2013 10:56 AM, Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> Add a function postinst_enable_logging, so that when 'debug-tweaks'
>> is in IMAGE_FEATURES, we create .enable_postinst_logging under /etc,
>> which is used by run-postinst scripts to determine whether to log or
>> not.
>>
>> [YOCTO #4262]
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>>   meta/classes/image.bbclass |    7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>> index 2b42e12..fc5e23c 100644
>> --- a/meta/classes/image.bbclass
>> +++ b/meta/classes/image.bbclass
>> @@ -175,6 +175,8 @@ MACHINE_POSTPROCESS_COMMAND ?= ""
>>   ROOTFS_POSTPROCESS_COMMAND_prepend = "run_intercept_scriptlets; "
>>   # Allow dropbear/openssh to accept logins from accounts with an 
>> empty password string if debug-tweaks is enabled
>>   ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", 
>> "debug-tweaks", "ssh_allow_empty_password; ", "",d)}'
>> +# Enable postinst logging if debug-tweaks is enabled
>> +ROOTFS_POSTPROCESS_COMMAND += '${@base_contains("IMAGE_FEATURES", 
>> "debug-tweaks", "postinst_enable_logging; ", "",d)}'
>>
>>   # some default locales
>>   IMAGE_LINGUAS ?= "de-de fr-fr en-gb"
>> @@ -484,6 +486,11 @@ ssh_allow_empty_password () {
>>       fi
>>   }
>>
>> +# Enable postinst logging if debug-tweaks is enabled
>> +postinst_enable_logging () {
>> +    touch ${IMAGE_ROOTFS}/etc/.enable_postinst_logging
>
> I do not think this should go here, maybe better to have it in the 
> /etc/default dir and named postinst, since that matches what other init
> related scripts do and source the file in each postinst script, it 
> should also have a standard guard to check existance of the file and 
> checks something like a "ENABLED" variable instead?  This could also 
> be extended for getting the log file location also.
>
> Sau!
>

Thanks a lot.
I'll follow your advices and send out a v2.

Best Regards,
Chen Qi

>
>> +}
>> +
>>   # Turn any symbolic /sbin/init link into a file
>>   remove_init_link () {
>>       if [ -h ${IMAGE_ROOTFS}/sbin/init ]; then
>>
>
>

[PATCH 2/4] dpkg: modify the run-postinst script to enable postinst logging

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

Enable postinst logging according to the existence of the file
/etc/.enable_postinst_logging.

In this way, the postinst logging is enabled if 'debug-tweaks' is
in IMAGE_FEATURES, and at the same time, we avoid unnecessary rebuilt
if IMAGE_FEATURES is changed.

[YOCTO #4262]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-devtools/dpkg/dpkg.inc |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/meta/recipes-devtools/dpkg/dpkg.inc b/meta/recipes-devtools/dpkg/dpkg.inc
index d773fbd..c013d03 100644
--- a/meta/recipes-devtools/dpkg/dpkg.inc
+++ b/meta/recipes-devtools/dpkg/dpkg.inc
@@ -35,10 +35,6 @@ do_configure () {
     autotools_do_configure
 }
 
-POSTLOG ?= "/var/log/postinstall.log"
-REDIRECT_CMD = "${@base_contains('IMAGE_FEATURES', 'debug-tweaks', '>${POSTLOG} 2>&1', '', d)}"
-REDIRECT_CMD[vardepsexclude] += "IMAGE_FEATURES POSTLOG"
-
 DPKG_INIT_POSITION ?= "98"
 
 do_install_append () {
@@ -67,7 +63,11 @@ if [ "x$D" != "x" ] && [ -f $D/var/lib/dpkg/status ]; then
 
 	# this happens at S98 where our good 'ole packages script used to run
 	echo "#!/bin/sh
-dpkg --configure -a ${REDIRECT_CMD}
+if [ -e /etc/.enable_postinst_logging ]; then
+    dpkg --configure -a >/var/log/postinstall.log 2>&1
+else
+    dpkg --configure -a
+fi
 rm -f ${sysconfdir}/rcS.d/S${DPKG_INIT_POSITION}run-postinsts
 " > $D${sysconfdir}/rcS.d/S${DPKG_INIT_POSITION}run-postinsts
 	chmod 0755 $D${sysconfdir}/rcS.d/S${DPKG_INIT_POSITION}run-postinsts
-- 
1.7.9.5

[PATCH 3/4] opkg: modify the run-postinst script to enable postinst logging

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

Enable postinst logging according to the existence of the file
/etc/.enable_postinst_logging.

In this way, the postinst logging is enabled if 'debug-tweaks' is
in IMAGE_FEATURES, and at the same time, we avoid unnecessary rebuilt
if IMAGE_FEATURES is changed.

[YOCTO #4262]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-devtools/opkg/opkg.inc |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/meta/recipes-devtools/opkg/opkg.inc b/meta/recipes-devtools/opkg/opkg.inc
index c34ac5b..254b0e7 100644
--- a/meta/recipes-devtools/opkg/opkg.inc
+++ b/meta/recipes-devtools/opkg/opkg.inc
@@ -57,10 +57,6 @@ do_install_append_class-native() {
 	fi
 }
 
-POSTLOG ?= "/var/log/postinstall.log"
-REDIRECT_CMD = "${@base_contains('IMAGE_FEATURES', 'debug-tweaks', '>${POSTLOG} 2>&1', '', d)}"
-REDIRECT_CMD[vardepsexclude] += "IMAGE_FEATURES POSTLOG"
-
 pkg_postinst_${PN} () {
 #!/bin/sh
 if [ "x$D" != "x" ] && [ -f $D${OPKGLIBDIR}/opkg/status ]; then
@@ -68,7 +64,11 @@ if [ "x$D" != "x" ] && [ -f $D${OPKGLIBDIR}/opkg/status ]; then
 
 	# this happens at S98 where our good 'ole packages script used to run
 	echo "#!/bin/sh
-opkg-cl configure ${REDIRECT_CMD}
+if [ -e /etc/.enable_postinst_logging ]; then
+    opkg-cl configure >/var/log/postinstall.log 2>&1
+else
+    opkg-cl configure
+fi
 rm -f /${sysconfdir}/rcS.d/S${POSTINSTALL_INITPOSITION}run-postinsts
 " > $D${sysconfdir}/rcS.d/S${POSTINSTALL_INITPOSITION}run-postinsts
 	chmod 0755 $D${sysconfdir}/rcS.d/S${POSTINSTALL_INITPOSITION}run-postinsts
-- 
1.7.9.5

[PATCH 4/4] rpm-postinsts.bb: enable postinst logging

[Qi.Chen]

From: Chen Qi <Qi.Chen@windriver.com>

Enable postinst logging according to the existence of the file
/etc/.enable_postinst_logging.

In this way, the postinst logging is enabled if 'debug-tweaks' is
in IMAGE_FEATURES, and at the same time, we avoid unnecessary rebuild
if IMAGE_FEATURES is changed.

[YOCTO #4262]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-devtools/rpm/rpm-postinsts.bb |   11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/meta/recipes-devtools/rpm/rpm-postinsts.bb b/meta/recipes-devtools/rpm/rpm-postinsts.bb
index 3d2d05d..ae0e37e 100644
--- a/meta/recipes-devtools/rpm/rpm-postinsts.bb
+++ b/meta/recipes-devtools/rpm/rpm-postinsts.bb
@@ -11,10 +11,6 @@ inherit allarch
 #
 POSTINSTALL_INITPOSITION ?= "98"
 
-POSTLOG ?= "/var/log/postinstall.log"
-REDIRECT_CMD = "${@base_contains('IMAGE_FEATURES', 'debug-tweaks', '>>${POSTLOG} 2>&1', '', d)}"
-REDIRECT_CMD[vardepsexclude] += "IMAGE_FEATURES POSTLOG"
-
 do_fetch() {
 	:
 }
@@ -40,7 +36,12 @@ if [ "x$D" != "x" ] && [ -f $D/var/lib/rpm/Packages ]; then
 [ -d /etc/rpm-postinsts ] && for i in `ls /etc/rpm-postinsts/`; do
 	i=/etc/rpm-postinsts/$i
 	echo "Running postinst $i..."
-	if [ -f $i ] && $i ${REDIRECT_CMD}; then
+	if [ -x $i ]; then
+		if [ -e /etc/.enable_postinst_logging ]; then
+			$i >/var/log/postinstall.log 2&>1
+		else
+			$i
+		fi
 		rm $i
 	else
 		echo "ERROR: postinst $i failed."
-- 
1.7.9.5

Re: [PATCH 0/4] Postinst logging reimplementation

[Burton, Ross]

On 16 May 2013 08:56,  <Qi.Chen@windriver.com> wrote:
> 1. The run-postinst scripts will log outputs and errors if /etc/.enable_postinst_logging is present.

I'm not very keen on dot-files in etc, and as the postinst init script
is one-shot shouldn't it be deleting that configuration file once it's
completed?

Ross