From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46542) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTGFf-0001iI-GC for qemu-devel@nongnu.org; Thu, 27 Mar 2014 15:46:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTGFa-0005Jl-F5 for qemu-devel@nongnu.org; Thu, 27 Mar 2014 15:46:11 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:56604 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTGFa-0005JY-5O for qemu-devel@nongnu.org; Thu, 27 Mar 2014 15:46:06 -0400 Date: Thu, 27 Mar 2014 20:46:03 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140327194603.GB10497@irqsave.net> References: <1395934399-18769-1-git-send-email-benoit.canet@irqsave.net> <1395934399-18769-4-git-send-email-benoit.canet@irqsave.net> <87a9cbcy6j.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <87a9cbcy6j.fsf@blackfin.pond.sub.org> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , lcapitulino@redhat.com, qemu-devel@nongnu.org, anthony@codemonkey.ws, wenchaoqemu@gmail.com The Thursday 27 Mar 2014 =E0 19:07:48 (+0100), Markus Armbruster wrote : > Beno=EEt Canet writes: >=20 > > 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 fil= es. > > > > Signed-off-by: Benoit Canet > > --- > > 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' } > > =20 > > +=3D=3D=3D Includes =3D=3D=3D > > + > > +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 inj= ected > > +in place of the include directive like a C #include would do. > > + > > =20 > > =3D=3D Code generation =3D=3D > > =20 > > 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']) > > =20 > > +modules =3D [] > > + > > def build_schema(path): > > with open(path, "r") as fp: > > try: > > @@ -281,13 +283,26 @@ def build_schema(path): > > def parse_schema(path): > > path =3D os.path.abspath(path) > > =20 > > + if path in modules: > > + print "Module inclusion loop detected with module: %s" %\ > > + get_filename(path) > > + sys.exit(1) > > + > > + modules.append(path) > > + >=20 > 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. >=20 > Easy to avoid: make modules a stack, push on include, pop on EOF. > Bonus: you can easily print the actual cycle. >=20 > Suggest to drop "Module" from the error message. >=20 > I don't like the identifier modules, either. >=20 > > schema =3D build_schema(path) > > =20 > > exprs =3D [] > > =20 > > for expr_elem in schema.exprs: > > expr =3D expr_elem['expr'] > > - if expr.has_key('enum'): > > + if expr.has_key('include'): > > + prefix =3D os.path.split(path)[0] > > + sub_path =3D os.path.join(prefix, expr['include']) > > + sub_exprs =3D parse_schema(sub_path) > > + exprs +=3D 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 :=3D $(addprefix tests/qapi-s= chema/, \ > > duplicate-key.json union-invalid-base.json flat-union-no-bas= e.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_lo= op.json) > > =20 > > GENERATED_HEADERS +=3D tests/test-qapi-types.h tests/test-qapi-visit= .h tests/test-qmp-commands.h > > =20 > > diff --git a/tests/qapi-schema/include.err b/tests/qapi-schema/includ= e.err > > new file mode 100644 > > index 0000000..e69de29 > > diff --git a/tests/qapi-schema/include.exit b/tests/qapi-schema/inclu= de.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/inclu= de.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' } >=20 > 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=EEt >=20 > [...]