On 16 April 2016 at 22:06, Jason Ekstrand <ja...@jlekstrand.net> wrote:
>> Then again importing bits from other projects tend to be quite a nasty >> solution. The place where I borrowed the idea from (libSDL) does/did >> not imports the protocol. > > > I don't think this is as bad an idea as you seem to think. I'll explain a > bit further down. > <extra cheeky> If it's a good idea why aren't others doing it ?</extra cheeky> Please don't take the above too seriously. >> This is precisely the subtlety in the whole thing. The wayland >> protocol tried (and I believe so far is) backwards compatible, then >> again the library that we use is not. This is a fundamental difference >> that causes these headaches. >> >> If one is to import protocol vX, what happens as libwayland tries to >> use feature A or B introduced in newer protocols via the same API ? >> They have to reimplement everything on their own - bad idea. >> >> How about the earlier patch was resolving another ABI break ? The >> developers rightfully broke it because things were racy. Do we want to >> check-in old version and be forced to use the racy interface ? > > > Client-side libwayland has *never* had an ABI break since version 1.0. The > libwayland-client ABI is very small (one or two dozen functions) and has > always maintained backwards-compatibility. We've made some pretty crazy > changes to libwayland's internals and worked very hard to maintain > compatibility. > > What has changed over time is the header file and the exact functions that > it calls. The protocol headers (as opposed to wayland-client-core.h) > contain only static inline wrapper functions that implement the protocol in > terms of the stable ABI provided by libwayland-client.so. As new features > get added to libwayland-client to remove race conditions, provide object > versioning, or any other improvements, the wayland-client-protocol headers > get updated to take advantage of the new functionality. That way clients > built against new versions of libwayland-client automatically get the > upgrade without any developer intervention. It's really a pretty good > system. > Interesting. Not too long ago I asked around (#xorg-devel iirc) about the proper terminology in a similar case and ABI popped up. I believe the term ABI is correct, as if we compile program X against wayland N+1 it may not be compatible with wayland N. Where if the very same application is rebuilt against wayland N it will work just fine. Note: program X has only a single code path - it does not detect nor differentiate between the different versions of wayland. This does sound like the definition of ABI break to me. I'm likely missing something here. If the above is off what is the correct terminology ? How are the symbols provided by libwayland called ? How about the user/programmer facing ones, provided by the headers ? > The problem is waffle's usage of libwayland and its generated headers. > Because waffle inserts its own shim layer in between, there are two versions > of the libwayland API in play: the one provided by libwayland-client.so and > the one provided by waffle's shim layer. Even though the libwayland-client > API is progressing in a backwards-compatable manner, waffle doesn't get new > entrypoints until it plumbs them through its shim layer so it's always a bit > behind. The problem with waffle's useage of the headers is that the > functions in the wayland-client-protocol.h header are calling into waffle's > shim layer and libwayland-client.so directly. This means that the > "automatic upgrade" process that normal wayland apps get won't work for > waffle. Double checking - "automatic upgrade" refers to the regenerated headers correct ? > Instead, waffle can't upgrade to a new version of the protocol > header until it updates its shim layer. Not quite. There is an issue only when new libwayland API gets unconditionally used, which (as I call it) breaks the ABI. Furthermore if one resolves a wayland issue without adding new API by tweaking the headers, this leaves waffle with imported headers 'vulnerable' (completely wrong terminology but you get the point). Afaict we have two options: A: 3-4 lines of shim layer every so often - afaics this is the second case twice wayland 1.0 B: Recheck-in the updated header and update the libwayland version on regular intervals. If we want to use old wayland versions (as you point out below) - one also has to add stubs. > This means that it can't trust the > system-installed one and needs to use its own. > I'm afraid I don't see how you made this deduction based on the previous statements. > Is it safe to check a copy in? Absolutely. Like I said above, > wayland-client-protocol-core.h is very careful to only provide static inline > wrappers around the actual set of symbols exported from libwayland-client.so > and those are kept backwards-compatable. You can compile against as old a > copy of wayland-client-protocol.h as you want and it should work fine. > Modulo the bugs that have been fixed and the possible new feature or two. The latter of which we don't really need atm. > What about the fixed race condition? Does waffle care? If waffle doesn't > care about multithreading, the race doesn't matter and you can feel free to > use an older version of the header. If it does care, then we should use a > version that has the race condition fixed and depend on at least that > version of libwayland. Same goes for the versioning stuff that caused this > discussion. > The race issue was just an example of bugfixes that we want to get by default. Obviously if there's something that we must have, we will bump the requirement at configure time. >> >> >> Another option would be to add a wrapper around >> >> wl_proxy_marshal_constructor_versioned that calls >> >> wl_proxy_marshal_constructor_versioned if it's available and falls back >> >> to >> >> wl_proxy_marshal_constructor it it's not. Then pull in the header from >> >> wayland 1.10. That way it will build and even link against any >> >> libwayland >> >> version but will use the new versioned constructor function when it's >> >> available. >> > >> > I don't like the above option. I think Waffle should *not* provide its >> > own >> > re-implementation of any Wayland function. >> > >> As mentioned above if we check in the protocol you would need to >> copy/reimplement it in waffle. If we don't we'll effectively use the >> one provided by libwayland, at the expense of writing a 3-4 line patch >> every time things break. Which admittedly is not that often. > > > Yup. But it's not like it contains any real code. It's just thin wrappers > around actual libwayland functions. > With the above "it" you're talking about the generated headers, correct ? Having to reimplement the wayland fixes within waffle and/or keeping close eye on the wayland list to know when we need to re-import the headers does not sound like something that would scale imho. >> >> > Is there a signficant benefit to Waffle if it uses the versioned >> > constructor? >> > Keep in mind that Waffle uses Wayland very minimally. It probably >> > doesn't >> > care about versioned objects. >> > >> I'd imagine at some point we might case. With my proposal (and patch >> should be out) in place this is just detail which we don't need to >> know ;-) > > > If we care but we still want to use old wayland versions, you can always > stub in a waffle provided implementation of the new function if it's > missing. The waffle implementation would just implement the new one in > terms of the old one. That said, I don't think we care about either of the > two issues above enough to bother. > As mentioned before - in one case we'll write stubs, in the other case we'll import header(s) and write stubs. The former seems like the better option imho :-) >> >> >> In either case, I think checking wayland-client-protocol.h into the >> >> repo is >> >> a must. >> > >> > I'm convinced. >> Unfortunately I'm not ;'-( > > > Are you now? > Not quite I'm afraid. As a queue, I'm doing to (slightly) beat the SDL drum. SDL caters for everyone's needs and has a much wider exposure than waffle. I'm suggesting the exact same approach like the one they opted for ;-) Emil _______________________________________________ waffle mailing list waffle@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/waffle
