[PATCH wayland 0/4] Untangle the symbol export duplication

[Pekka Paalanen]

On Wed, 14 Sep 2016 16:57:17 +0100
Emil Velikov <emil.l.velikov at gmail.com> wrote:

> Hi Pekka,
> 
> Huge thanks for the input.
> 
> On 14 September 2016 at 14:35, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Tue, 30 Aug 2016 18:24:18 +0100
> > Emil Velikov <emil.l.velikov at gmail.com> wrote:
> >  
> >> Hi all,
> >>
> >> For a while I've noticed that on mesa side we have few providers of the
> >> wl_drm_interface symbol and literally all our 'wayland binaries' are
> >> linked against both the client and server wayland libs.
> >>
> >> After having a look, it seems that:
> >>  - the server exposes the interface symbols for 'inheritance' purposes,
> >> thus other servers can build upon these primitives while designing their
> >> own protocols (interfaces ?).  
> >
> > I have no idea what that means. wl_drm is private to Mesa.
> >
> > You are not supposed to use wl_drm in your apps, and *no-one* is
> > supposed to build on it by any other way than copying the XML file and
> > renaming everything. The recommended way would be to start writing XML
> > from scratch though.
> >  
> The wl_drm* symbols are what brought me here. AFAICT all the
> information/discussion is generic and not specific to it.
> 
> See below for more.
> 
> >>  - at the same time the client needs to have a reference to the
> >> interface symbol in order to register an instance of said interface.  
> >
> > Anyway, those details don't really matter at all, see below.
> >  
> >> This means that if the "wrong" symbol gets picked at runtime and the
> >> client does not correctly manage older versions in its
> >> .registry_handle_global function (yes we have a case or two of those in
> >> the wild) things will end up horribly wrong.
> >>
> >> I think that a better option would have been to distinguish the two
> >> (call one instance or the other singleton) and let the scanner attribute
> >> for the name difference. Regardless of that is that it's too late to
> >> change things since this would lead to breaking the ABI.
> >>
> >> One way around it to update the scanner to provide newer symbols and
> >> annotate the old ones as deprecated. This way when/if we break the ABI
> >> we can untangle things. One day...
> >>
> >> Does the above sound about right - can anyone let me know if I'm loosing
> >> the plot ?
> >>
> >> That said, I've went ahead and removed duplication where possible - by
> >> folding the util symbols into libwayland-util. Please check with patch 3/4
> >> how compatibility with existing and new users is be preserved.  
> >
> > Actually, the <foo>_interface symbols are not meant to be exported *at
> > all*. But there is a catch that is the source for all the confusion.
> >
> > For historical reasons, libwayland-server and libwayland-client are
> > designed to export the wl_*_interface pointers, because libwayland
> > ships the headers generated from wayland.xml.  
> 
> > The headers cannot work
> > if something does not provide the symbols from the generated
> > wayland-protocol.c file.  
> Having a dull moment - what do you mean with the above ? How can
> headers "work" or "not work" ?

Hi,

you can generate the headers and use them, but you cannot write all the
necessary code to get a working thing without also having access to the
symbols exported from wayland-protocol.c. To be specific, you can never
create any object type whose wl_*_interface symbol you do not have
access to. This applies both to server and client side.

Furthermore, the client side generated headers explicitly use the
*_interface symbols. On server side they are used by the hand-written
code in a compositor.

(Confusingly, the server side also has *_interface as a struct type in
addition to the variable.)

> > IMHO and in hindsight, this was a mistake - we
> > should have installed only the XML file and require everyone to
> > generate their own wayland.xml bindings to keep things consistent. But,
> > it's done, and now it is API and ABI, so we cannot change that.
> >  
> That may have been a good idea indeed.
> 
> > That is the reason why wayland-scanner emits all the WL_EXPORTs in the
> > *-protocol.c file. IMO it really should not do that, except for
> > wayland.xml itself only when building libwayland.
> >
> > It's an unfortunate accident that all libs using Wayland extensions
> > from wayland-protocols or anywhere are exporting the *_interface
> > symbols. There is really no need for that as I see. If two pieces of
> > software use the same extension, they can just (generate and) build in
> > their own copies of the *-protocol.c file.
> >  
> Note: I'll be using protocol and interface interchangeably.
> 
> Here's my take on it:
> Server foo implements protocol A which builds on top of the core
> wayland primitives/interfaces X and Y.
> For linking purposes server foo will depend on
> libwayland-{server,client} to provide the {X,Y}_interface symbols.

