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

Fri Jan 19 07:22:23 UTC 2018

On Thu, Jan 18, 2018 at 10:48:14AM +0200, Pekka Paalanen wrote:
> 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.

I don't see the point in keeping xdg-shell unstable v5 support in
weston, so from that point of view, *weston* I'd say don't need any
symbol prefix feature.

Jonas

> 
> 
> Thanks,
> pq