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

[Emil Velikov]

On 15 September 2016 at 11:47, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 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.
>
Afaict (I'm looking at the mesa generated files yet they should be
similar/identical to others):
 - The generated headers contain only the *_interface symbol
declaration. None of the generated code makes use of it.
 - Both client and server, take const pointer to the symbol in order
to bind an instance (client) and create the global (?) resource
(server).
Which in itself requires a provider for the said symbol(s). I believe
the latter is what you meant with "don't work", correct ?

> (Confusingly, the server side also has *_interface as a struct type in
> addition to the variable.)
>
Indeed. It was fun first time I saw it ;-)

>> > 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.
>
Which (m)any new servers would like to do.

>> 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.
>
As above. Furthermore any client may wish to bind an instance of the
new X and Y as well A (for whatever reason), even if A isn't created
by libfoo-server. Disclaimer: not 100% sure if the latter will
work/isn't against wayland design policy.

> 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?
>
Having libfoo library which ships with respective header and exports
the specific _interface symbol(s) is a good idea IMHO.
Regardless of the language (used to write or utilise the library) and
number of projects that use the interface, one can employ
dependency/version tracking alongside the version checks that wayland
provides (the ones you mention below).

Projects that have a protocol/interface that is used only internally,
can either static link or use a private shared library.

>> > 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.
>
When there are multiple providers for the *_interface symbol(s), which
one gets picked is an interesting question. Combine that with the lack
of MIN(...) in (some) clients, equals a recipe for disaster.
Admittedly this (complete lack of MIN) is an application bug, yet it's
something that cannot (should not) be fixed/handled in wayland,
correct ?

> After all, people already use the headers they know are good, so getting
> a mismatching symbol for *_interface at runtime is quite a surprise.
>
Personally I haven't had bad experiences but as we get more wayland
programs and people using those, it is unlikely that we'll be so
fortunate.

> 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.
>
>From my humble experience developers/engineers cannot invest into
understanding how each block works in deatail. Thus they take many
things as granted.
That said, it's perfectly reasonable that they do rely upon those,
even though they don't know/realise that, since things are exported by
default.

Hiding the symbols is likely to lead to local workarounds, in the form
of "let's check-in this generated files and fix them because
wayland-scanner produces strange/broken files". Sadly we've all seen a
fair few similar cases.

>> 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.
>
Ack, I see - so libwayland-client and libwayland-server will expose
the _interface symbols ?

Isn't this the case that we started with - which afaict is far from
ideal. This can easily be resolved by introducing a shared lib,
libwayland-interface.so (as mentioned above). This is how this patch
series resolves all the other duplicated symbols (by introducing
libwayland-utils.so).

Similar to libwayland-util.so, any old and new projects will continue
to work and the symbol duplication will be resolved.
Any projects which introduce add/their own protocol can be advised to
have a shared lib, by updating wayland-scanner to produce a meaningful
message.
Obviously they can still continue to static link, with the message
suggesting how to "hide" the _interface symbols if they choose to do
so.

> Mesa must stop exporting the wl_drm.xml symbols.
>
I know that giving vaapi as an example is discouraged, yet doing so
will break the library :-\
I'm not sure how many others are in a similar boat and deliberately
breaking apps is (almost) never a wise move.

tldr: I'm suggesting creating a libwayland-interface.so shared library
and updating wayland-scanner to suggest/educate devs to either a)
create their own shared ones as needed or b) how to hide the symbols
if they opt to static link the protocol code.

Using shared library:
pros: code reuse, single provider for the _interface symbol.
cons: extra bit in dependency tracking/tree

Using static linking:
pros: simpler dependency tree/tracking
cons: 'bloated' binaries, duplicated symbols which may or may not be
conflicting (in one keeps the WL_EXPORT), broken applications (if one
drops WL_EXPORT)

As always, thanks for the great input Pekka !

Emil