On 14 November 2016 at 09:28, Pekka Paalanen <[email protected]> wrote: > 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. > 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 _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
