[PATCH wayland] scanner: Add --strict flag

[Jonas Ådahl]

On Thu, Oct 12, 2017 at 10:09:43AM +0300, Pekka Paalanen wrote:
> On Wed, 11 Oct 2017 17:31:33 +0800
> Jonas Ådahl <jadahl at gmail.com> wrote:
> 
> > Add a --strict flag for making wayland-scanner fail if the DTD
> > verification fails. This is useful for testing, so that a test case can
> > fail a scan when the protocol doesn't comply with the DTD.
> > 
> > Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> > ---
> > 
> > This could be used by wayland-protocols (and others) to verify that
> > protocols comply to the DTD. wayland-protocols already test-scans
> > protocols,  but protocols that doesn't pass the DTD just results in
> > warnings, the test still succeeds. I realize this could probably be
> > done by also using the dtd file directly, so this would only be for
> > convenience.
> > 
> > 
> > Jonas
> > 
> > 
> >  src/scanner.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/scanner.c b/src/scanner.c
> > index c345ed6..1308fc7 100644
> > --- a/src/scanner.c
> > +++ b/src/scanner.c
> > @@ -72,7 +72,9 @@ usage(int ret)
> >  			"                                 the scanner was built against.\n"
> >  			"    -c,  --include-core-only     include the core version of the headers,\n"
> >  			"                                 that is e.g. wayland-client-core.h instead\n"
> > -			"                                 of wayland-client.h.\n");
> > +			"                                 of wayland-client.h.\n"
> > +			"    -s,  --strict                exit immediately with an error if DTD\n"
> > +			"                                 verification fails.\n");
> 
> Hi Jonas,
> 
> I like the idea, but with a caveat, and I'd like to propose to take it
> a little further.
> 
> Let's not limit the --strict option to just DTD validation. I assume
> the scanner could do also other kinds of checks that we would not be
> able to turn into errors by default because of backwards compatibility.
> It would be useful for --strict to switch also those warnings into
> errors.
> 
> Backwards compatiblity brings me to the caveat. We should be very
> explicit in the scanner documentation that --strict does not preserve
> backwards compatibility. We need to allow ourselves to add more
> warnings in the future, DTD or otherwise, but we cannot add them if we
> imply --strict to remain backwards compatible. In essence, --strict
> would be like gcc's -Werror.

This sounds like a good idea.

> 
> That also means that projects using --strict do so on their own. I
> would guess that wayland-protocols could still use --strict since it's
> only testing, and testing can be disabled(?) via configure. Or vice
> versa, enabling --strict in wayland-protocols tests would require
> explicit enabling via configure or meson options. I suppose
> distributors would prefer the latter.

With the meson stuff, tests can be indeed be disabled. When being
enabled, I'd make the --strict thing only run on 'meson test' anyway
(however so far I have only managed to have meson always scan and build
the build testing part as part of the general build step).

Jonas

> 
> 
> Thanks,
> pq
> 
> >  	exit(ret);
> >  }
> >  
> > @@ -1801,6 +1803,7 @@ int main(int argc, char *argv[])
> >  	bool help = false;
> >  	bool core_headers = false;
> >  	bool version = false;
> > +	bool strict = false;
> >  	bool fail = false;
> >  	int opt;
> >  	enum {
> > @@ -1813,11 +1816,12 @@ int main(int argc, char *argv[])
> >  		{ "help",              no_argument, NULL, 'h' },
> >  		{ "version",           no_argument, NULL, 'v' },
> >  		{ "include-core-only", no_argument, NULL, 'c' },
> > +		{ "strict",            no_argument, NULL, 's' },
> >  		{ 0,                   0,           NULL, 0 }
> >  	};
> >  
> >  	while (1) {
> > -		opt = getopt_long(argc, argv, "hvc", options, NULL);
> > +		opt = getopt_long(argc, argv, "hvcs", options, NULL);
> >  
> >  		if (opt == -1)
> >  			break;
> > @@ -1832,6 +1836,9 @@ int main(int argc, char *argv[])
> >  		case 'c':
> >  			core_headers = true;
> >  			break;
> > +		case 's':
> > +			strict = true;
> > +			break;
> >  		default:
> >  			fail = true;
> >  			break;
> > @@ -1894,6 +1901,10 @@ int main(int argc, char *argv[])
> >  		"* WARNING: XML failed validation against built-in DTD *\n"
> >  		"*                                                     *\n"
> >  		"*******************************************************\n");
> > +		if (strict) {
> > +			fclose(input);
> > +			exit(EXIT_FAILURE);
> > +		}
> >  	}
> >  
> >  	/* create XML parser */
>