Patchwork gconf.bbclass: make postinstall fail silently when running offline

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

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

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

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