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

[Emil Velikov]

On 14 November 2016 at 09:28, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 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.
>
Looking at it from another angle - I might have high-jack/diverted the thread.
Sorry about that gents, it wasn't my intent.

I'll keep that separate - be that here or on bz/phab.

-Emil