On 03/27/2014 09:33 AM, Benoît Canet wrote:
> The new directive in the form { 'include': 'path/to/file.json' } will trigger
> the
> parsing of path/to/file.json.
> The directive will be replaced by the result of the parsing.
>
> This will allow for easy modularisation of qapi JSON descriptions files.
>
> Signed-off-by: Benoit Canet <ben...@irqsave.net>
> ---
> docs/qapi-code-gen.txt | 14 ++++++++++++++
> scripts/qapi.py | 17 ++++++++++++++++-
> tests/Makefile | 2 +-
> tests/qapi-schema/include.exit | 1 +
> tests/qapi-schema/include.json | 4 ++++
> tests/qapi-schema/include.out | 8 ++++++++
> tests/qapi-schema/include/include.json | 7 +++++++
> tests/qapi-schema/include_loop.exit | 1 +
> tests/qapi-schema/include_loop.json | 1 +
> tests/qapi-schema/include_loop.out | 1 +
> 10 files changed, 54 insertions(+), 2 deletions(-)
> create mode 100644 tests/qapi-schema/include.err
> create mode 100644 tests/qapi-schema/include.exit
> create mode 100644 tests/qapi-schema/include.json
> create mode 100644 tests/qapi-schema/include.out
> create mode 100644 tests/qapi-schema/include/include.json
> create mode 100644 tests/qapi-schema/include_loop.err
> create mode 100644 tests/qapi-schema/include_loop.exit
> create mode 100644 tests/qapi-schema/include_loop.json
> create mode 100644 tests/qapi-schema/include_loop.out
>
> for expr_elem in schema.exprs:
> expr = expr_elem['expr']
> - if expr.has_key('enum'):
> + if expr.has_key('include'):
> + prefix = os.path.split(path)[0]
> + sub_path = os.path.join(prefix, expr['include'])
Should you validate that expr['include'] is a string, so we know the
user didn't do something stupid like { 'include': true } or { 'include':
{ 'hello':'world' } }. And if you add validation, you also need to add
tests.
> + elif expr.has_key('enum'):
> add_enum(expr['enum'], expr['data'])
For that matter (pre-existing, but you suffer from the same problem) -
there's no validation against unexpected keys, which means {
'enum':'foo', 'garbage':'blah' } and { 'include':'path',
'garbage':'blah' } both manage to silently ignore the 'garbage' key. If
you fix it for 'enum', do that first as a separate commit.
> +++ b/tests/qapi-schema/include.json
> @@ -0,0 +1,4 @@
> +{ 'enum': 'Status',
> + 'data': [ 'good', 'bad', 'ugly' ] }
> +{ 'include': './include/include.json' }
This tests successful inclusion relative to directory names.
> +++ b/tests/qapi-schema/include_loop.json
> @@ -0,0 +1 @@
> +{ 'include': 'include_loop.json' }
And this tests that self-inclusion is rejected.
But still missing tests for:
error message when the error occurs in an included file (ideally, the
error message should be the location within the included file, not the
outer file)
error occurring after a successful include (ideally, the lines injected
by the included file do NOT upset the line number tracking of the
original file)
multi-file loops being rejected (a includes b includes a)
each include name is relative to the current file even across nested
inclusion that changes directory (such as 'a' includes 'include/b'
includes '../c', should work when 'a' and 'c' are in the same directory,
and NOT try to pick up 'c' in the parent directory of 'a').
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
