On Fri, 11 Nov 2016 20:31:54 +0000 Emil Velikov <[email protected]> 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 <[email protected]> wrote: > > From: Pekka Paalanen <[email protected]> > > > > 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
pgpGSIPzRQGoM.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
