On Thu, Nov 05, 2015 at 03:40:27PM +0000, Auke Booij wrote: > On 5 November 2015 at 14:58, Pekka Paalanen <ppaala...@gmail.com> wrote: > > On Mon, 19 Oct 2015 11:30:47 +1000 > > Peter Hutterer <peter.hutte...@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.hutte...@who-t.net> wrote: > >> > > >> > > On Fri, Oct 09, 2015 at 01:16:49PM +0200, Nils Chr. Brause wrote: > >> > > > Hi, > >> > > > > >> > > > Reviewed-by: Nils Christopher Brause <nilschrbra...@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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel