[weston] xkbcommon library is not optional.

[Arnaud Vrac]

On Fri, Oct 16, 2015 at 11:10 AM, Pekka Paalanen <ppaalanen at gmail.com>
wrote:

> 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.
>

If you know of a way to strip some parts of the database, I'm interested.
I've tried only keep evdev mappings and a few layouts, but going at it it
seemed like a very hard task so I just gave up.


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



-- 
Arnaud
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20151016/07061418/attachment.html>