[PATCH] gconf.bbclass: make postinstall fail silently when running offline

[PATCH] gconf.bbclass: make postinstall fail silently when running offline

[Laurentiu Palcu]

Gconf backend does not accept special characters in configuration source
addresses. When populating SDK target sysroot from core-image-sato, for
example, the configuration source address contains "1.3+snapshot" in it
and '+' is an invalid character. Thus, gconftool-2 will fail and the
build will stop at do_rootfs because the log contains the "ERROR"
string.

Since failing offline will postpone the postinstall execution for
target's first boot, we can silently fail here, so the build can
complete.

[YOCTO #3893]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 meta/classes/gconf.bbclass |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/meta/classes/gconf.bbclass b/meta/classes/gconf.bbclass
index e9076b2..11ad495 100644
--- a/meta/classes/gconf.bbclass
+++ b/meta/classes/gconf.bbclass
@@ -23,8 +23,14 @@ fi
 SCHEMA_LOCATION=$D/etc/gconf/schemas
 for SCHEMA in ${SCHEMA_FILES}; do
 	if [ -e $SCHEMA_LOCATION/$SCHEMA ]; then
-		HOME=$D/root gconftool-2 \
-			--makefile-install-rule $SCHEMA_LOCATION/$SCHEMA > /dev/null
+		export HOME=$D/root
+		if [ "x$D" != "x" ]; then
+			gconftool-2 \
+				--makefile-install-rule $SCHEMA_LOCATION/$SCHEMA > /dev/null 2>&1 || exit 1
+		else
+			gconftool-2 \
+				--makefile-install-rule $SCHEMA_LOCATION/$SCHEMA > /dev/null
+		fi
 	fi
 done
 }
-- 
1.7.9.5

Re: [PATCH] gconf.bbclass: make postinstall fail silently when running offline

[Richard Purdie]

On Wed, 2013-02-20 at 13:57 +0200, Laurentiu Palcu wrote:
> Gconf backend does not accept special characters in configuration source
> addresses. When populating SDK target sysroot from core-image-sato, for
> example, the configuration source address contains "1.3+snapshot" in it
> and '+' is an invalid character. Thus, gconftool-2 will fail and the
> build will stop at do_rootfs because the log contains the "ERROR"
> string.
> 
> Since failing offline will postpone the postinstall execution for
> target's first boot, we can silently fail here, so the build can
> complete.
> 
> [YOCTO #3893]
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
> ---
>  meta/classes/gconf.bbclass |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)

Just to be clear, I think we need to patch gconf to fix this, there is
no good reason it shouldn't be handling the + character (unless you've
found one I don't know about). Hiding stderr is a recipe for problems in
future and we want these postinstalls to run at build time.

Cheers,

Richard

Re: [PATCH] gconf.bbclass: make postinstall fail silently when running offline

[Laurentiu Palcu]

On 02/22/2013 03:49 PM, Richard Purdie wrote:
> Just to be clear, I think we need to patch gconf to fix this, there is
> no good reason it shouldn't be handling the + character (unless you've
> found one I don't know about).

I didn't write GConf. I don't know the reason why they chose to declare
the following list of characters invalid:
"\t\r\n\"$&<>,+=#!()'|{}[]?~`;%\\".

I did look in the commit message and found no other explanations except
this:
"gconf/gconf-backend.c: (gconf_address_valid), (gconf_get_backend):
check the backend address doesn't contain any special characters."

Here is the entire commit:
http://git.gnome.org/browse/gconf/commit/?id=3d720f4a0c00af31c1d53fc4aa45d6d6580c433e

OK, let's say I remove '+' from that list. I agree, it doesn't look like
it would have bad consequences but what happens if, in the future, the
path contains another "invalid" character? Where do we draw the line?

> Hiding stderr is a recipe for problems in
> future and we want these postinstalls to run at build time.

There was a patchset on the ML these days that enabled postinstall
output redirection to a certain file. Why not have this activated all
the time for the native dpkg/opkg/rpm and, on request, for the target
binaries? We could inspect the logs in case some postinstalls failed to
run on host but we don't end the build since, maybe, some people would
still be fine with running those postinstalls on target.

Wouldn't this be better?

Thanks,
Laurentiu

