On 04/15/2016 09:36 AM, Jason Ekstrand wrote: > On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov <emil.l.veli...@gmail.com > <mailto:emil.l.veli...@gmail.com>> wrote: > > On 15 April 2016 at 03:32, Michel Dänzer <mic...@daenzer.net > <mailto:mic...@daenzer.net>> wrote: > > On 15.04.2016 11:14, Michel Dänzer wrote: > >> On 14.04.2016 22:16, Emil Velikov wrote: > >>> On 14 April 2016 at 09:23, Michel Dänzer <mic...@daenzer.net > <mailto:mic...@daenzer.net>> wrote: > >>>> From: Michel Dänzer <michel.daen...@amd.com > <mailto:michel.daen...@amd.com>> > >>>> > >>>> Fixes build failure due to wl_proxy_marshal_constructor_versioned > being > >>>> unresolved when building against current wayland. > >>>> > >>>> This API was introduced in wayland 1.9.91 by commit 557032e3 ("Track > >>>> protocol object versions inside wl_proxy."). The waffle code doesn't > >>>> reference wl_proxy_marshal_constructor_versioned directly but > >>>> indirectly via wayland-scanner. > >>>> > >>>> v2: > >>>> * Add paragraph about how wl_proxy_marshal_constructor_versioned was > >>>> introduced. (Emil Velikov) > >>>> * Only resolve wl_proxy_marshal_constructor_versioned with wayland >= > >>>> 1.9.91. > >>>> > >>>> Signed-off-by: Michel Dänzer <michel.daen...@amd.com > <mailto:michel.daen...@amd.com>> > >>>> --- > >>>> src/waffle/wayland/wayland_wrapper.c | 5 +++++ > >>>> src/waffle/wayland/wayland_wrapper.h | 8 ++++++++ > >>>> 2 files changed, 13 insertions(+) > >>>> > >>>> diff --git a/src/waffle/wayland/wayland_wrapper.c > b/src/waffle/wayland/wayland_wrapper.c > >>>> index 6ffd5a9..fb66f9a 100644 > >>>> --- a/src/waffle/wayland/wayland_wrapper.c > >>>> +++ b/src/waffle/wayland/wayland_wrapper.c > >>>> @@ -106,6 +106,11 @@ wayland_wrapper_init(void) > >>>> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_add_listener); > >>>> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal); > >>>> RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor); > >>>> +#if WAYLAND_VERSION_MAJOR == 1 && \ > >>>> + (WAYLAND_VERSION_MINOR > 9 || \ > >>>> + (WAYLAND_VERSION_MINOR == 9 && WAYLAND_VERSION_MICRO >= 91)) > >>>> + > RETRIEVE_WL_CLIENT_SYMBOL(wl_proxy_marshal_constructor_versioned); > >>>> +#endif > >>>> #undef RETRIEVE_WL_CLIENT_SYMBOL > >>>> > >>> I am slightly worried about this approach. It adds a so called 'hidden > >>> dependency' and with it a possibility of things going horribly wrong. > >>> It is something that we try to avoid with mesa as the deps version at > >>> build time != run-time one. Or in other words, one might build against > >>> wayland 1.9 and things will go crazy as you run wayland 1.10, or vice > >>> versa.
I prefer to avoid adding this "hidden dependency" (as Emil calls it) for a Wayland function that Waffle doesn't use or need. Jason's solution of importing wayland-client-protocol.h avoids that dependency, and it also prevents this build-breakage problem from occuring in the future. > I think this is actually mostly ok. In the Wayland project, we were very > careful to ensure that anything built against 1.9 would work when linked > against 1.10. This should continue to be the case even with waffle's > shim-layer. > > The problem is not that libwayland added a new symbol nor is it a problem > that wayland-protocol.h was updated to use that new symbol. The problem is > that waffle sticks a shim layer in between wayland-protocol.h and libwayland. > This makes the wayland-protocol.h file effectively internal to waffle but > still updatable by the distro. This is a layering violation. To keep this > from happening in the future, you probably want to just check a version of > wayland-client-protocol.h into the waffle repo so that it doesn't change out > from under you and make waffle just use wayland-client-core.h. You can even > check in the version from libwayland 1.9 if you'd like to keep waffle > building against older versions. I think I understand you, but I'm not confident. Wayland's header dependencies are, we can all admit, confusing. If Waffle does the following... a. Check into the repo the wayland-client-protocol.h from Wayland 1.9. ... then ... c. Waffle will successfully build against distro-provided Wayland headers for wayland >= 1.9. Specifically, the system's wayland-client.h will include Waffle's imported wayland-client-protocol.h, and nothing will explode. d. If Waffle is built against the system's wayland-client.h from Wayland 1.x (where x >= 9), the libwaffle can successfully dlopen and run against libwayland 1.y (where y > x). Jason, is that correct? To allow Waffle to continue building against older Wayland version, we may be able to import Wayland 1.8's (instead of 1.9's) wayland-client-protocol.h, as 1.8 is the first release in which the wayland-client-protocol.h was split out from wayland-client.h. > 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. 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. > In either case, I think checking wayland-client-protocol.h into the repo is > a must. I'm convinced. _______________________________________________ waffle mailing list firstname.lastname@example.org https://lists.freedesktop.org/mailman/listinfo/waffle