[PATCH wayland v2] Remove protocol/wayland.dtd

[Peter Hutterer]

On Thu, Nov 05, 2015 at 03:40:27PM +0000, Auke Booij wrote:
> 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.

unfortunately that doesn't work for external protocol files that rely on the
scanner, but not the wayland.xml protocol file

Cheers,
   Peter