Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Emil Velikov]

On 23 June 2016 at 00:58, Jason Ekstrand  wrote:
>
>
> On Tue, Jun 21, 2016 at 9:06 PM, Chad Versace 
> 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 
>> > > wrote:
>> > > > On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov
>> > > > 
>> > > > wrote:
>> > > >>
>> > > >> On 16 April 2016 at 22:06, Jason Ekstrand 
>> > > >> 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.
>
IMHO that should be addressed in wayland (or the generator that
produces the header). One way to do it is the 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Jason Ekstrand]

On Tue, Jun 21, 2016 at 9:06 PM, Chad Versace 
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 
> wrote:
> > > > On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov <
> emil.l.veli...@gmail.com>
> > > > wrote:
> > > >>
> > > >> On 16 April 2016 at 22:06, Jason Ekstrand 
> 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
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Chad Versace]

On Thu 14 Apr 2016, Michel Dänzer wrote:
> From: Michel Dänzer 
> 
> 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 
> ---
>  src/waffle/wayland/wayland_wrapper.c | 5 +
>  src/waffle/wayland/wayland_wrapper.h | 8 
>  2 files changed, 13 insertions(+)

Michel, thanks for the patch. It's merged to master.

I squashed a small fix into your patch: `#include `
was needed to make your version check work.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Chad Versace]

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  wrote:
> > > On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov 
> > > wrote:
> > >>
> > >> On 16 April 2016 at 22:06, Jason Ekstrand  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.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Chad Versace]

On Sun 17 Apr 2016, Emil Velikov wrote:
> On 17 April 2016 at 01:41, Jason Ekstrand  wrote:
> > On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov 
> > wrote:
> >>
> >> On 16 April 2016 at 22:06, Jason Ekstrand  wrote:
> >>
> >> >> Then again importing bits from other projects tend to be quite a nasty
> >> >> solution. The place where I borrowed the idea from (libSDL) does/did
> >> >> not imports the protocol.
> >> >
> >> >
> >> > I don't think this is as bad an idea as you seem to think.  I'll explain
> >> > a
> >> > bit further down.

[snip]

> >> >> >> 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.
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Emil Velikov]

On 17 April 2016 at 01:41, Jason Ekstrand  wrote:
> On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov 
> wrote:
>>
>> On 16 April 2016 at 22:06, Jason Ekstrand  wrote:
>>
>> >> Then again importing bits from other projects tend to be quite a nasty
>> >> solution. The place where I borrowed the idea from (libSDL) does/did
>> >> not imports the protocol.
>> >
>> >
>> > I don't think this is as bad an idea as you seem to think.  I'll explain
>> > a
>> > bit further down.
>> >
>>  If it's a good idea why aren't others doing it ?> cheeky>
>> Please don't take the above too seriously.
>>
>> >> This is precisely the subtlety in the whole thing. The wayland
>> >> protocol tried (and I believe so far is) backwards compatible, then
>> >> again the library that we use is not. This is a fundamental difference
>> >> that causes these headaches.
>> >>
>> >> If one is to import protocol vX, what happens as libwayland tries to
>> >> use feature A or B introduced in newer protocols via the same API ?
>> >> They have to reimplement everything on their own - bad idea.
>> >>
>> >> How about the earlier patch was resolving another ABI break ? The
>> >> developers rightfully broke it because things were racy. Do we want to
>> >> check-in old version and be forced to use the racy interface ?
>> >
>> >
>> > Client-side libwayland has *never* had an ABI break since version 1.0.
>> > The
>> > libwayland-client ABI is very small (one or two dozen functions) and has
>> > always maintained backwards-compatibility.  We've made some pretty crazy
>> > changes to libwayland's internals and worked very hard to maintain
>> > compatibility.
>> >
>> > What has changed over time is the header file and the exact functions
>> > that
>> > it calls.  The protocol headers (as opposed to wayland-client-core.h)
>> > contain only static inline wrapper functions that implement the protocol
>> > in
>> > terms of the stable ABI provided by libwayland-client.so.  As new
>> > features
>> > get added to libwayland-client to remove race conditions, provide object
>> > versioning, or any other improvements, the wayland-client-protocol
>> > headers
>> > get updated to take advantage of the new functionality.  That way
>> > clients
>> > built against new versions of libwayland-client automatically get the
>> > upgrade without any developer intervention.  It's really a pretty good
>> > system.
>> >
>> Interesting. Not too long ago I asked around (#xorg-devel iirc) about
>> the proper terminology in a similar case and ABI popped up.
>>
>> I believe the term ABI is correct, as if we compile program X against
>> wayland N+1 it may not be compatible with wayland N. Where if the very
>> same application is rebuilt against wayland N it will work just fine.
>> Note: program X has only a single code path - it does not detect nor
>> differentiate between the different versions of wayland.
>> This does sound like the definition of ABI break to me.
>>
>> I'm likely missing something here. If the above is off what is the
>> correct terminology ?
>> How are the symbols provided by libwayland called ? How about the
>> user/programmer facing ones, provided by the headers ?
>
>
> Let's start with terms.  API or Application Programming Interface is a
> general term referring to the functions, types, structs, enums, etc that
> allows an application (or other library) to interact with a library.  ABI or
> Application Binary Interface refers to the binary mechanism for interacting
> with a library.
>
> As an example of the distinction, let's say I have an enum value.  If I
> change the name but not the value, that's an API change but not an ABI
> change.  Applications compiled against an old header will work just fine
> with a new version of the library because they pass in the same old value
> it's expecting.  Applications compiled against a new version will have to be
> updated because the name of the thing changed.
>
> On the other side of the fence, suppose I change the value but not the name.
> This is an ABI change but not an API change.  Applications built against an
> old version will fail on newer versions because they will pass in the wrong
> value.  However, if you rebuild your application against the new version, it
> will build just fine (it has the same name) and the value will get updated
> so it works again.
>
> Do you see the difference?  Another way to think about it is that the ABI is
> the actual symbols exported by libwayland-client.so and whatever magic
> constants etc. it expects while the API is the stuff contained in the header
> files.
>
> The only thing that Wayland guarantees will be backwards-compatible is the
> ABI and the wayland-core API.  The header files should be considered "nice
> little helpers" and not part of the core API.  In the usual case, the header
> gets completely compiled into your program and the only interface point
> between your binary and libwayland.so is the core 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Jason Ekstrand]

