[waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Jason Ekstrand]

On Tue, Jun 21, 2016 at 9:06 PM, Chad Versace <chad.versace at intel.com>
wrote:

> On Mon 20 Jun 2016, Chad Versace wrote:
> > On Sun 17 Apr 2016, Emil Velikov wrote:
> > > On 17 April 2016 at 01:41, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > > > On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov <
> emil.l.velikov at gmail.com>
> > > > wrote:
> > > >>
> > > >> On 16 April 2016 at 22:06, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> >
> > > >> >> >> 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 ;-)
> > > >
> > > >
> > > > I doubt its the "exact" same or they'd be having build breaks too.
> > > They do actually [1]. The person who fixed it is familiar wayland
> > > developer [2] yet he choose the same approach as me ;-)
> > >
> > > > If you
> > > > want to provide a sort of glue layer to an application, your
> suggestion of
> > > > "optional" entrypoints is probably exactly what you want.
> > > Indeed. Thank you.
> > >
> > > >  However, waffle
> > > > itself needs to call something and it either needs to be smart
> enough to
> > > > call the right thing depending on the libwayland version it just
> dlopened or
> > > > it needs to just always call old ones.
> > > >
> > > The cases of waffle being "dumb" (not being smart enough) are so
> > > infrequent, that it beats the added overhead that importing the header
> > > will bring.
> > >
> > > Thanks for the discussion. Hopefully you're seeing things from my POV
> ;-)
> > > Emil
> > >
> > > [1]
> https://github.com/spurious/SDL-mirror/tree/master/src/video/wayland/SDL_wayland{dyn.{c,h},sym.h}
> > > [2]
> https://github.com/spurious/SDL-mirror/commit/737415012588a2636794292129a2d39f2a28fe3c
> >
> > Both Jason's and Emil's approaches seem valid to me. And my preference
> > flip-flops every few minutes as I read the thread.
> >
> > In April, I was strongly convinced of Jason's position. Now I'm leaning
> > slightly toward's Emil's.
> >
> > I want to look more closely at SDL's approach (Emil, thanks for the
> > links), and then make a final decision. But it's late in the day for me
> > and my brain is done. Exhausted brains can't be trusted, so the decision
> > will have to wait until morning.
>
> Everyone, thanks for the lengthy discussion. The winner is... Michel's
> patch v2, which is basically Emil's and SDL's position.
>
> I decided against importing any Wayland headers, because the Wayland
> headers actually contain a lot of inline function *definitions*. When
> upstream Wayland applies bugfixes and improvements to those functions,
> by not using imported headers Waffle automatically receives the bugfixes
> and improvements simply by being rebuilt; this seems to be the intent of
> the Wayland authors for client projects. If Waffle were to use imported
> headers then, to receive the same improvements, someone (likely me)
> would need to diligently keep the imported headers up-to-date.
>
> As a bonus, Michel's patch is considerably smaller and requires less
> maintenance than an import-some-headers patch.
>
> And Michel's patch provides correct behavior, at least in my opinion:
>
>     - If a user or distro builds libwaffle against wayland < 1.10, then
>       that same libwaffle will continue to work with wayland >= 1.10.
>
>     - If a user or distro builds libwaffle against wayland == 1.10, then
>       the libwaffle will correctly emit an informative error message and
>       fail if it dlopens a libwayland-client < 1.10, thanks to the 'goto
>       error' in
> src/waffle/wayland/waylan_wrapper.c:RETRIEVE_WL_CLIENT_SYMBOL.
>       Specifically, the libwaffle will not crash or do undefined
>       behavior; it gracefully emits an error and fails responsibly.
>

This makes me a bit sad.  One of the problems that Michael's patch does
*not* solve is that, thanks to a Wayland header update (yay improvements!)
the waffle build broke.  All that's been accomplished on that front is that
the problem is papered over (we added the new entrypoing) and the next time
a Wayland header update comes along that uses another new entrypoint,
waffle will break again.  Maybe this is considered acceptable; that's not
really my call.

--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/waffle/attachments/20160622/c51f82b0/attachment.html>