Re: [PATCH] gconf.bbclass: make postinstall fail silently when running offline

[Burton, Ross]

Hi,

On 22 February 2013 14:58, Laurentiu Palcu <laurentiu.palcu@intel.com> wrote:
> I didn't write GConf. I don't know the reason why they chose to declare
> the following list of characters invalid:
> "\t\r\n\"$&<>,+=#!()'|{}[]?~`;%\\".
>
> I did look in the commit message and found no other explanations except
> this:
> "gconf/gconf-backend.c: (gconf_address_valid), (gconf_get_backend):
> check the backend address doesn't contain any special characters."
>
> Here is the entire commit:
> http://git.gnome.org/browse/gconf/commit/?id=3d720f4a0c00af31c1d53fc4aa45d6d6580c433e
>
> OK, let's say I remove '+' from that list. I agree, it doesn't look like
> it would have bad consequences but what happens if, in the future, the
> path contains another "invalid" character? Where do we draw the line?

+ is notable in that list because it's not handled specially by the
shell, whereas practictically the rest of them are.

> There was a patchset on the ML these days that enabled postinstall
> output redirection to a certain file. Why not have this activated all
> the time for the native dpkg/opkg/rpm and, on request, for the target
> binaries? We could inspect the logs in case some postinstalls failed to
> run on host but we don't end the build since, maybe, some people would
> still be fine with running those postinstalls on target.

Having those logs is useful for inspection, but as we're trying to run
all postinsts on the host this trivially-triggered problem needs to be
resolved.  Patching the sanity test is a reasonable solution.

Ross

[PATCH] gconf: add patch to remove '+' from invalid characters list

[Laurentiu Palcu]

This will allow gconftool-2 to run when configuration source addresses
contain the '+' sign.

[YOCTO #3893]

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
---
 .../remove_plus_from_invalid_characters_list.patch |   19 +++++++++++++++++++
 meta/recipes-gnome/gnome/gconf_3.2.5.bb            |    1 +
 2 files changed, 20 insertions(+)
 create mode 100644 meta/recipes-gnome/gnome/gconf-3.2.5/remove_plus_from_invalid_characters_list.patch

diff --git a/meta/recipes-gnome/gnome/gconf-3.2.5/remove_plus_from_invalid_characters_list.patch b/meta/recipes-gnome/gnome/gconf-3.2.5/remove_plus_from_invalid_characters_list.patch
new file mode 100644
index 0000000..59a7ca7
--- /dev/null
+++ b/meta/recipes-gnome/gnome/gconf-3.2.5/remove_plus_from_invalid_characters_list.patch
@@ -0,0 +1,19 @@
+Upstream-Status: Pending
+
+Remove '+' from invalid characters list
+
+Signed-off-by: Laurentiu Palcu <laurentiu.palcu@intel.com>
+
+Index: GConf-3.2.5/gconf/gconf-backend.c
+===================================================================
+--- GConf-3.2.5.orig/gconf/gconf-backend.c
++++ GConf-3.2.5/gconf/gconf-backend.c
+@@ -37,7 +37,7 @@ static const char invalid_chars[] =
+   /* Space is common in user names (and thus home directories) on Windows */
+   " "
+ #endif
+-  "\t\r\n\"$&<>,+=#!()'|{}[]?~`;%\\";
++  "\t\r\n\"$&<>,=#!()'|{}[]?~`;%\\";
+ 
+ static gboolean
+ gconf_address_valid (const char  *address,
diff --git a/meta/recipes-gnome/gnome/gconf_3.2.5.bb b/meta/recipes-gnome/gnome/gconf_3.2.5.bb
index c80c0b0..abdf151 100644
--- a/meta/recipes-gnome/gnome/gconf_3.2.5.bb
+++ b/meta/recipes-gnome/gnome/gconf_3.2.5.bb
@@ -15,6 +15,7 @@ inherit gnomebase gtk-doc
 
 SRC_URI = "${GNOME_MIRROR}/GConf/${@gnome_verdir("${PV}")}/GConf-${PV}.tar.xz;name=archive \
            file://obsolete_automake_macros.patch \
+           file://remove_plus_from_invalid_characters_list.patch \
 "
 
 S = "${WORKDIR}/GConf-${PV}"
-- 
1.7.9.5