[PATCH wayland 2/3] scanner: Allow adding a prefix to exported symbols

Thu Jan 18 08:48:14 UTC 2018

On Wed, 17 Jan 2018 15:43:07 -0600
Derek Foreman <derekf at osg.samsung.com> wrote:

> On 2017-08-18 07:41 AM, Pekka Paalanen wrote:
> > On Fri, 28 Jul 2017 14:06:23 +0100
> > Emil Velikov <emil.l.velikov at gmail.com> wrote:
> >   
> >> On 27 July 2017 at 14:01, Pekka Paalanen <ppaalanen at gmail.com> wrote:  
> >>> On Wed, 26 Jul 2017 16:09:32 +0100
> >>> Emil Velikov <emil.l.velikov at gmail.com> wrote:
> >>>     
> >>>> On 25 July 2017 at 10:24, Pekka Paalanen <ppaalanen at gmail.com> wrote:  
> >>>>> On Tue, 25 Jul 2017 15:25:58 +0800
> >>>>> Jonas Ådahl <jadahl at gmail.com> wrote:
> >>>>>     
> >>>>>> On Mon, Jul 24, 2017 at 02:16:04PM +0300, Pekka Paalanen wrote:  
> >>>>>>> On Mon,  3 Jul 2017 17:16:45 +0800
> >>>>>>> Jonas Ådahl <jadahl at gmail.com> wrote:
> >>>>>>>     
> >>>>>>>> Two different protocols may use interfaces with identical names.
> >>>>>>>> Implementing support for both those protocols would result in symbol
> >>>>>>>> clashes, as wayland-scanner generates symbols from the interface names.
> >>>>>>>>
> >>>>>>>> Make it possible to avoiding these clashes by adding a way to add a
> >>>>>>>> prefix to the symbols generated by wayland-scanner. Implementations
> >>>>>>>> (servers and clients) can then use these prefix:ed symbols to implement
> >>>>>>>> different objects with the same name.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> >>>>>>>> ---
> >>>>>>>>
> >>>>>>>> Something like this would be needed if a compositor/client wants to implement
> >>>>>>>> xdg-shell unstable v5 alongside xdg-shell stable, unless we want to rename all
> >>>>>>>> our xdg-shell interfaces. Implementing xdg-shell unstable v6 alongside
> >>>>>>>> xdg-shell stable does not have this issue.
> >>>>>>>>
> >>>>>>>> See issue raised here:
> >>>>>>>> https://lists.freedesktop.org/archives/wayland-devel/2017-June/034380.html
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Jonas
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>   src/scanner.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++-------------
> >>>>>>>>   1 file changed, 73 insertions(+), 21 deletions(-)  
> >>>>>>>
> >>>>>>>
> >>>>>>> Hi,
> >>>>>>>
> >>>>>>> while this seems to change the ABI symbol names, it does not change the
> >>>>>>> names in the documentation, and it does not change the names of
> >>>>>>> #defines of enums, or the inline functions. That means that this is not
> >>>>>>> enough to fulfill the purpose: being able to use two similarly named
> >>>>>>> but different protocols by adding a prefix.  
> >>>>>>
> >>>>>> The idea I had was rather that one would avoid changing any names on
> >>>>>> non-symbols. It'd still be possible to implement both by doing it in
> >>>>>> separate C files. I can see the point in adding the prefix on everything
> >>>>>> though, so I'll provide a patch for that.
> >>>>>>     
> >>>>>
> >>>>> Hi,
> >>>>>
> >>>>> recapping the discussion from IRC, we pretty much agreed that prefixing
> >>>>> is not a nice solution. Jonas found out that we cannot actually prefix
> >>>>> everything, because there usually are references to other protocol
> >>>>> things (like you would never want to prefix wl_surface). So it becomes
> >>>>> very hard to prefix things appropriately.  
> > 
> > ...
> >   
> >> Tl:Dr; I think that everything should be prefixed. See below for details.
> >>
> >>   - ABI symbols - to prevent symbol collision
> >>   - inline/other API - to minimise ambiguity, confusion, plus you can
> >> have multiple implementations in the same translation unit*
> >>
> >>
> >> *Some projects may want to support xdg-shell vX and vX+1 in the same file.
> >>   - The inline API from vX may work with vY or vise-versa. Yet it's
> >> easier to pass the wrong object to the inline function.
> >>   - One can (and will have) cases, where API foo() is available for
> >> only one version.
> >>   - If the function signature (of the inline API) differs the compiler
> >> will barf at you - say vX+1 adds extra argument to function bar()  
> > 
> > Yes, I agree, which is why I was not too fond of this patch originally.
> > This patch does not prefix API, but only ABI.
> > 
> > The difficulty is in what interfaces to prefix as a whole (ABI and API)
> > and what not. As an example, one would never want to prefix wl_surface,
> > yet extension interfaces use wl_surface as a message argument.
> > 
> > Jonas, I think we can discriminate between interfaces (protocol object
> > types) defined in the XML file being processed vs. external interfaces
> > from elsewhere (like wl_surface). Would that be good enough for
> > prefixing not just #defines and inline functions but the C types as
> > well?
> > 
> > The case where it would not work if one wants to prefix things from
> > both one.xml and two.xml, and two.xml used interfaces defined in
> > one.xml. Is that an acceptable limitation?
> > 
> > As a stretch I'd rather avoid for now, there could be an option to
> > prefix all external interfaces with another prefix, but it would not be
> > able to make a difference between things from wayland.xml and one.xml,
> > for instance.  
> 
> I think I'd like to avoid that too...
> 
> > Where are we at with the name collision problem in general now?  
> 
> Bit of a necropost, but was any kind of consensus in reach here? :)
> 
> Emil's nominated this for inclusion in an upcoming release, and it would 
> certainly get us out of the xdg-shell-v5 vs xdg-shell-stable collisions 
> we face now (if we intend to have weston support xdg-shell-stable 
> without dropping v5 at some point).
> 
> Does this have a strong use case outside of just dealing with the v5 vs 
> stable thing?  We could probably solve that with a sed script (in 
> wayland-protocols?)

Hi,

the original proposal was to prefix ABI symbols, and leave everything
else as is. Maybe writing down again how exactly that is supposed to be
used and what problems it solves would crystallize the idea. Perhaps in
the form of wayland-scanner user instructions which the original patch
seems to be lacking?

Earlier I didn't like prefixing only some bits of the API, but on
second thought, if that's all what's needed, maybe it isn't that bad
from hand-written code readability point of view?

The discussion showed that any other solution becomes hard and/or messy
compared to that. This is not a library ABI/API change either, just a
new operation mode in wayland-scanner.

I join you with the question about use cases and how badly do we need
this.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180118/f338c45f/attachment.sig>