Yes and no.

You do not need X_interface if you only use the type as an argument in
your own requests and events.

X_interface is only needed when creating objects of that kind.

> Similarly on the client foo side, one needs to know the specifics of
> the interfaces A, X and/or Y in order to register/instantiate them.
> Which in itself, pulls the link time dependency of
> libwayland-{server,client}. With the latter being the recommended one
> afaict.

Again, only when creating objects of type X and Y, not when dealing
with any aspect of A.

You could technically have A create objects of type X, but we strongly
advice against it, because having the same type constructed from
independent parent-object trees will break interface versioning.

The reason why libwayland exports the interface symbols is because it
does not create the object of those types itself. That's the only
reason to export them in general, too.

> The only way around this would be to have all the protocol A, X and Y
> code statically linked inside both client/server foo. Which in itself,
> will cause binary bloat. From memory we're talking about ~20K for the
> core wayland ones. That is 20K for each binary that uses wayland -
> surely less than ideal.

IMO the problems caused by sharing well outweigh that. Or would you
rather have every XML file turned into a library with its own .so and
headers to be installed? Also the private protocol extensions that are
not meant to be used out side of a given project?

> > Libwayland has been written to cope just fine with multiple copies of
> > interface definitions, or it at least should.
> >  
> Can libwayland cope with clients which do not check versions and
> always assume that vX will be present ? I believe not, yet again I'm
> not sure if one can really achieve that. Speaking of which I should
> really send a patch or two to fix things :-\

I do not understand the question.

The interface version negotiation happens when you use wl_registry to
bind a global. The server advertises the version
MIN(server_implemented, available_in_symbols). The client will choose
version MIN(advertised, client_implemented, available_in_symbols).

People often ignore the available_in_symbols there, because they
generate their own .c and .h files straight from the XML file which
they know to be at least the expected version. Exporting the
*_interface symbols is what causes confusion there.

After all, people already use the headers they know are good, so getting
a mismatching symbol for *_interface at runtime is quite a surprise.

Hence, let's stop the exporting. That will solve the accidents of
getting not the *_interface symbol one compiled oneself.


> >
> > I would seriously suggest that we make wayland-scanner *not* emit any
> > WL_EXPORTs for the *-protocol.c file if at all possible. I would like
> > that to be the default, while libwayland build would opt-in for the
> > exports.
> >
> > Can we do that, or would it break any ABI we actually care about?
> >  
> How will one resolve the dependencies of the now exported _interface
> symbols at link time ?

No-one should be using them to begin with, unless they have
intentionally written a library their own interfaces similar to
libwayland exposing wayland.xml interfaces. In the latter case, it is
only an API break as they just need to tell wayland-scanner to export
stuff.

I would prefer wayland-scanner to default to not exporting, because I
believe most exports are accidental and might not get fixed otherwise.
If one exported intentionally, they will notice the break, and fix it.

> Hiding the interface symbols from libwayland-{server,client} will lead
> to a massive ABI break, since effectively everything (that I've
> looked) which uses wayland depends on a selection of those.

That is the only exception. I said: "while libwayland build would opt-in
for the exports." I never meant libwayland to stop exporting, just
everything else should stop exporting.

> "Everything" being: mesa (gbm, egl, vulkan), webkit2gtk,
> KF5/kwin/kscreen*, weston binaries, Qt wayland integration, mpv, vaapi
> (front end and drivers).

libwayland must continue exporting the wayland.xml symbols, yes. This
is the exception to the rule.

Mesa must stop exporting the wl_drm.xml symbols.


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