On Sat, Apr 16, 2016 at 4:12 PM, Emil Velikov 
wrote:

> On 16 April 2016 at 22:06, Jason Ekstrand  wrote:
>
> >> Then again importing bits from other projects tend to be quite a nasty
> >> solution. The place where I borrowed the idea from (libSDL) does/did
> >> not imports the protocol.
> >
> >
> > I don't think this is as bad an idea as you seem to think.  I'll explain
> a
> > bit further down.
> >
>  If it's a good idea why aren't others doing it ? cheeky>
> Please don't take the above too seriously.
>
> >> This is precisely the subtlety in the whole thing. The wayland
> >> protocol tried (and I believe so far is) backwards compatible, then
> >> again the library that we use is not. This is a fundamental difference
> >> that causes these headaches.
> >>
> >> If one is to import protocol vX, what happens as libwayland tries to
> >> use feature A or B introduced in newer protocols via the same API ?
> >> They have to reimplement everything on their own - bad idea.
> >>
> >> How about the earlier patch was resolving another ABI break ? The
> >> developers rightfully broke it because things were racy. Do we want to
> >> check-in old version and be forced to use the racy interface ?
> >
> >
> > Client-side libwayland has *never* had an ABI break since version 1.0.
> The
> > libwayland-client ABI is very small (one or two dozen functions) and has
> > always maintained backwards-compatibility.  We've made some pretty crazy
> > changes to libwayland's internals and worked very hard to maintain
> > compatibility.
> >
> > What has changed over time is the header file and the exact functions
> that
> > it calls.  The protocol headers (as opposed to wayland-client-core.h)
> > contain only static inline wrapper functions that implement the protocol
> in
> > terms of the stable ABI provided by libwayland-client.so.  As new
> features
> > get added to libwayland-client to remove race conditions, provide object
> > versioning, or any other improvements, the wayland-client-protocol
> headers
> > get updated to take advantage of the new functionality.  That way clients
> > built against new versions of libwayland-client automatically get the
> > upgrade without any developer intervention.  It's really a pretty good
> > system.
> >
> Interesting. Not too long ago I asked around (#xorg-devel iirc) about
> the proper terminology in a similar case and ABI popped up.
>
> I believe the term ABI is correct, as if we compile program X against
> wayland N+1 it may not be compatible with wayland N. Where if the very
> same application is rebuilt against wayland N it will work just fine.
> Note: program X has only a single code path - it does not detect nor
> differentiate between the different versions of wayland.
> This does sound like the definition of ABI break to me.
>
> I'm likely missing something here. If the above is off what is the
> correct terminology ?
> How are the symbols provided by libwayland called ? How about the
> user/programmer facing ones, provided by the headers ?
>

Let's start with terms.  API or Application Programming Interface is a
general term referring to the functions, types, structs, enums, etc that
allows an application (or other library) to interact with a library.  ABI
or Application Binary Interface refers to the binary mechanism for
interacting with a library.

As an example of the distinction, let's say I have an enum value.  If I
change the name but not the value, that's an API change but not an ABI
change.  Applications compiled against an old header will work just fine
with a new version of the library because they pass in the same old value
it's expecting.  Applications compiled against a new version will have to
be updated because the name of the thing changed.

On the other side of the fence, suppose I change the value but not the
name.  This is an ABI change but not an API change.  Applications built
against an old version will fail on newer versions because they will pass
in the wrong value.  However, if you rebuild your application against the
new version, it will build just fine (it has the same name) and the value
will get updated so it works again.

Do you see the difference?  Another way to think about it is that the ABI
is the actual symbols exported by libwayland-client.so and whatever magic
constants etc. it expects while the API is the stuff contained in the
header files.

The only thing that Wayland guarantees will be backwards-compatible is the
ABI and the wayland-core API.  The header files should be considered "nice
little helpers" and not part of the core API.  In the usual case, the
header gets completely compiled into your program and the only interface
point between your binary and libwayland.so is the core API.


> > The problem is waffle's usage of libwayland and its generated headers.
> > Because waffle inserts its own shim layer in between, there are two
> versions
> > of the libwayland API in play: the one provided 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Jason Ekstrand]

