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

[Pekka Paalanen]

On Fri, 11 Nov 2016 20:31:54 +0000
Emil Velikov <emil.l.velikov at gmail.com> wrote:

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

Hi Emil,

I pretty much agree on all of your points, this would be a fine start of
a new thread. However, in this thread it is all totally off-topic and
irrelevant.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20161114/4b84998e/attachment.sig>