[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for us
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files. |
Date: |
Thu, 27 Mar 2014 19:07:48 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Benoît Canet <address@hidden> writes:
> 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 <address@hidden>
> ---
> 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
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index d78921f..a16aa47 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -180,6 +180,20 @@ An example command is:
> 'data': { 'arg1': 'str', '*arg2': 'str' },
> 'returns': 'str' }
>
> +=== Includes ===
> +
> +A schema file can include other sub schema files with the include
> +directive.
> +
> +An example of include directive is:
> +
> +{ 'include': 'path/to/sub_schema.json' }
> +
> +The include path is relative to the current schema file.
> +The include parsing method is recursive.
> +The expressions resulting from the parsing of the sub schema are injected
> +in place of the include directive like a C #include would do.
> +
>
> == Code generation ==
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 597042a..0b0c8e4 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -269,6 +269,8 @@ def check_exprs(schema):
> if expr.has_key('union'):
> check_union(expr, expr_elem['info'])
>
> +modules = []
> +
> def build_schema(path):
> with open(path, "r") as fp:
> try:
> @@ -281,13 +283,26 @@ def build_schema(path):
> def parse_schema(path):
> path = os.path.abspath(path)
>
> + if path in modules:
> + print "Module inclusion loop detected with module: %s" %\
> + get_filename(path)
> + sys.exit(1)
> +
> + modules.append(path)
> +
I'm afraid this will detect a loop when you include the same file
twice. Maybe that's not useful, but it's definitely not a loop.
Easy to avoid: make modules a stack, push on include, pop on EOF.
Bonus: you can easily print the actual cycle.
Suggest to drop "Module" from the error message.
I don't like the identifier modules, either.
> schema = build_schema(path)
>
> exprs = []
>
> 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'])
> + sub_exprs = parse_schema(sub_path)
> + exprs += sub_exprs
> + continue
> + elif expr.has_key('enum'):
> add_enum(expr['enum'], expr['data'])
> elif expr.has_key('union'):
> add_union(expr)
> diff --git a/tests/Makefile b/tests/Makefile
> index c4ed5c2..b5e4cf0 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -164,7 +164,7 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
> duplicate-key.json union-invalid-base.json flat-union-no-base.json \
> flat-union-invalid-discriminator.json \
> flat-union-invalid-branch-key.json flat-union-reverse-define.json \
> - flat-union-string-discriminator.json)
> + flat-union-string-discriminator.json include.json include_loop.json)
>
> GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h
> tests/test-qmp-commands.h
>
> diff --git a/tests/qapi-schema/include.err b/tests/qapi-schema/include.err
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/include.exit b/tests/qapi-schema/include.exit
> new file mode 100644
> index 0000000..573541a
> --- /dev/null
> +++ b/tests/qapi-schema/include.exit
> @@ -0,0 +1 @@
> +0
> diff --git a/tests/qapi-schema/include.json b/tests/qapi-schema/include.json
> new file mode 100644
> index 0000000..ffece21
> --- /dev/null
> +++ b/tests/qapi-schema/include.json
> @@ -0,0 +1,4 @@
> +{ 'enum': 'Status',
> + 'data': [ 'good', 'bad', 'ugly' ] }
> +{ 'include': './include/include.json' }
> +{ 'foo': '42' }
Doubt I'd bother with a sub-directory. Certainly covered by your
artistic license, though :)
[...]