On Fri, Apr 15, 2016 at 3:20 PM, Emil Velikov 
wrote:

> On 15 April 2016 at 21:56, Chad Versace  wrote:
> > On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
> >> On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov  > wrote:
> >>
> >> On 15 April 2016 at 03:32, Michel Dänzer  > 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  > wrote:
> >>  From: Michel Dänzer >
> >> 
> >>  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 >
> >>  ---
> >>   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.
> >
> Then again importing bits from other projects tend to be quite a nasty
> solution. The place where I borrowed the idea from (libSDL) does/did
> not imports the protocol.
>

I don't think this is as bad an idea as you seem to think.  I'll explain a
bit further down.


> >> 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.
> >

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Emil Velikov]

On 15 April 2016 at 21:56, Chad Versace  wrote:
> On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
>> On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov > > wrote:
>>
>> On 15 April 2016 at 03:32, Michel Dänzer > > 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 > > wrote:
>>  From: Michel Dänzer > >
>> 
>>  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 > >
>>  ---
>>   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.
>
Then again importing bits from other projects tend to be quite a nasty
solution. The place where I borrowed the idea from (libSDL) does/did
not imports the protocol.

>> 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 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Jason Ekstrand]

On Fri, Apr 15, 2016 at 1:56 PM, Chad Versace 
wrote:

> On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
> > On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov  > wrote:
> >
> > On 15 April 2016 at 03:32, Michel Dänzer  > 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  > wrote:
> >  From: Michel Dänzer >
> > 
> >  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 >
> >  ---
> >   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
>

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Chad Versace]

On 04/15/2016 09:36 AM, Jason Ekstrand wrote:
> On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov  > wrote:
> 
> On 15 April 2016 at 03:32, Michel Dänzer  > 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  > wrote:
>  From: Michel Dänzer  >
> 
>  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  >
>  ---
>   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 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Jason Ekstrand]

On Fri, Apr 15, 2016 at 3:03 AM, Emil Velikov 
wrote:

> On 15 April 2016 at 03:32, Michel Dänzer  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  wrote:
>  From: Michel Dänzer 
> 
>  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 
>  ---
>   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.
>

Sorry for the late reply.  I tried to reply yesterday but wasn't a list
member yet so it ended up in the moderation bitbucket.

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.

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.

In either case, I think checking wayland-client-protocol.h into the repo is
a must.


> >>> Obviously that's not perfect, although unavoidable. Why ? As distros
> >>> do not know about the requirement (i.e. it's not mandated at configure
> >>> time) they won't rebuild and things won't work. At the same time if
> >>> they do rebuild (again without the explicit requirement), things will
> >>> break if one needs to revert to older (yet still in version range as
> >>> per the deps list) wayland.
> >>
> >> That's not true at least for Debian and derivatives, which keep track of
> >> which symbols were added in which version and generate accordingly
> >> versioned dependencies.
> >
> > It occurred to me (just after sending out the previous post, sigh...)
> > that this automatic mechanism might not work for waffle's dependency on
> > wayland if we don't link the wayland libraries directly. Even so, IME
> > this is a common issue distro maintainers of libraries have to deal
> > with, nothing particularly tricky.
> >
> Interesting - last time I've played around with Debian/derivatives, it
> 

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Emil Velikov]

On 15 April 2016 at 03:32, Michel Dänzer  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  wrote:
 From: Michel Dänzer 

 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 
 ---
  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.
