[PATCH] XKB: Remove component listing support

[Daniel Stone]

Hi,

On 2 November 2012 14:37, Peter Hutterer <peter.hutterer at who-t.net> wrote:

> can you covert your test to a XIT test and add it to (a newly created)
> tests/server/xkb?
> http://cgit.freedesktop.org/~whot/xorg-integration-tests/


Are these tests ever supposed to work with anything other than master? I
wrote a test which I'll send to the list, but if I push that, the tests are
going to fail on literally every released version of the server ever, which
doesn't sound particularly useful.

Is there much of a compelling reason for not having the tests in-tree with
the server? What's the actual split between xorg-gtest and
xorg-integration-tests? Why does the former have a couple of random tests
and a device declaration file if it seems to mainly just be a framework? Is
there much use in having the split, given how codependent they seem to be?

Also, it relies on evemu, which isn't packaged in Debian, which is kind of
irritating.  (Also, evemu's autogen.sh isn't actually the standard X.Org
autogen.sh, and doesn't run ./configure for you.  And it's on Launchpad,
using Bazaar ... I die.  For bonus points, it's even licensed under GPLv3,
which means a hell of a lot of companies will not go near it no matter
what, and probably just skip the integration tests.)

With all the linking of massive chunks of C++, it's also crazy slow to
build: I can autoreconf, configure, make, and make check Weston and its
sample clients from scratch at least four times over in the time it takes
to just build the tests, let alone run them.  Probably closer to five or
six.  When I do get to run them, the Synaptics tests (massively valuable,
don't get me wrong) all interfere with my running server, presumably
because it doesn't know to ignore a test device.  That's something that'd
be really nice to have a flag for.  The Wacom tests are all failing
(presumably due to not having the driver installed), but between the
two-second delay for server starting and five-second device wait, are
taking seven seconds a pop.  There's forty-eight of those tests, which
makes it just over five and a half minutes.  I also now have a bajillion
files directly under /tmp.

The keyboard tests all failed because the dummy display driver
configuration didn't get written out, so it tried to spawn a new Intel
server, only to die when it realised it couldn't become a DRM master.

Sorry if this comes across as kind of harsh - I really, really like the
idea, but it comes across as pretty irritating to actually write the tests.
 Take this with a grain of salt since I barely touch X these days, but it
just seems way too clunky become part of my regular workflow.  It's all
fixable, mind (with the possible exception of the massive chunks o' C++),
but the out-of-the-box experience isn't much fun.


> > -    list.pattern[_XkbListKeycodes] = GetComponentSpec(&str, FALSE,
> &status);
> > -    list.pattern[_XkbListTypes] = GetComponentSpec(&str, FALSE,
> &status);
> > -    list.pattern[_XkbListCompat] = GetComponentSpec(&str, FALSE,
> &status);
> > -    list.pattern[_XkbListSymbols] = GetComponentSpec(&str, FALSE,
> &status);
> > -    list.pattern[_XkbListGeometry] = GetComponentSpec(&str, FALSE,
> &status);
> > -    if (status != Success)
> > -        return status;
> > +    for (i = 0; i < _XkbListNumComponents; i++) {
> > +        size = *((uint8_t *)str);
> > +        str += (size + 1);
> > +    }
>
> please add a short comment that str is a set of pascal-style strings so
> we're really just skipping over the content of the string here. it's not
> really obvious for the unsuspecting reviewer.
>

Sure, I've done this now.  I've also changed the 5 to 6: turns out
that 1ad80678 (committed 2009) broke this function anyway.  It shifted all
the component masks up one (so your keycode spec was actually the keymap
spec, types were keycodes, etc), and it'd also give you BadLength unless
you specified * for everything, or otherwise managed to have your length
such that the length of keymap string spec + 1 fit into the four-character
padding.


> I suspect if anyone was really using this instead of the xml, we won't hear
> of it until this feeds into RHEL or a similar distro...
>

Given that's the second problem I've found with this code, I doubt it'll be
too much of an issue.


> anyway, with the comment:
> Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>
>

Thanks, resent if you want to pull into your tree.

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20121105/cdf1c748/attachment-0001.html>