[PATCH wayland 3/3] tests: add scanner tests

Emil Velikov emil.l.velikov at gmail.com
Fri Nov 11 20:31:54 UTC 2016


Hi Pekka,

It's great to see some tests for the scanner. There's a few thoughts I
may have mentioned before - please don't read too much into them.

On 10 November 2016 at 09:57, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
>
> Add tests that ensure that wayland-scanner output for a given input does
> not change unexpectedly. This makes it very easy to review
> wayland-scanner patches.
>
> Before, when patches were proposed for wayland-scanner, I had to
> build wayland without the patches, save the generated files into a
> temporary directory, apply the patches, build again, and diff the old
> vs. new generated file.
>
> No more. Now whenever someone makes intentional changes to
> wayland-scanner's output, he will also have to patch the example output
> files to match. That means that reviewers see the diff of the generated
> files straight from the patch itself. Verifying the diff is true is as
> easy as 'make check'.
>
> The tests use separate example XML files instead of wayland.xml
> directly, so that wayland.xml can be updated without fixing scanner
> tests, avoiding the churn.
>
> example.xml starts as a copy of wayland.xml. If wayland.xml starts using
> new wayland-scanner features, they should be copied into example.xml
> again to be covered by the tests.
>
That's the weakest point in the current proposal and something that
should be addressed, imho.

"The problem": wayland{,-scanner} has changed ABI in a non-backwards
(or forward depending on the POV) manner a few times. Such that when
built against the newer wayland{,-scanner} it won't work with older
one.
While that happens rarely, one should provide control, so that one can
choose "use the new X code-paths" or not. This way today's implicit
dependency will be transformed to explicit one.

Obviously one can argue that we should use the same version of wayland
at build and runtime. Yet binary distributions (or any distribution
for that manner) do not limit upper version of the dependencies. Thus
it's not uncommon to have scenario like the following:
Package requires: vX [pulled of the configure error/warning] Build
against: vX+1. Thus at run-time [with vX] the prebuild package will
fail due to the implicit dependency for vX+1 during build time.

To resolve that, I'm suggesting:
 a) example.xml must always be kept in sync with wayland.xml
 b) guards around new API should be used and propagated [in the
generated files] by the scanner.

#ifdef USE_NEW_API_FOO
   foo_new()
#else
  foo()
#endif

The guards can be in two(or more?) forms - explicit per-feature/fix
(USE_NEW API_*) or general I want all fixes (USE_WAYLAND_API_*). Both
approaches have positive and negative sides.

 c) make check can be deployed to establish if the API used matches
the user preference.
One way to do it is to have a dummy binary before/after each API/ABI
break thus one can parse the API pulled/used by it via nm, objdump or
alike.


All that said, _none_ of the above is something that should be
addressed at this stage.
Although it would be great to happen before the next API/ABI break ;-)


Thanks
Emil


More information about the wayland-devel mailing list