[PATCH wayland] introduce new headers wayland-client/server-core.h

[Giulio Camuffo]

2015-04-28 12:48 GMT+03:00 Pekka Paalanen <ppaalanen at gmail.com>:
> On Tue, 28 Apr 2015 09:40:39 +0300
> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>
>> 2015-04-28 9:29 GMT+03:00 Marek Chalupa <mchqwerty at gmail.com>:
>> >
>> >
>> > On Mon, Apr 27, 2015 at 4:06 PM, Giulio Camuffo <giuliocamuffo at gmail.com>
>> > wrote:
>> >>
>> >> 2015-04-27 21:53 GMT+03:00 Bill Spitzak <spitzak at gmail.com>:
>> >> > On 04/26/2015 11:48 PM, Marek Chalupa wrote:
>> >> >
>> >> >>
>> >> >> Is the --include-core-headers option necessary? Until now the scanner
>> >> >> included wayland-client/server.h
>> >> >> which included the %-protocol.h, but since there are inclusion guards,
>> >> >> the protocol header was not
>> >> >> included again - so including wayland-client/server.h is the same as
>> >> >> including wayland-client/server-core.h, isn't it?
>> >> >> So including the core headers should be good for all cases.
>> >>
>> >> The problem is when you have an extension.xml, and generate
>> >> extension-client-protocol.h. Then you have some preexisting code
>> >> including that and relying on it including wayland-client.h and then
>> >> wayland-client-protocol.h, and if you use something from that it will
>> >> not build anymore, because wayland-client-core.h doesn't include it.
>> >>
>> >
>> > True...
>> > So when you want to include only core file and not whole
>> > wayland-client/server.h in generated headers?
>>
>> When you are writing new code or porting some old one, and you want to
>> use a wayland.xml from a different version than the libwayland.
>
> I'm going to talk about client headers, but it all applies equally to
> server headers too, so I won't bother writing client/server all the
> time.
>
> Yes, I agree. There could be old code around, that relies on
> my-client-protocol.h to pull in wayland-client.h which pulls in
> wayland-client-protocol.h. I do not see a good reason to break that,
> even though it is a minor issue.
>
> What my-client-protocol.h depends on are all found in
> wayland-client-core.h. The fact that wayland-client.h pulls in
> wayland-client-protocol.h is mostly a coincidence, but as said above, I
> think we want to make it opt-in to remove that implication. So the
> --include-core-headers option does have it's place.
>
> What this patch allows is to generate headers that will not pull in
> wayland-client-protocol.h. This is useful not only for using a newer
> wayland.xml but also for language bindings who should not be wrapping
> the generated entrypoints from wayland-client-protocol.h at all. Those
> would never want to pull in wayland-client-protocol.h, nor would they
> be using wayland-scanner either. I imagine this would be very useful
> for e.g. C++ bindings.
>
> Also, now we get a more clear separation between what is libwayland ABI
> and what is just generated helpers. This is explained in
> http://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-Code-Generation
> and now we can clearly say that everything in wayland-client-core.h is
> library ABI. Previously it wasn't so clear, because wayland-client.h
> includes wayland-client-protocol.h.
>
> Giulio also mentioned in IRC, that wayland-egl.h needs similar
> treatment, we cannot simply replace the #include wayland-client.h with
> wayland-client-core.h there. I'm not sure why wayland-egl.h includes
> anything really, but since it does, it makes sense to keep the same
> stability.
>
> However, for wayland-egl.h, what if the core version of it did not
> include anything? It doesn't need to, it just needs forward
> declarations of wl_surface and... that's it.

That makes sense.

>
> I would like to see this patch split into three:
> 1. the header stuff not including wayland-egl.h changes
> 2. wayland-scanner changes
> 3. wayland-egl.h changes
>
> 2 depends on 1, but 1 would be useful also on its own.
>
> Drop all includes from wayland-egl-core.h, and also rebased to master.

Fair enough, i'll get to it hopefully later today.

>
>> >> > Even if there is a reason to not include the core headers (I don't see
>> >> > it,
>> >> > however) it certainly would be the default to include them. Therefore
>> >> > the
>> >> > switch should at least be reversed so no switch means to include them.
>
> Bill, when you do not specify --include-core-headers, the generated
> headers will include the old wayland-client.h or wayland-server.h,
> rather than nothing. The generated headers will always include their
> needed dependencies. The question here is, will they also pull in
> wayland-client-protocol.h or not.
>
>> >> No, because it would then not be backward compatible, breaking the
>> >> sole reason the switch exists for.
>
> On Sat, 25 Apr 2015 17:39:05 +0300
> Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
>
>> @@ -1002,17 +1048,44 @@ emit_header(struct protocol *protocol, enum side side)
>>              "struct wl_resource;\n\n",
>>              protocol->uppercase_name, s,
>>              protocol->uppercase_name, s,
>> -            (side == SERVER) ? "wayland-server.h" : "wayland-client.h");
>> +            (side == SERVER) ? (protocol->include_core_headers ?
>> +                                "wayland-server-core.h" :
>> +                                "wayland-server.h") :
>> +                                (protocol->include_core_headers ?
>> +                                "wayland-client-core.h" :
>> +                                "wayland-client.h"));
>
> How about pulling this piece into a get_include_name(side, core)
> function? The nested ternaries are getting a bit out of hand.

Ok, i'll do that.



Thanks,
Giulio

>
>
> Thanks,
> pq