<div dir="ltr"><div>On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen <span dir="ltr"><<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Jason,<br>
<br>
thanks for working on this, it does seem very useful, practically a<br>
mandatory feature to support.<br></blockquote><div><div><br></div><div>Hi Pekka,</div><div>Yeah, I've been itching to knock this out for a while.  Just finally got around to it.  Comments below.</div></div><div> <br>
</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
<Thinking out loud><br>
<br>
We already had the versioning rules documented, right? Whenever bumping<br>
an interface version, also the parent interface (factory) version must<br>
be bumped. That makes the version inference you implemented work. It<br>
also means that an interface may at runtime get a version number bigger<br>
than what the protocol spec has ever defined.<br>
<br>
Oh but it also goes the other way.<br>
<br>
Let's say interface A has methods to create objects with interfaces B<br>
and C.<br>
<br>
Let A.ver = 1, B.ver = 1, and C.ver = 1 in the specification.<br>
<br>
Bump B: A.ver = 2, B.ver = 2, C.ver = 1<br>
Bump B: A.ver = 3, B.ver = 3, C.ver = 1<br>
Bump C: A.ver = 4, B.ver = 3, C.ver = x<br>
<br>
Now, wl_proxy_get_version() for objects of type B and C always returns<br>
the version of A, essentially.<br>
<br>
Therefore, to properly handle the bump of C, we must have x=4. That is,<br>
when a child interface version is bumped, it must be set to the<br>
parent's new version, not just incremented by one.<br>
<br>
If x=2, then if A.ver was 2 or 3, it would falsely claim that C has the<br>
new feature that was added in the C bump.<br>
<br>
Oh yeah, we do have it documented in<br>
<a href="http://wayland.freedesktop.org/docs/html/sect-Protocol-Versioning.html" target="_blank">http://wayland.freedesktop.org/docs/html/sect-Protocol-Versioning.html</a><br>
<br>
Seems to work, and I suppose the existing revisions in protocols in<br>
wayland and weston follow these rules already?<br>
<br>
</out loud><br></blockquote><div><br></div><div>Yes, that's a correct reading.  What it comes down to is that, since only the root of the object creation tree is versioned, the entire tree has one version.  Clients/servers are just going to have to deal with that.  Sure, we could make it take the maximum of the version of the parent proxy and the version listed in the wl_interface.  However, I don't think that this has a substantial benefit especially since the client and server are both in control of the version they advertise and the version they accept.  What it comes down to is that they'll have to use ranges instead of wl_proxy_version == something.  But we have no way to know what all the intermediate versions are, so they're going to have to do that for all but the latest version anyway.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><br>
On Wed, 2 Apr 2014 09:48:29 -0500<br>
Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
<br>
> It's worth noting that there is one small backwards-compatability issue<br>
> here.  Namely, if the client is built against protocol stubs from an<br>
> earlier version of libwayland but links against a library built against a<br>
> newer version, then all objects created by the client will report a version<br>
> of 1.  This is because the old api uses wl_proxy_marshal_constructor in<br>
> wl_registry_bind so all objects will inherit the protocol version of<br>
> wl_display which is 1.  The library the client linked against is aware of<br>
> the wl_proxy_version function but has no way of knowing that the library<br>
> does not.<br>
<br>
</div>I was about to say that wl_registry_bind() does pass the version to<br>
wl_proxy_marshal_constructor, but that indeed is in the request<br>
arguments and not a mandatory argument.<br>
<br>
But wl_proxy_marshal_constructor() would still have all the information<br>
it needs, if we special-case wl_registry.bind inside it. Ugly, but I<br>
guess it'd work for wl_registry. Would that make the<br>
backward-compatibility issue go away? In all other cases you would take<br>
the version from the parent wl_proxy, which you always have available<br>
in wl_proxy_marshal_constructor(), and the versioned variant would not<br>
be needed?<br></blockquote><div><br></div><div>Yes, we could do that, and I considered it.  However, it would only bump the compatibility issue back to wl_proxy_marshal_constructor.  Older code that uses wl_proxy_create inside of wl_registry_bind is still in trouble.  I don't think it's a huge savings to just bump the compatibility issues back to 1.3 rather than 1.4/1.5.  In the long run, I don't think it's worth the mess that we would create inside wl_proxy_marshal_constructor.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
But I guess it would still be broken on any other request that used the<br>
interfaceless format of new_id?<br></blockquote><div><br></div><div>Nope, that's not a problem.  The wayland-scanner program doesn't actually special case wl_registry_bind but interfaceless new_id's in general.  Anything else that specifies a new_id with no interface will hit the same code-path and get wl_proxy_marshal_constructor_versioned.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><br>
> One possible solution for this is to set the version of wl_display to zero<br>
> and use zero to mean "unversioned".  Then, if a library wants to use<br>
> something that's not strictly backwards-compatable, it can check for zero<br>
> and use whatever it's non-versioned fallback is.<br></div></blockquote><div><br></div><div>Thoughts on this? ^^</div><div><br></div><div>Thanks,</div><div>--Jason Ekstrand</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="">
<br>
<br>
</div>Thanks,<br>
pq<br>
<div class=""><div class="h5"><br>
<br>
> --Jason Ekstrand<br>
> On Apr 2, 2014 12:28 AM, "Jason Ekstrand" <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
><br>
> > This provides a standardized mechanism for tracking protocol object<br>
> > versions in client code.  The wl_display object is created with version 1.<br>
> > Every time an object is created from within wl_registry_bind, it gets the<br>
> > bound version.  Every other time an object is created, it simply inherits<br>
> > it's version from the parent object that created it.<br>
> ><br>
> ><br>
> > ---<br>
> > I've been meaning to scratch this itch for a while.  I've left it as an RFC<br>
> > for now because it builds, but I don't have any code to test it yet.  I've<br>
> > got something I'm hoping to hack on this weekend that will use it.  For<br>
> > now, feel free to comment.<br>
> ><br>
> >  src/scanner.c        |  29 +++++++++----<br>
> >  src/wayland-client.c | 112<br>
> > +++++++++++++++++++++++++++++++++++++++++++++++----<br>
> >  src/wayland-client.h |  13 ++++++<br>
> >  3 files changed, 139 insertions(+), 15 deletions(-)<br>
</div></div></blockquote></div><br></div></div>