>>>
>>> Obviously that's not perfect, although unavoidable. Why ? As distros
>>> do not know about the requirement (i.e. it's not mandated at configure
>>> time) they won't rebuild and things won't work. At the same time if
>>> they do rebuild (again without the explicit requirement), things will
>>> break if one needs to revert to older (yet still in version range as
>>> per the deps list) wayland.
>>
>> That's not true at least for Debian and derivatives, which keep track of
>> which symbols were added in which version and generate accordingly
>> versioned dependencies.
>
> It occurred to me (just after sending out the previous post, sigh...)
> that this automatic mechanism might not work for waffle's dependency on
> wayland if we don't link the wayland libraries directly. Even so, IME
> this is a common issue distro maintainers of libraries have to deal
> with, nothing particularly tricky.
>
Interesting - last time I've played around with Debian/derivatives, it
did track the symbols exposed by the libraries although it did not
check if anyone that depends on the new symbol needs to be rebuild and
more importantly the versions need to be bumped. Sadly other
distributions do not do that to this moment (afaict) - Arch, Gentoo,
Fedora (?), Suse (?).

To put it bluntly - with one approach things will eventually break
[and get fixed by distro maintainers], while with the other things
will just work.

Afaict the latter does add much more code/complexity over the former, right ?

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Michel Dänzer]

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  wrote:
>>> From: Michel Dänzer 
>>>
>>> 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 
>>> ---
>>>  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.
>>
>> Obviously that's not perfect, although unavoidable. Why ? As distros
>> do not know about the requirement (i.e. it's not mandated at configure
>> time) they won't rebuild and things won't work. At the same time if
>> they do rebuild (again without the explicit requirement), things will
>> break if one needs to revert to older (yet still in version range as
>> per the deps list) wayland.
> 
> That's not true at least for Debian and derivatives, which keep track of
> which symbols were added in which version and generate accordingly
> versioned dependencies.

It occurred to me (just after sending out the previous post, sigh...)
that this automatic mechanism might not work for waffle's dependency on
wayland if we don't link the wayland libraries directly. Even so, IME
this is a common issue distro maintainers of libraries have to deal
with, nothing particularly tricky.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Michel Dänzer]

On 14.04.2016 22:16, Emil Velikov wrote:
> On 14 April 2016 at 09:23, Michel Dänzer  wrote:
>> From: Michel Dänzer 
>>
>> 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 
>> ---
>>  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.
> 
> Obviously that's not perfect, although unavoidable. Why ? As distros
> do not know about the requirement (i.e. it's not mandated at configure
> time) they won't rebuild and things won't work. At the same time if
> they do rebuild (again without the explicit requirement), things will
> break if one needs to revert to older (yet still in version range as
> per the deps list) wayland.

That's not true at least for Debian and derivatives, which keep track of
which symbols were added in which version and generate accordingly
versioned dependencies. If other distros aren't doing this yet, that's
not upstream's problem.


> TL;DR: The situation is quite sensitive and fragile. The only robust
> solutions that I can think of are: a) non-fatal (only for newer
> symbols) dlsym

The problem with that is that we wouldn't catch the lack of a symbol
which is really required, even when we know perfectly well that it's
required (we know which versions of wayland-scanner generate references
to which symbols).


> or b) bumping the req. version at configure time.

That's not necessary.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned v2

[Emil Velikov]

On 14 April 2016 at 09:23, Michel Dänzer  wrote:
> From: Michel Dänzer 
>
> 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 
> ---
>  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.

Obviously that's not perfect, although unavoidable. Why ? As distros
do not know about the requirement (i.e. it's not mandated at configure
time) they won't rebuild and things won't work. At the same time if
they do rebuild (again without the explicit requirement), things will
break if one needs to revert to older (yet still in version range as
per the deps list) wayland.

TL;DR: The situation is quite sensitive and fragile. The only robust
solutions that I can think of are: a) non-fatal (only for newer
symbols) dlsym or b) bumping the req. version at configure time.

-Emil
___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle

Re: [waffle] [PATCH] wayland: Wrap wl_proxy_marshal_constructor_versioned

[Michel Dänzer]

On 08.04.2016 00:28, Emil Velikov wrote:
> On 7 April 2016 at 09:20, Michel Dänzer  wrote:
>> From: Michel Dänzer 
>>
>> Fixes build failure due to wl_proxy_marshal_constructor_versioned being
>> unresolved when building against current wayland.
>>
> Thanks for that patch. Can you please add the commit (and/or version)
> where this API is introduced ?

Added in v2.


> I recently sent out a similar patch [1] can you please take a look ? I
> would love to hear your input on the proposed topic.
[...]
> [1] https://lists.freedesktop.org/archives/waffle/2016-April/001406.html

I mostly agree with your analysis, but instead of always allowing dlsym
to fail for some symbols, I think it's better to guard their
RETRIEVE_WL_CLIENT_SYMBOL lines by version checks as I did in v2.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
waffle mailing list
waffle@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/waffle