The Thursday 27 Mar 2014 à 19:07:48 (+0100), Markus Armbruster wrote :
> Benoît Canet <benoit.ca...@irqsave.net> 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 <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
> >
> > 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 :)
Don't kid me you'll need it for the next QEMU logo :)
Best regards
Benoît
>
> [...]
