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

[Derek Foreman]

On 2018-01-19 01:22 AM, Jonas Ådahl wrote:
> 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.
> 

Ok, I guess at this point we should drop it from consideration for the 
next release.

Pekka's synopsis is a really good status report for anyone interested in 
picking this up later without reading all the previous threads.

Thanks all,
Derek

> Jonas
> 
>>
>>
>> Thanks,
>> pq
> 
>