<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Fri, Apr 15, 2016 at 1:56 PM, Chad Versace <<a href="mailto:chad.versace@intel.com" target="_blank">chad.versace@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 04/15/2016 09:36 AM, Jason Ekstrand wrote: > On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a> <mailto:<a href="mailto:emil.l.velikov@gmail.com">emil.l.velikov@gmail.com</a>>> wrote: > > On 15 April 2016 at 03:32, Michel Dänzer <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a> <mailto:<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>>> 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 <<a href="mailto:michel@daenzer.net">michel@daenzer.net</a> <mailto:<a href="mailto:michel@daenzer.net">michel@daenzer.net</a>>> wrote: > >>>> From: Michel Dänzer <<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a> <mailto:<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a>>> > >>>> > >>>> 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 <<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a> <mailto:<a href="mailto:michel.daenzer@amd.com">michel.daenzer@amd.com</a>>> > >>>> --- > >>>> 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? </blockquote><div> </div><div>It should be, yes. It's best to make sure that the header file you stash in the repo uses wayland-client-core.h, but I think the include guards will probably be enough in any case. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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. </blockquote><div> </div><div>If you want newer features. In particular if you want EGL_EXT_swap_buffers_with_damage to work correctly, you need to use wl_surface version 4 (or maybe it's 5) and use the versioned constructors. </div><div>If you don't care about newer features, then just checking in the 1.9 header is probably fine. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > In either case, I think checking wayland-client-protocol.h into the repo is > a must. I'm convinced. </blockquote></div> </div></div>