[weston] xkbcommon library is not optional.

[Pekka Paalanen]

On Fri, 16 Oct 2015 10:27:56 +0200
Hardening <rdp.effort at gmail.com> wrote:

> Le 16/10/2015 03:28, Bryce Harrington a écrit :
> > On Tue, Oct 13, 2015 at 01:59:13PM +0200, Joaquim Duran wrote:
> >> Hello,
> >>
> >> When configuring the Weston project, it is possible to disable (don't
> >> include) the library libxkbcommon. To compile Weston successfully,
> >> even if the option --disable-xkbcommon is specified, the library must
> >> be installed because the file src/compositor.h requires it.
> > 
> > Joaquim, good find.
> > 
> > From the comments in configure.ac:
> > 
> >           AS_HELP_STRING([--disable-xkbcommon], [Disable libxkbcommon
> >                   support: This is only useful in environments
> >                   where you do not have a hardware keyboard. If
> >                   libxkbcommon support is disabled clients will not
> >                   be sent a keymap and and must know how to
> >                   interpret the keycode sent for any key event.]),,
> > 
> > So it sounds like this is a special case that is intended to work.
> > 
> > The header include was from commit 855028fe three years ago, while the
> > --disable-xkbcommon was added by 382ff46f just two years ago.  That
> > makes me think that your situation was simply overlooked.
> > 
> > If that's true, then presumably the fix would involve peppering
> > compositor.* and other files with tests like,
> > 
> > #ifdef ENABLE_XKBCOMMON
> > ...
> > #endif
> > 
> > For example it looks like the weston_xkb_info struct would need to
> > either be removed or stubbed out, and users of it be disabled.  There
> > are also some weston calls that use xkb_keymap, xkb_rule_names,
> > etc. structures for params that'd need addressed.
> > 
> > On the face of it, seems like it could be a fair amount of work, and a
> > bit invasive, but maybe there's some tricks to make it simpler
> > (typedeffing the unsupported struct args and so on.)
> > 
> > Alternatively, --disable-xkbcommon could be dropped.  However I get the
> > sense it actually solves a real world need, and functional regression is
> > rarely a good idea.
> > 
> 
> I'm to fix the bug. Anyway it's almost sure that there's no real usage
> of this flag as otherwise weston does not compile...

I would be suprised if it didn't work at the time it was added. I
suspect we broke it later, and no-one noticed.

I recall some discussions about some obscure embedded platform where
xkbcommon, even when it's tiny, was deemed as waste of space,
especially along with the xkeyboard-config database (even though you
don't even need the whole database). Or maybe it was about wasting
memory. Can't recall.

Hrm, now that I actually test --disable-xkbcommon, Weston *does* build.
All I get during build is:

  CC       src/weston-input.o
/home/pq/git/weston/src/input.c: In function ‘weston_seat_init_keyboard’:
/home/pq/git/weston/src/input.c:2226:1: warning: label ‘err’ defined but not used [-Wunused-label]
/home/pq/git/weston/src/input.c: In function ‘weston_keyboard_reset_state’:
/home/pq/git/weston/src/input.c:2238:20: warning: unused variable ‘state’ [-Wunused-variable]
/home/pq/git/weston/src/input.c: At top level:
/home/pq/git/weston/src/input.c:555:1: warning: ‘weston_xkb_info_destroy’ used but never defined [enabled by default]
/home/pq/git/weston/src/input.c:989:1: warning: ‘run_modifier_bindings’ defined but not used [-Wunused-function]

And no other warning or error.

I do not get a build failure, because libxkbcommon is installed in my
system, and so I don't need additional -I flags to find its headers.
When we build modules (backends, plugins, anything), missing symbols
are not fatal because the parent executable is providing some we must
not error on.

At least x11-backend.so will fail to load doe to missing
xkbcommon symbols. Makes me wonder if there is some other backend that
doesn't fail so.

So it's possible someone is or was actually getting away with this,
having xkbcommon installed for the build but not at runtime.

We can remove --disable-xkbcommon, but be prepared having to revert and
fix that properly later.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151016/29061a4c/attachment.sig>