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.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?
yep, just tested it here, it's a 3-line change to the patch to load from a string. > 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? Cheers, Peter _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
