[RFC libwayland] Track protocol object versions inside wl_proxy.

Thu Apr 10 07:42:55 PDT 2014

On Thu, Apr 10, 2014 at 6:37 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> Hi Jason,
>
> thanks for working on this, it does seem very useful, practically a
> mandatory feature to support.
>

Hi Pekka,
Yeah, I've been itching to knock this out for a while.  Just finally got
around to it.  Comments below.

>
> <Thinking out loud>
>
> We already had the versioning rules documented, right? Whenever bumping
> an interface version, also the parent interface (factory) version must
> be bumped. That makes the version inference you implemented work. It
> also means that an interface may at runtime get a version number bigger
> than what the protocol spec has ever defined.
>
> Oh but it also goes the other way.
>
> Let's say interface A has methods to create objects with interfaces B
> and C.
>
> Let A.ver = 1, B.ver = 1, and C.ver = 1 in the specification.
>
> Bump B: A.ver = 2, B.ver = 2, C.ver = 1
> Bump B: A.ver = 3, B.ver = 3, C.ver = 1
> Bump C: A.ver = 4, B.ver = 3, C.ver = x
>
> Now, wl_proxy_get_version() for objects of type B and C always returns
> the version of A, essentially.
>
> Therefore, to properly handle the bump of C, we must have x=4. That is,
> when a child interface version is bumped, it must be set to the
> parent's new version, not just incremented by one.
>
> If x=2, then if A.ver was 2 or 3, it would falsely claim that C has the
> new feature that was added in the C bump.
>
> Oh yeah, we do have it documented in
> http://wayland.freedesktop.org/docs/html/sect-Protocol-Versioning.html
>
> Seems to work, and I suppose the existing revisions in protocols in
> wayland and weston follow these rules already?
>
> </out loud>
>

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.

>
> On Wed, 2 Apr 2014 09:48:29 -0500
> Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> > It's worth noting that there is one small backwards-compatability issue
> > here.  Namely, if the client is built against protocol stubs from an
> > earlier version of libwayland but links against a library built against a
> > newer version, then all objects created by the client will report a
> version
> > of 1.  This is because the old api uses wl_proxy_marshal_constructor in
> > wl_registry_bind so all objects will inherit the protocol version of
> > wl_display which is 1.  The library the client linked against is aware of
> > the wl_proxy_version function but has no way of knowing that the library
> > does not.
>
> I was about to say that wl_registry_bind() does pass the version to
> wl_proxy_marshal_constructor, but that indeed is in the request
> arguments and not a mandatory argument.
>
> But wl_proxy_marshal_constructor() would still have all the information
> it needs, if we special-case wl_registry.bind inside it. Ugly, but I
> guess it'd work for wl_registry. Would that make the
> backward-compatibility issue go away? In all other cases you would take
> the version from the parent wl_proxy, which you always have available
> in wl_proxy_marshal_constructor(), and the versioned variant would not
> be needed?
>

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.

> But I guess it would still be broken on any other request that used the
> interfaceless format of new_id?
>

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.

>
> > One possible solution for this is to set the version of wl_display to
> zero
> > and use zero to mean "unversioned".  Then, if a library wants to use
> > something that's not strictly backwards-compatable, it can check for zero
> > and use whatever it's non-versioned fallback is.
>

Thoughts on this? ^^

Thanks,
--Jason Ekstrand

>
>
> Thanks,
> pq
>
>
> > --Jason Ekstrand
> > On Apr 2, 2014 12:28 AM, "Jason Ekstrand" <jason at jlekstrand.net> wrote:
> >
> > > This provides a standardized mechanism for tracking protocol object
> > > versions in client code.  The wl_display object is created with
> version 1.
> > > Every time an object is created from within wl_registry_bind, it gets
> the
> > > bound version.  Every other time an object is created, it simply
> inherits
> > > it's version from the parent object that created it.
> > >
> > >
> > > ---
> > > I've been meaning to scratch this itch for a while.  I've left it as
> an RFC
> > > for now because it builds, but I don't have any code to test it yet.
>  I've
> > > got something I'm hoping to hack on this weekend that will use it.  For
> > > now, feel free to comment.
> > >
> > >  src/scanner.c        |  29 +++++++++----
> > >  src/wayland-client.c | 112
> > > +++++++++++++++++++++++++++++++++++++++++++++++----
> > >  src/wayland-client.h |  13 ++++++
> > >  3 files changed, 139 insertions(+), 15 deletions(-)
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140410/f757a301/attachment-0001.html>