On Fri, Apr 15, 2016 at 1:56 PM, Chad Versace <chad.vers...@intel.com> wrote:
> 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? > 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. > 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. > 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. If you don't care about newer features, then just checking in the 1.9 header is probably fine. --Jason > > In either case, I think checking wayland-client-protocol.h into the repo > is > > a must. > > I'm convinced. >
_______________________________________________ waffle mailing list waffle@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/waffle
