Patchwork libx11: enable Xcms by default

login
register
mail settings
Submitter Jonathan Liu
Date June 20, 2013, 1:54 a.m.
Message ID <1371693296-6834-1-git-send-email-net147@gmail.com>
Download mbox | patch
Permalink /patch/52073/
State Accepted
Commit d860ee68208b84efb8049669ca18acc69f2f2d1b
Headers show

Comments

Jonathan Liu - June 20, 2013, 1:54 a.m.
The following do not work if Xcms is not enabled in libx11:
- starting xterm and doing ls --color / fails to color code the
  directories:
  xterm: Cannot allocate color "rgb:5c/5c/ff"
- xsetroot -solid rgb:5c/5c/ff fails with the following error:
  xsetroot:  unknown color "rgb:5c/5c/ff"
- xsetroot -solid rgbi:1.0/1.0/1.0 fails with the following error:
  xsetroot:  unknown color "rgbi:1.0/1.0/1.0"

More specifically, applications that pass Xcms color names to
XParseColor do not work properly.

[YOCTO #4576]

Signed-off-by: Jonathan Liu <net147@gmail.com>
---
 meta/recipes-graphics/xorg-lib/libx11-diet_1.5.0.bb | 2 ++
 meta/recipes-graphics/xorg-lib/libx11.inc           | 4 +---
 2 files changed, 3 insertions(+), 3 deletions(-)
Phil Blundell - June 20, 2013, 6:34 a.m.
On Thu, 2013-06-20 at 11:54 +1000, Jonathan Liu wrote:
> The following do not work if Xcms is not enabled in libx11:
> - starting xterm and doing ls --color / fails to color code the
>   directories:
>   xterm: Cannot allocate color "rgb:5c/5c/ff"
> - xsetroot -solid rgb:5c/5c/ff fails with the following error:
>   xsetroot:  unknown color "rgb:5c/5c/ff"
> - xsetroot -solid rgbi:1.0/1.0/1.0 fails with the following error:
>   xsetroot:  unknown color "rgbi:1.0/1.0/1.0"
> 
> More specifically, applications that pass Xcms color names to
> XParseColor do not work properly.

The comment that you are deleting suggests that this behaviour is by
design:

> -# Let people with incredibly archaic requirements enable Xcms, but disable it by
> -# default.

Since there's already a PACKAGECONFIG option for this, can't you just
enable that in your distro configuration rather than changing the
default?  I don't think it's ever been the expectation that the default
settings will represent "maximum functionality".

p.
Jonathan Liu - June 20, 2013, 8:29 a.m.
On 20 June 2013 16:34, Phil Blundell <pb@pbcl.net> wrote:
> On Thu, 2013-06-20 at 11:54 +1000, Jonathan Liu wrote:
>> The following do not work if Xcms is not enabled in libx11:
>> - starting xterm and doing ls --color / fails to color code the
>>   directories:
>>   xterm: Cannot allocate color "rgb:5c/5c/ff"
>> - xsetroot -solid rgb:5c/5c/ff fails with the following error:
>>   xsetroot:  unknown color "rgb:5c/5c/ff"
>> - xsetroot -solid rgbi:1.0/1.0/1.0 fails with the following error:
>>   xsetroot:  unknown color "rgbi:1.0/1.0/1.0"
>>
>> More specifically, applications that pass Xcms color names to
>> XParseColor do not work properly.
>
> The comment that you are deleting suggests that this behaviour is by
> design:
>
>> -# Let people with incredibly archaic requirements enable Xcms, but disable it by
>> -# default.
>
> Since there's already a PACKAGECONFIG option for this, can't you just
> enable that in your distro configuration rather than changing the
> default?  I don't think it's ever been the expectation that the default
> settings will represent "maximum functionality".
>
> p.
>
>

I already do enable it in my distro configuration. The motivation for
having this enabled by default is that discovering that applications
don't work as expected and then trying to track down what is causing
it can take up quite a bit of time. This time would have been better
spent on actual application development.

If it is not needed, the PACKAGECONFIG can be overridden or
libx11-diet used instead.

Regards,
Jonathan
Ross Burton - June 20, 2013, 9:54 a.m.
On 20 June 2013 07:34, Phil Blundell <pb@pbcl.net> wrote:
> The comment that you are deleting suggests that this behaviour is by
> design:
>
>> -# Let people with incredibly archaic requirements enable Xcms, but disable it by
>> -# default.
>
> Since there's already a PACKAGECONFIG option for this, can't you just
> enable that in your distro configuration rather than changing the
> default?  I don't think it's ever been the expectation that the default
> settings will represent "maximum functionality".

That comment is from me, but I'm actually coming around to enabling
Xcms by default and letting people who care about the size increase
turn it off.  As Jonathan discovered, it's entirely possible to use
Xcms without knowing it - whilst the colour management bit is likely
never used, the colour parsing code is.

Ross
Ross Burton - June 26, 2013, 10:19 a.m.
On 20 June 2013 10:54, Burton, Ross <ross.burton@intel.com> wrote:
>> Since there's already a PACKAGECONFIG option for this, can't you just
>> enable that in your distro configuration rather than changing the
>> default?  I don't think it's ever been the expectation that the default
>> settings will represent "maximum functionality".
>
> That comment is from me, but I'm actually coming around to enabling
> Xcms by default and letting people who care about the size increase
> turn it off.  As Jonathan discovered, it's entirely possible to use
> Xcms without knowing it - whilst the colour management bit is likely
> never used, the colour parsing code is.

I did some build testing.  Enabling Xcms comes at a cost of ~88K
(x86-64 target, as I'm currently building for the NUC), which is a 7%
increase.  Considering that it's possible to use Xcms without knowing
it (as Jonathan discovered, by using fancy colour names), the size
increase isn't massive, the existence of both a PACKAGECONFIG to turn
it off again *and* and libx11-diet that's even smaller than libx11, I
think we should enable it by default.

People who know they don't use Xcms and care about the ~80K can set
this in a distro config or switch to libx11-diet if space really is
that important.

So, Signed-off-by: Ross Burton <ross.burton@intel.com>

Ross

Patch

diff --git a/meta/recipes-graphics/xorg-lib/libx11-diet_1.5.0.bb b/meta/recipes-graphics/xorg-lib/libx11-diet_1.5.0.bb
index 0a90f46..0b50f09 100644
--- a/meta/recipes-graphics/xorg-lib/libx11-diet_1.5.0.bb
+++ b/meta/recipes-graphics/xorg-lib/libx11-diet_1.5.0.bb
@@ -18,3 +18,5 @@  SRC_URI[md5sum] = "78b4b3bab4acbdf0abcfca30a8c70cc6"
 SRC_URI[sha256sum] = "c382efd7e92bfc3cef39a4b7f1ecf2744ba4414a705e3bc1e697f75502bd4d86"
 
 EXTRA_OECONF += "--disable-xlocale"
+
+PACKAGECONFIG ??= ""
diff --git a/meta/recipes-graphics/xorg-lib/libx11.inc b/meta/recipes-graphics/xorg-lib/libx11.inc
index e0fce3e..b4eb7d8 100644
--- a/meta/recipes-graphics/xorg-lib/libx11.inc
+++ b/meta/recipes-graphics/xorg-lib/libx11.inc
@@ -25,9 +25,7 @@  DEPENDS += "xproto-native"
 EXTRA_OECONF += "--with-keysymdefdir=${STAGING_INCDIR}/X11/ --disable-xf86bigfont"
 EXTRA_OEMAKE += 'CWARNFLAGS=""'
 
-# Let people with incredibly archaic requirements enable Xcms, but disable it by
-# default.
-PACKAGECONFIG ??= ""
+PACKAGECONFIG ??= "xcms"
 PACKAGECONFIG[xcms] = "--enable-xcms,--disable-xcms"
 
 # src/util/makekeys is built natively but needs -D_GNU_SOURCE defined.