[PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

On 9 November 2015 at 10:54, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> On 9/11/2015 20:39 , Auke Booij wrote:
>>
>> On 6 November 2015 at 11:26, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>>>
>>> On Fri, 6 Nov 2015 09:47:03 +1000
>>> Peter Hutterer <peter.hutterer at who-t.net> wrote:
>>>
>>>> On Thu, Nov 05, 2015 at 04:58:09PM +0200, Pekka Paalanen wrote:
>>>>>
>>>>> On Mon, 19 Oct 2015 11:30:47 +1000
>>>>> Peter Hutterer <peter.hutterer at who-t.net> wrote:
>>>>>
>>>>>> On Fri, Oct 16, 2015 at 11:42:21AM +0300, Pekka Paalanen wrote:
>>>
>>>
>>>>>>> If the original objection to a DTD was because it required manually
>>>>>>> writing a lint phase in every project build system using the XML
>>>>>>> files,
>>>>>>> then having wayland-scanner invoke the check automatically solves
>>>>>>> that.
>>>>>>
>>>>>>
>>>>>> the question that remains though is: the dtd must be an external file
>>>>>> for
>>>>>> extensions to be validated. Which means we need to either pass the dtd
>>>>>> as
>>>>>> argument to the scanner (requires makefile changes everywhere), or we
>>>>>> hardcode the path into wayland-scanner (issues with running the
>>>>>> scanner from
>>>>>> within the source tree) or we add it as variable to pkgconfig
>>>>>> (requires
>>>>>> makefile changes again). any other solutions to fix this are welcome.
>>>>>> even if all we do is call out to xmllint we still run into that issue.
>>>>>
>>>>>
>>>>> Or, hopefully we can embed the DTD file into the scanner binary - is
>>>>> there no way to let libxml2 read it as a plain string?
>>>>
>>>>
>>>> yep, just tested it here, it's a 3-line change to the patch to load from
>>>> a
>>>> string.
>>>
>>>
>>> Cool.
>>>
>>>>> I used the following trick to embed some files in Wesgr:
>>>>>
>>>>> https://github.com/ppaalanen/wesgr/commit/02a840cb7db7eeb80071a353f66865d729738ae5
>>>>
>>>>
>>>> nice one. though if we're going for the embedded route anyway, we could
>>>> just
>>>> have it a const char in the scanner.c file. The dtd isn't really
>>>> expected to
>>>> change and when it does, the scanner needs to change too. Maybe we
>>>> should
>>>> just go for the easy route?
>>>
>>>
>>> I'll let you pick the way. Should be easy to change it later if needed
>>> anyway.
>>>
>>> Would it be bad if wayland-scanner embedded the DTD and in addition we
>>> installed the DTD as a file too? So scanner used only its internal
>>> copy. I'd probably try that if someone wants the DTD as a separate file
>>> for their own use. In that case the trick would be handy.
>>>
>>>
>>> Thanks,
>>> pq
>>
>>
>> That sounds like the scanner will become confusing to use ("wait, so
>> where DOES it get the DTD from, if not this file?"), unless you'd
>> actually read the source code. Or this should be very clearly
>> documented.
>
>
> commenting in the code - sure. but communicating this to the user isn't
> needed IMO, it's not really different to the normal scanner behaviour of
> failing when there's something it doesn't expect (though unfortunately right
> now the scanner also ignores things where IMO it should just fail).
>
> Cheers,
>   Peter

I'm not sure we're talking about the same issue here. What I'm
imagining is a user/developer trying to get into the internals of
wayland dev, encountering the protocols, and then seeing a DTD file
next to them. He* thinks "ah, surely that's used for verification, and
it's where I can start implementing my dirty hack". He modifies the
DTD , but the scanner doesn't behave differently. He removes the DTD,
but the scanner still passes the DTD check. Then he thinks "wait, so
where DOES it get the DTD from, if not this file?", at which point he
opens up the source code and learns that the DTD exists in two places.

Documenting this clearly will save every future person who wants to
get involved in protocol development 1 hour of figuring things out
when they don't work. That, or not installing the DTD in two places
(so either install as a file or embedded in the executable, but not
both).

 * or she