[PATCH wayland v2] Remove protocol/wayland.dtd

[Auke Booij]

On 5 November 2015 at 14:58, Pekka Paalanen <ppaalanen at gmail.com> 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:
>> > On Fri, 16 Oct 2015 12:29:11 +1000
>> > Peter Hutterer <peter.hutterer at who-t.net> wrote:
>> >
>> > > On Fri, Oct 09, 2015 at 01:16:49PM +0200, Nils Chr. Brause wrote:
>> > > > Hi,
>> > > >
>> > > > Reviewed-by: Nils Christopher Brause <nilschrbrause at googlemail.com>
>> > > >
>> > > > I ran distcheck and it worked. :)
>> > >
>> > > a bit late, but I would like to register my disagreement with this patch :)
>> > >
>> > > Having the DTD is a much simpler and less bug-prone description of what the
>> > > protocol should look like. Having the scanner define the protocol means the
>> > > protocol is whatever the current version of the scanner supports, which is
>> > > not a good contract.
>> >
>> > Hi Peter,
>> >
>> > can't argue with that. It's just that the DTD was unused since
>> > 6292fe2af6a45decb7fd39090e74dd87bc4e22b2, Feb 2014.
>> >
>> > > I'd argue for reverting this and fixing any mismatch if there is one. And
>> > > using the DTD to validate before the scanner even runs.
>> >
>> > We talked in IRC about this, and came to the conclusion that maybe we
>> > could have wayland-scanner invoke a validity checker against a DTD or
>> > an XSD.
>>
>> I played around with that. As a quick basic solution we can hook validation
>> with libxml2 into the scanner and print a warning if the xml doesn't
>> validate. That's less than 50 LOC and won't break things since it doesn't
>> touch the actual parsing. and it won't break existing protocols that use
>> "creative" tags, all it will do is warn, not fail. See the diff below (after
>> reverting 06fb8bd37.
>>
>> it's an ugly solution though, but without scanner tests probably the best
>> thing we can do.
>
> Yeah, I agree. I think I'd be ok going forward with this. The patch
> looks like a good start if we can solve the DTD file loading.
>
> Libxml2 dependency should probably be build-time optional so far.
>
>> > 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?
>
> I used the following trick to embed some files in Wesgr:
> https://github.com/ppaalanen/wesgr/commit/02a840cb7db7eeb80071a353f66865d729738ae5
>
>
> Thanks,
> pq


If I understand correctly, you want to require the scanner to read the
DTD. Hence, since the scanner defines the protocol, the DTD is now
officially part of the protocol. So you might as well require that
wayland.dtd can be found in the same directory as the protocol XML
file itself.