* [Qemu-devel] [PATCH V2 0/3] Create an include directive for QAPI JSON files
@ 2014-03-27 15:33 Benoît Canet
2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 1/3] test-qapi: Make test-qapi.py spit useful error messages Benoît Canet
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Benoît Canet @ 2014-03-27 15:33 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, wenchaoqemu, armbru, lcapitulino, anthony
The first patch make an error path in the test more explicit.
The second make the qapi generator script take their input as first argument.
It's done to be able to do cycle detection in the next patch.
The third create an include directive for QAPI JSON files.
The purpose of this series is to be able to add QMP to qemu-nbd in a not so
distant future.
Best regards
Benoît
in V2:
Split second patch in two patches [Eric]
Document in docs/qapi-code-gen.txt [Eric]
Benoît Canet (3):
test-qapi: Make test-qapi.py spit useful error messages.
qapi: Change the qapi scripts to take their input as first argument.
qapi: Create an include directive for use in the JSON description
files.
Makefile | 24 +++++-----
docs/qapi-code-gen.txt | 14 ++++++
scripts/qapi-commands.py | 8 +++-
scripts/qapi-types.py | 8 +++-
scripts/qapi-visit.py | 8 +++-
scripts/qapi.py | 46 ++++++++++++++++----
tests/Makefile | 16 +++----
tests/qapi-schema/duplicate-key.err | 2 +-
.../qapi-schema/flat-union-invalid-branch-key.err | 2 +-
.../flat-union-invalid-discriminator.err | 2 +-
tests/qapi-schema/flat-union-no-base.err | 2 +-
.../flat-union-string-discriminator.err | 2 +-
tests/qapi-schema/funny-char.err | 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 +
tests/qapi-schema/missing-colon.err | 2 +-
tests/qapi-schema/missing-comma-list.err | 2 +-
tests/qapi-schema/missing-comma-object.err | 2 +-
tests/qapi-schema/non-objects.err | 2 +-
tests/qapi-schema/quoted-structural-chars.err | 2 +-
tests/qapi-schema/test-qapi.py | 6 +--
tests/qapi-schema/trailing-comma-list.err | 2 +-
tests/qapi-schema/trailing-comma-object.err | 2 +-
tests/qapi-schema/unclosed-list.err | 2 +-
tests/qapi-schema/unclosed-object.err | 2 +-
tests/qapi-schema/unclosed-string.err | 2 +-
tests/qapi-schema/union-invalid-base.err | 2 +-
32 files changed, 132 insertions(+), 55 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
--
1.7.10.4
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCH V2 1/3] test-qapi: Make test-qapi.py spit useful error messages. 2014-03-27 15:33 [Qemu-devel] [PATCH V2 0/3] Create an include directive for QAPI JSON files Benoît Canet @ 2014-03-27 15:33 ` Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files Benoît Canet 2 siblings, 0 replies; 10+ messages in thread From: Benoît Canet @ 2014-03-27 15:33 UTC (permalink / raw) To: qemu-devel Cc: Benoît Canet, wenchaoqemu, armbru, lcapitulino, anthony, Benoit Canet In case of exception str(e) with e being the exception is more detailled. Signed-off-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/qapi-schema/test-qapi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index b3d1e1d..ac6da13 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -18,8 +18,8 @@ try: exprs = parse_schema(sys.stdin) except SystemExit: raise -except: - print >>sys.stderr, "Crashed:", sys.exc_info()[0] +except Exception, e: + print >>sys.stderr, "Crashed:", str(e) exit(1) pprint(exprs) -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument. 2014-03-27 15:33 [Qemu-devel] [PATCH V2 0/3] Create an include directive for QAPI JSON files Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 1/3] test-qapi: Make test-qapi.py spit useful error messages Benoît Canet @ 2014-03-27 15:33 ` Benoît Canet 2014-03-27 17:27 ` Eric Blake 2014-03-27 17:56 ` Markus Armbruster 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files Benoît Canet 2 siblings, 2 replies; 10+ messages in thread From: Benoît Canet @ 2014-03-27 15:33 UTC (permalink / raw) To: qemu-devel Cc: Benoît Canet, wenchaoqemu, armbru, lcapitulino, anthony, Benoit Canet This patch is here to pave the way for the JSON include directive which will need to do include loop detection. Signed-off-by: Benoit Canet <benoit@irqsave.net> --- Makefile | 24 ++++++++-------- scripts/qapi-commands.py | 8 ++++-- scripts/qapi-types.py | 8 ++++-- scripts/qapi-visit.py | 8 ++++-- scripts/qapi.py | 29 ++++++++++++++------ tests/Makefile | 14 +++++----- tests/qapi-schema/duplicate-key.err | 2 +- .../qapi-schema/flat-union-invalid-branch-key.err | 2 +- .../flat-union-invalid-discriminator.err | 2 +- tests/qapi-schema/flat-union-no-base.err | 2 +- .../flat-union-string-discriminator.err | 2 +- tests/qapi-schema/funny-char.err | 2 +- tests/qapi-schema/missing-colon.err | 2 +- tests/qapi-schema/missing-comma-list.err | 2 +- tests/qapi-schema/missing-comma-object.err | 2 +- tests/qapi-schema/non-objects.err | 2 +- tests/qapi-schema/quoted-structural-chars.err | 2 +- tests/qapi-schema/test-qapi.py | 2 +- tests/qapi-schema/trailing-comma-list.err | 2 +- tests/qapi-schema/trailing-comma-object.err | 2 +- tests/qapi-schema/unclosed-list.err | 2 +- tests/qapi-schema/unclosed-object.err | 2 +- tests/qapi-schema/unclosed-string.err | 2 +- tests/qapi-schema/union-invalid-base.err | 2 +- 24 files changed, 76 insertions(+), 51 deletions(-) diff --git a/Makefile b/Makefile index ec74039..9bec4ff 100644 --- a/Makefile +++ b/Makefile @@ -236,24 +236,24 @@ gen-out-type = $(subst .,-,$(suffix $@)) qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") qapi-types.c qapi-types.h :\ -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/qapi-schema.json $(gen-out-type) -o "." -b < $<, " GEN $@") qapi-visit.c qapi-visit.h :\ -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(SRC_PATH)/qapi-schema.json $(gen-out-type) -o "." -b < $<, " GEN $@") qmp-commands.h qmp-marshal.c :\ -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(SRC_PATH)/qapi-schema.json $(gen-out-type) -m -o "." < $<, " GEN $@") QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index 9734ab0..2995eab 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -369,13 +369,17 @@ def gen_command_def_prologue(prefix="", proxy=False): try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:m", + opts, args = getopt.gnu_getopt(sys.argv[2:], "chp:o:m", ["source", "header", "prefix=", "output-dir=", "type=", "middle"]) except getopt.GetoptError, err: print str(err) sys.exit(1) +if len(sys.argv) < 2: + print "The first argument must be the file name to parse" + sys.exit(1) + output_dir = "" prefix = "" dispatch_type = "sync" @@ -420,7 +424,7 @@ except os.error, e: if e.errno != errno.EEXIST: raise -exprs = parse_schema(sys.stdin) +exprs = parse_schema(sys.argv[1]) commands = filter(lambda expr: expr.has_key('command'), exprs) commands = filter(lambda expr: not expr.has_key('gen'), commands) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 10864ef..07df991 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -279,13 +279,17 @@ void qapi_free_%(type)s(%(c_type)s obj) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + opts, args = getopt.gnu_getopt(sys.argv[2:], "chbp:o:", ["source", "header", "builtins", "prefix=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) +if len(sys.argv) < 2: + print "The first argument must be the file name to parse" + sys.exit(1) + output_dir = "" prefix = "" c_file = 'qapi-types.c' @@ -378,7 +382,7 @@ fdecl.write(mcgen(''' ''', guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(sys.argv[1]) exprs = filter(lambda expr: not expr.has_key('gen'), exprs) fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 45ce3a9..cd53c97 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -397,13 +397,17 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e name=name) try: - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", + opts, args = getopt.gnu_getopt(sys.argv[2:], "chbp:o:", ["source", "header", "builtins", "prefix=", "output-dir="]) except getopt.GetoptError, err: print str(err) sys.exit(1) +if len(sys.argv) < 2: + print "The first argument must be the file name to parse" + sys.exit(1) + output_dir = "" prefix = "" c_file = 'qapi-visit.c' @@ -494,7 +498,7 @@ fdecl.write(mcgen(''' ''', prefix=prefix, guard=guardname(h_file))) -exprs = parse_schema(sys.stdin) +exprs = parse_schema(sys.argv[1]) # to avoid header dependency hell, we always generate declarations # for built-in types in our header files and simply guard them diff --git a/scripts/qapi.py b/scripts/qapi.py index b474c39..597042a 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -12,6 +12,7 @@ # See the COPYING file in the top-level directory. from ordereddict import OrderedDict +import os import sys builtin_types = [ @@ -35,6 +36,9 @@ builtin_type_qtypes = { 'uint64': 'QTYPE_QINT', } +def get_filename(path): + return os.path.split(path)[1] + class QAPISchemaError(Exception): def __init__(self, schema, msg): self.fp = schema.fp @@ -48,7 +52,8 @@ class QAPISchemaError(Exception): self.col += 1 def __str__(self): - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg) + return "%s:%s:%s: %s" % (get_filename(self.fp.name), + self.line, self.col, self.msg) class QAPIExprError(Exception): def __init__(self, expr_info, msg): @@ -57,7 +62,8 @@ class QAPIExprError(Exception): self.msg = msg def __str__(self): - return "%s:%s: %s" % (self.fp.name, self.line, self.msg) + return "%s:%s: %s" % (get_filename(self.fp.name), + self.line, self.msg) class QAPISchema: @@ -263,12 +269,19 @@ def check_exprs(schema): if expr.has_key('union'): check_union(expr, expr_elem['info']) -def parse_schema(fp): - try: - schema = QAPISchema(fp) - except QAPISchemaError, e: - print >>sys.stderr, e - exit(1) +def build_schema(path): + with open(path, "r") as fp: + try: + schema = QAPISchema(fp) + except QAPISchemaError, e: + print >>sys.stderr, e + exit(1) + return schema + +def parse_schema(path): + path = os.path.abspath(path) + + schema = build_schema(path) exprs = [] diff --git a/tests/Makefile b/tests/Makefile index 2d021fb..c4ed5c2 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -215,14 +215,14 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ libqemuutil.a tests/test-qapi-types.c tests/test-qapi-types.h :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-types.py + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") tests/test-qapi-visit.c tests/test-qapi-visit.h :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-visit.py + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") +$(SRC_PATH)/scripts/qapi-commands.py + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a @@ -362,7 +362,7 @@ check-tests/test-qapi.py: tests/test-qapi.py .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json - $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py <$^ >$*.test.out 2>$*.test.err; echo $$? >$*.test.exit, " TEST $*.out") + $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py $^ >$*.test.out 2>$*.test.err; echo $$? >$*.test.exit, " TEST $*.out") @diff -q $(SRC_PATH)/$*.out $*.test.out @diff -q $(SRC_PATH)/$*.err $*.test.err @diff -q $(SRC_PATH)/$*.exit $*.test.exit diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err index 0801c6a..21303ef 100644 --- a/tests/qapi-schema/duplicate-key.err +++ b/tests/qapi-schema/duplicate-key.err @@ -1 +1 @@ -<stdin>:2:10: Duplicate key "key" +duplicate-key.json:2:10: Duplicate key "key" diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err index 1125caf..34b4584 100644 --- a/tests/qapi-schema/flat-union-invalid-branch-key.err +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err @@ -1 +1 @@ -<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' +flat-union-invalid-branch-key.json:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err index cad9dbf..b6ce3b7 100644 --- a/tests/qapi-schema/flat-union-invalid-discriminator.err +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err @@ -1 +1 @@ -<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' +flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err index e2d7443..7929455 100644 --- a/tests/qapi-schema/flat-union-no-base.err +++ b/tests/qapi-schema/flat-union-no-base.err @@ -1 +1 @@ -<stdin>:7: Flat union 'TestUnion' must have a base field +flat-union-no-base.json:7: Flat union 'TestUnion' must have a base field diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err index 8748270..fc1e2d5 100644 --- a/tests/qapi-schema/flat-union-string-discriminator.err +++ b/tests/qapi-schema/flat-union-string-discriminator.err @@ -1 +1 @@ -<stdin>:13: Discriminator 'kind' must be of enumeration type +flat-union-string-discriminator.json:13: Discriminator 'kind' must be of enumeration type diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err index d3dd293..ee65869 100644 --- a/tests/qapi-schema/funny-char.err +++ b/tests/qapi-schema/funny-char.err @@ -1 +1 @@ -<stdin>:2:36: Stray ";" +funny-char.json:2:36: Stray ";" diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err index 9f2a355..676cce5 100644 --- a/tests/qapi-schema/missing-colon.err +++ b/tests/qapi-schema/missing-colon.err @@ -1 +1 @@ -<stdin>:1:10: Expected ":" +missing-colon.json:1:10: Expected ":" diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err index 4fe0700..d0ed8c3 100644 --- a/tests/qapi-schema/missing-comma-list.err +++ b/tests/qapi-schema/missing-comma-list.err @@ -1 +1 @@ -<stdin>:2:20: Expected "," or "]" +missing-comma-list.json:2:20: Expected "," or "]" diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err index b0121b5..ad9b457 100644 --- a/tests/qapi-schema/missing-comma-object.err +++ b/tests/qapi-schema/missing-comma-object.err @@ -1 +1 @@ -<stdin>:2:3: Expected "," or "}" +missing-comma-object.json:2:3: Expected "," or "}" diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err index a6c2dc2..e958cf0 100644 --- a/tests/qapi-schema/non-objects.err +++ b/tests/qapi-schema/non-objects.err @@ -1 +1 @@ -<stdin>:1:1: Expected "{" +non-objects.json:1:1: Expected "{" diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err index a6c2dc2..77732d0 100644 --- a/tests/qapi-schema/quoted-structural-chars.err +++ b/tests/qapi-schema/quoted-structural-chars.err @@ -1 +1 @@ -<stdin>:1:1: Expected "{" +quoted-structural-chars.json:1:1: Expected "{" diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index ac6da13..4f217d2 100644 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -15,7 +15,7 @@ from pprint import pprint import sys try: - exprs = parse_schema(sys.stdin) + exprs = parse_schema(sys.argv[1]) except SystemExit: raise except Exception, e: diff --git a/tests/qapi-schema/trailing-comma-list.err b/tests/qapi-schema/trailing-comma-list.err index ff839a3..13b79f9 100644 --- a/tests/qapi-schema/trailing-comma-list.err +++ b/tests/qapi-schema/trailing-comma-list.err @@ -1 +1 @@ -<stdin>:2:36: Expected "{", "[" or string +trailing-comma-list.json:2:36: Expected "{", "[" or string diff --git a/tests/qapi-schema/trailing-comma-object.err b/tests/qapi-schema/trailing-comma-object.err index f540962..d1d57f0 100644 --- a/tests/qapi-schema/trailing-comma-object.err +++ b/tests/qapi-schema/trailing-comma-object.err @@ -1 +1 @@ -<stdin>:2:38: Expected string +trailing-comma-object.json:2:38: Expected string diff --git a/tests/qapi-schema/unclosed-list.err b/tests/qapi-schema/unclosed-list.err index 0e837a7..1a699a2 100644 --- a/tests/qapi-schema/unclosed-list.err +++ b/tests/qapi-schema/unclosed-list.err @@ -1 +1 @@ -<stdin>:1:20: Expected "," or "]" +unclosed-list.json:1:20: Expected "," or "]" diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err index e6dc950..3ddb126 100644 --- a/tests/qapi-schema/unclosed-object.err +++ b/tests/qapi-schema/unclosed-object.err @@ -1 +1 @@ -<stdin>:1:21: Expected "," or "}" +unclosed-object.json:1:21: Expected "," or "}" diff --git a/tests/qapi-schema/unclosed-string.err b/tests/qapi-schema/unclosed-string.err index 948d883..cdd3dca 100644 --- a/tests/qapi-schema/unclosed-string.err +++ b/tests/qapi-schema/unclosed-string.err @@ -1 +1 @@ -<stdin>:1:11: Missing terminating "'" +unclosed-string.json:1:11: Missing terminating "'" diff --git a/tests/qapi-schema/union-invalid-base.err b/tests/qapi-schema/union-invalid-base.err index dd8e3d1..5e38b12 100644 --- a/tests/qapi-schema/union-invalid-base.err +++ b/tests/qapi-schema/union-invalid-base.err @@ -1 +1 @@ -<stdin>:7: Base 'TestBaseWrong' is not a valid type +union-invalid-base.json:7: Base 'TestBaseWrong' is not a valid type -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument. 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument Benoît Canet @ 2014-03-27 17:27 ` Eric Blake 2014-03-28 8:27 ` Markus Armbruster 2014-03-27 17:56 ` Markus Armbruster 1 sibling, 1 reply; 10+ messages in thread From: Eric Blake @ 2014-03-27 17:27 UTC (permalink / raw) To: Benoît Canet, qemu-devel Cc: anthony, Benoit Canet, armbru, wenchaoqemu, lcapitulino [-- Attachment #1: Type: text/plain, Size: 2331 bytes --] On 03/27/2014 09:33 AM, Benoît Canet wrote: > This patch is here to pave the way for the JSON include directive which > will need to do include loop detection. > Would also be nice to mention that it improves the error message quality. While 3/3 is definitely 2.1 material, 1/3 and 2/3 could arguably be committed in 2.0 as bug fixes due to the improved errors (but it's a stretch - personally I'm fine with saving the whole series for 2.1, as the error messages are in the build chain only for developers to see, and not user-visible in the end product) > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > 24 files changed, 76 insertions(+), 51 deletions(-) > > diff --git a/Makefile b/Makefile > index ec74039..9bec4ff 100644 > --- a/Makefile > +++ b/Makefile > @@ -236,24 +236,24 @@ gen-out-type = $(subst .,-,$(suffix $@)) > qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py > > qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ > -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") While I don't mind GNU getopt's ability to reorganize options to occur after non-options, I'm not sure it's the smartest thing to do here. Either the input file should be a new option (as in '-i input -o output') or you should consider ordering the command line to put the file name after all options ('-o output input' rather than 'input -o output'). For that matter, I feel that named options are always more flexible than positional arguments. That said, it looks like this script _already_ has a positional argument (gen-out-type) occurring before options, so you aren't making it any worse. Therefore, if you don't respin it to take the input file name as an option, I can live with: Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument. 2014-03-27 17:27 ` Eric Blake @ 2014-03-28 8:27 ` Markus Armbruster 0 siblings, 0 replies; 10+ messages in thread From: Markus Armbruster @ 2014-03-28 8:27 UTC (permalink / raw) To: Eric Blake Cc: Benoît Canet, wenchaoqemu, qemu-devel, lcapitulino, anthony, Benoit Canet Eric Blake <eblake@redhat.com> writes: > On 03/27/2014 09:33 AM, Benoît Canet wrote: >> This patch is here to pave the way for the JSON include directive which >> will need to do include loop detection. >> > > Would also be nice to mention that it improves the error message > quality. While 3/3 is definitely 2.1 material, 1/3 and 2/3 could > arguably be committed in 2.0 as bug fixes due to the improved errors > (but it's a stretch - personally I'm fine with saving the whole series > for 2.1, as the error messages are in the build chain only for > developers to see, and not user-visible in the end product) > >> Signed-off-by: Benoit Canet <benoit@irqsave.net> >> --- > >> 24 files changed, 76 insertions(+), 51 deletions(-) > >> >> diff --git a/Makefile b/Makefile >> index ec74039..9bec4ff 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -236,24 +236,24 @@ gen-out-type = $(subst .,-,$(suffix $@)) >> qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py >> >> qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ >> -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) >> - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") >> +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) >> + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > > While I don't mind GNU getopt's ability to reorganize options to occur > after non-options, I'm not sure it's the smartest thing to do here. > Either the input file should be a new option (as in '-i input -o > output') or you should consider ordering the command line to put the > file name after all options ('-o output input' rather than 'input -o > output'). For that matter, I feel that named options are always more > flexible than positional arguments. Using positional arguments for input files of compiler-like tools is well-established practice. I share your preference for putting positional arguments after the options. > That said, it looks like this script _already_ has a positional argument > (gen-out-type) occurring before options, so you aren't making it any > worse. $(gen-out-type) expands into -EXT, where EXT is $@'s extension. Took me some staring to figure that out :) > Therefore, if you don't respin it to take the input file name as > an option, I can live with: > > Reviewed-by: Eric Blake <eblake@redhat.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument. 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument Benoît Canet 2014-03-27 17:27 ` Eric Blake @ 2014-03-27 17:56 ` Markus Armbruster 1 sibling, 0 replies; 10+ messages in thread From: Markus Armbruster @ 2014-03-27 17:56 UTC (permalink / raw) To: Benoît Canet Cc: lcapitulino, Benoit Canet, qemu-devel, anthony, wenchaoqemu Benoît Canet <benoit.canet@irqsave.net> writes: > This patch is here to pave the way for the JSON include directive which > will need to do include loop detection. > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > Makefile | 24 ++++++++-------- > scripts/qapi-commands.py | 8 ++++-- > scripts/qapi-types.py | 8 ++++-- > scripts/qapi-visit.py | 8 ++++-- > scripts/qapi.py | 29 ++++++++++++++------ > tests/Makefile | 14 +++++----- > tests/qapi-schema/duplicate-key.err | 2 +- > .../qapi-schema/flat-union-invalid-branch-key.err | 2 +- > .../flat-union-invalid-discriminator.err | 2 +- > tests/qapi-schema/flat-union-no-base.err | 2 +- > .../flat-union-string-discriminator.err | 2 +- > tests/qapi-schema/funny-char.err | 2 +- > tests/qapi-schema/missing-colon.err | 2 +- > tests/qapi-schema/missing-comma-list.err | 2 +- > tests/qapi-schema/missing-comma-object.err | 2 +- > tests/qapi-schema/non-objects.err | 2 +- > tests/qapi-schema/quoted-structural-chars.err | 2 +- > tests/qapi-schema/test-qapi.py | 2 +- > tests/qapi-schema/trailing-comma-list.err | 2 +- > tests/qapi-schema/trailing-comma-object.err | 2 +- > tests/qapi-schema/unclosed-list.err | 2 +- > tests/qapi-schema/unclosed-object.err | 2 +- > tests/qapi-schema/unclosed-string.err | 2 +- > tests/qapi-schema/union-invalid-base.err | 2 +- > 24 files changed, 76 insertions(+), 51 deletions(-) > > diff --git a/Makefile b/Makefile > index ec74039..9bec4ff 100644 > --- a/Makefile > +++ b/Makefile > @@ -236,24 +236,24 @@ gen-out-type = $(subst .,-,$(suffix $@)) > qapi-py = $(SRC_PATH)/scripts/qapi.py $(SRC_PATH)/scripts/ordereddict.py > > qga/qapi-generated/qga-qapi-types.c qga/qapi-generated/qga-qapi-types.h :\ > -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") You remove the prerequisite $(SRC_PATH)/qga/qapi-schema.json. Breaks remake after schema change. Why do you redirect input from $(SRC_PATH)/scripts/qapi-types.py? I suspect what we really want is the exact old rule with '< $<' replaced by just '$<'. More of the same below. > qga/qapi-generated/qga-qapi-visit.c qga/qapi-generated/qga-qapi-visit.h :\ > -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > qga/qapi-generated/qga-qmp-commands.h qga/qapi-generated/qga-qmp-marshal.c :\ > -$(SRC_PATH)/qga/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(SRC_PATH)/qga/qapi-schema.json $(gen-out-type) -o qga/qapi-generated -p "qga-" < $<, " GEN $@") > > qapi-types.c qapi-types.h :\ > -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o "." -b < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-types.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/qapi-schema.json $(gen-out-type) -o "." -b < $<, " GEN $@") > qapi-visit.c qapi-visit.h :\ > -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o "." -b < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-visit.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(SRC_PATH)/qapi-schema.json $(gen-out-type) -o "." -b < $<, " GEN $@") > qmp-commands.h qmp-marshal.c :\ > -$(SRC_PATH)/qapi-schema.json $(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -m -o "." < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-commands.py $(qapi-py) > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(SRC_PATH)/qapi-schema.json $(gen-out-type) -m -o "." < $<, " GEN $@") > > QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h) > $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index 9734ab0..2995eab 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -369,13 +369,17 @@ def gen_command_def_prologue(prefix="", proxy=False): > > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chp:o:m", > + opts, args = getopt.gnu_getopt(sys.argv[2:], "chp:o:m", This looks wrong. You should continue to pass all arguments to gnu_getopt(). You get back two values, the options (assigned to opts here), and the non-option arguments (assigned to args). > ["source", "header", "prefix=", > "output-dir=", "type=", "middle"]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > > +if len(sys.argv) < 2: > + print "The first argument must be the file name to parse" > + sys.exit(1) > + You should test len(args). Are arguments beyond the first used? If no, better reject length other than 1. > output_dir = "" > prefix = "" > dispatch_type = "sync" > @@ -420,7 +424,7 @@ except os.error, e: > if e.errno != errno.EEXIST: > raise > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(sys.argv[1]) You should use args[0]. Same for the other utilities. > commands = filter(lambda expr: expr.has_key('command'), exprs) > commands = filter(lambda expr: not expr.has_key('gen'), commands) > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index 10864ef..07df991 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -279,13 +279,17 @@ void qapi_free_%(type)s(%(c_type)s obj) > > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", > + opts, args = getopt.gnu_getopt(sys.argv[2:], "chbp:o:", > ["source", "header", "builtins", > "prefix=", "output-dir="]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > > +if len(sys.argv) < 2: > + print "The first argument must be the file name to parse" > + sys.exit(1) > + > output_dir = "" > prefix = "" > c_file = 'qapi-types.c' > @@ -378,7 +382,7 @@ fdecl.write(mcgen(''' > ''', > guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(sys.argv[1]) > exprs = filter(lambda expr: not expr.has_key('gen'), exprs) > > fdecl.write(guardstart("QAPI_TYPES_BUILTIN_STRUCT_DECL")) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 45ce3a9..cd53c97 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -397,13 +397,17 @@ void visit_type_%(name)s(Visitor *m, %(name)s * obj, const char *name, Error **e > name=name) > > try: > - opts, args = getopt.gnu_getopt(sys.argv[1:], "chbp:o:", > + opts, args = getopt.gnu_getopt(sys.argv[2:], "chbp:o:", > ["source", "header", "builtins", "prefix=", > "output-dir="]) > except getopt.GetoptError, err: > print str(err) > sys.exit(1) > > +if len(sys.argv) < 2: > + print "The first argument must be the file name to parse" > + sys.exit(1) > + > output_dir = "" > prefix = "" > c_file = 'qapi-visit.c' > @@ -494,7 +498,7 @@ fdecl.write(mcgen(''' > ''', > prefix=prefix, guard=guardname(h_file))) > > -exprs = parse_schema(sys.stdin) > +exprs = parse_schema(sys.argv[1]) > > # to avoid header dependency hell, we always generate declarations > # for built-in types in our header files and simply guard them > diff --git a/scripts/qapi.py b/scripts/qapi.py > index b474c39..597042a 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -12,6 +12,7 @@ > # See the COPYING file in the top-level directory. > > from ordereddict import OrderedDict > +import os > import sys > > builtin_types = [ > @@ -35,6 +36,9 @@ builtin_type_qtypes = { > 'uint64': 'QTYPE_QINT', > } > > +def get_filename(path): > + return os.path.split(path)[1] > + > class QAPISchemaError(Exception): > def __init__(self, schema, msg): > self.fp = schema.fp > @@ -48,7 +52,8 @@ class QAPISchemaError(Exception): > self.col += 1 > > def __str__(self): > - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg) > + return "%s:%s:%s: %s" % (get_filename(self.fp.name), > + self.line, self.col, self.msg) > This makes the error message refer to the input file by its non-directory part. As mentioned in review of Lluís's patch, plase don't do that. I understand the directory part gets in the way of tests because $(SRC_PATH), but the way to cope with that is having the tests normalize output with a suitable filter, not making the error messages less informational and potentially ambiguous. > class QAPIExprError(Exception): > def __init__(self, expr_info, msg): > @@ -57,7 +62,8 @@ class QAPIExprError(Exception): > self.msg = msg > > def __str__(self): > - return "%s:%s: %s" % (self.fp.name, self.line, self.msg) > + return "%s:%s: %s" % (get_filename(self.fp.name), > + self.line, self.msg) > Likewise. > class QAPISchema: > > @@ -263,12 +269,19 @@ def check_exprs(schema): > if expr.has_key('union'): > check_union(expr, expr_elem['info']) > > -def parse_schema(fp): > - try: > - schema = QAPISchema(fp) > - except QAPISchemaError, e: > - print >>sys.stderr, e > - exit(1) > +def build_schema(path): This isn't a path, it's a file name. Please call it fname, filename, or something like that. > + with open(path, "r") as fp: > + try: > + schema = QAPISchema(fp) > + except QAPISchemaError, e: > + print >>sys.stderr, e > + exit(1) > + return schema > + > +def parse_schema(path): > + path = os.path.abspath(path) This turns path into an normalized absolute file name. The result ends up in self.fp.name. Therefore, your change makes error messages refer to input files by their absolute filename, currently less the directory part. Not nice. Note that abspath() can change the non-directory part! Symbolic links are so much fun... I figure you want normalization to detect include cycles. If that's the case, consider doing it in the patch that needs it. Independently, please use normalized file names *only* for include cycle detection, *not* for error messages. > + > + schema = build_schema(path) > > exprs = [] > > diff --git a/tests/Makefile b/tests/Makefile > index 2d021fb..c4ed5c2 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -215,14 +215,14 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \ > libqemuutil.a > > tests/test-qapi-types.c tests/test-qapi-types.h :\ > -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-types.py > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") > tests/test-qapi-visit.c tests/test-qapi-visit.h :\ > -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-visit.py > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") > tests/test-qmp-commands.h tests/test-qmp-marshal.c :\ > -$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py > - $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") > +$(SRC_PATH)/scripts/qapi-commands.py > + $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(gen-out-type) -o tests -p "test-" < $<, " GEN $@") > > tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a > @@ -362,7 +362,7 @@ check-tests/test-qapi.py: tests/test-qapi.py > > .PHONY: $(patsubst %, check-%, $(check-qapi-schema-y)) > $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json > - $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py <$^ >$*.test.out 2>$*.test.err; echo $$? >$*.test.exit, " TEST $*.out") > + $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py $^ >$*.test.out 2>$*.test.err; echo $$? >$*.test.exit, " TEST $*.out") > @diff -q $(SRC_PATH)/$*.out $*.test.out > @diff -q $(SRC_PATH)/$*.err $*.test.err > @diff -q $(SRC_PATH)/$*.exit $*.test.exit > diff --git a/tests/qapi-schema/duplicate-key.err b/tests/qapi-schema/duplicate-key.err > index 0801c6a..21303ef 100644 > --- a/tests/qapi-schema/duplicate-key.err > +++ b/tests/qapi-schema/duplicate-key.err > @@ -1 +1 @@ > -<stdin>:2:10: Duplicate key "key" > +duplicate-key.json:2:10: Duplicate key "key" > diff --git a/tests/qapi-schema/flat-union-invalid-branch-key.err b/tests/qapi-schema/flat-union-invalid-branch-key.err > index 1125caf..34b4584 100644 > --- a/tests/qapi-schema/flat-union-invalid-branch-key.err > +++ b/tests/qapi-schema/flat-union-invalid-branch-key.err > @@ -1 +1 @@ > -<stdin>:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' > +flat-union-invalid-branch-key.json:13: Discriminator value 'value_wrong' is not found in enum 'TestEnum' > diff --git a/tests/qapi-schema/flat-union-invalid-discriminator.err b/tests/qapi-schema/flat-union-invalid-discriminator.err > index cad9dbf..b6ce3b7 100644 > --- a/tests/qapi-schema/flat-union-invalid-discriminator.err > +++ b/tests/qapi-schema/flat-union-invalid-discriminator.err > @@ -1 +1 @@ > -<stdin>:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' > +flat-union-invalid-discriminator.json:13: Discriminator 'enum_wrong' is not a member of base type 'TestBase' > diff --git a/tests/qapi-schema/flat-union-no-base.err b/tests/qapi-schema/flat-union-no-base.err > index e2d7443..7929455 100644 > --- a/tests/qapi-schema/flat-union-no-base.err > +++ b/tests/qapi-schema/flat-union-no-base.err > @@ -1 +1 @@ > -<stdin>:7: Flat union 'TestUnion' must have a base field > +flat-union-no-base.json:7: Flat union 'TestUnion' must have a base field > diff --git a/tests/qapi-schema/flat-union-string-discriminator.err b/tests/qapi-schema/flat-union-string-discriminator.err > index 8748270..fc1e2d5 100644 > --- a/tests/qapi-schema/flat-union-string-discriminator.err > +++ b/tests/qapi-schema/flat-union-string-discriminator.err > @@ -1 +1 @@ > -<stdin>:13: Discriminator 'kind' must be of enumeration type > +flat-union-string-discriminator.json:13: Discriminator 'kind' must be of enumeration type > diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err > index d3dd293..ee65869 100644 > --- a/tests/qapi-schema/funny-char.err > +++ b/tests/qapi-schema/funny-char.err > @@ -1 +1 @@ > -<stdin>:2:36: Stray ";" > +funny-char.json:2:36: Stray ";" > diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err > index 9f2a355..676cce5 100644 > --- a/tests/qapi-schema/missing-colon.err > +++ b/tests/qapi-schema/missing-colon.err > @@ -1 +1 @@ > -<stdin>:1:10: Expected ":" > +missing-colon.json:1:10: Expected ":" > diff --git a/tests/qapi-schema/missing-comma-list.err b/tests/qapi-schema/missing-comma-list.err > index 4fe0700..d0ed8c3 100644 > --- a/tests/qapi-schema/missing-comma-list.err > +++ b/tests/qapi-schema/missing-comma-list.err > @@ -1 +1 @@ > -<stdin>:2:20: Expected "," or "]" > +missing-comma-list.json:2:20: Expected "," or "]" > diff --git a/tests/qapi-schema/missing-comma-object.err b/tests/qapi-schema/missing-comma-object.err > index b0121b5..ad9b457 100644 > --- a/tests/qapi-schema/missing-comma-object.err > +++ b/tests/qapi-schema/missing-comma-object.err > @@ -1 +1 @@ > -<stdin>:2:3: Expected "," or "}" > +missing-comma-object.json:2:3: Expected "," or "}" > diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err > index a6c2dc2..e958cf0 100644 > --- a/tests/qapi-schema/non-objects.err > +++ b/tests/qapi-schema/non-objects.err > @@ -1 +1 @@ > -<stdin>:1:1: Expected "{" > +non-objects.json:1:1: Expected "{" > diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err > index a6c2dc2..77732d0 100644 > --- a/tests/qapi-schema/quoted-structural-chars.err > +++ b/tests/qapi-schema/quoted-structural-chars.err > @@ -1 +1 @@ > -<stdin>:1:1: Expected "{" > +quoted-structural-chars.json:1:1: Expected "{" > diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py > index ac6da13..4f217d2 100644 > --- a/tests/qapi-schema/test-qapi.py > +++ b/tests/qapi-schema/test-qapi.py > @@ -15,7 +15,7 @@ from pprint import pprint > import sys > > try: > - exprs = parse_schema(sys.stdin) > + exprs = parse_schema(sys.argv[1]) > except SystemExit: > raise > except Exception, e: What if argv[1] is None? Suggest to error out unless len(sys.argv) == 1. [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files. 2014-03-27 15:33 [Qemu-devel] [PATCH V2 0/3] Create an include directive for QAPI JSON files Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 1/3] test-qapi: Make test-qapi.py spit useful error messages Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument Benoît Canet @ 2014-03-27 15:33 ` Benoît Canet 2014-03-27 17:39 ` Eric Blake 2014-03-27 18:07 ` Markus Armbruster 2 siblings, 2 replies; 10+ messages in thread From: Benoît Canet @ 2014-03-27 15:33 UTC (permalink / raw) To: qemu-devel Cc: Benoît Canet, wenchaoqemu, armbru, lcapitulino, anthony, Benoit Canet 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 <benoit@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) + 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' } diff --git a/tests/qapi-schema/include.out b/tests/qapi-schema/include.out new file mode 100644 index 0000000..89e43e8 --- /dev/null +++ b/tests/qapi-schema/include.out @@ -0,0 +1,8 @@ +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])]), + OrderedDict([('bar', '33')]), + OrderedDict([('enum', 'Fruits'), ('data', ['orange', 'apple', 'gooseberry'])]), + OrderedDict([('baz', '54')]), + OrderedDict([('foo', '42')])] +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}, + {'enum_name': 'Fruits', 'enum_values': ['orange', 'apple', 'gooseberry']}] +[] diff --git a/tests/qapi-schema/include/include.json b/tests/qapi-schema/include/include.json new file mode 100644 index 0000000..f445eee --- /dev/null +++ b/tests/qapi-schema/include/include.json @@ -0,0 +1,7 @@ + +{ 'bar': '33' } + +{ 'enum': 'Fruits', + 'data': [ 'orange', 'apple', 'gooseberry' ] } + +{ 'baz': '54' } diff --git a/tests/qapi-schema/include_loop.err b/tests/qapi-schema/include_loop.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/include_loop.exit b/tests/qapi-schema/include_loop.exit new file mode 100644 index 0000000..d00491f --- /dev/null +++ b/tests/qapi-schema/include_loop.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/include_loop.json b/tests/qapi-schema/include_loop.json new file mode 100644 index 0000000..cb8ff03 --- /dev/null +++ b/tests/qapi-schema/include_loop.json @@ -0,0 +1 @@ +{ 'include': 'include_loop.json' } diff --git a/tests/qapi-schema/include_loop.out b/tests/qapi-schema/include_loop.out new file mode 100644 index 0000000..35da4dd --- /dev/null +++ b/tests/qapi-schema/include_loop.out @@ -0,0 +1 @@ +Module inclusion loop detected with module: include_loop.json -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files. 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files Benoît Canet @ 2014-03-27 17:39 ` Eric Blake 2014-03-27 18:07 ` Markus Armbruster 1 sibling, 0 replies; 10+ messages in thread From: Eric Blake @ 2014-03-27 17:39 UTC (permalink / raw) To: Benoît Canet, qemu-devel Cc: anthony, Benoit Canet, armbru, wenchaoqemu, lcapitulino [-- Attachment #1: Type: text/plain, Size: 3608 bytes --] 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 <benoit@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 [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files. 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files Benoît Canet 2014-03-27 17:39 ` Eric Blake @ 2014-03-27 18:07 ` Markus Armbruster 2014-03-27 19:46 ` Benoît Canet 1 sibling, 1 reply; 10+ messages in thread From: Markus Armbruster @ 2014-03-27 18:07 UTC (permalink / raw) To: Benoît Canet Cc: lcapitulino, Benoit Canet, qemu-devel, anthony, wenchaoqemu Benoît Canet <benoit.canet@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 <benoit@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 :) [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files. 2014-03-27 18:07 ` Markus Armbruster @ 2014-03-27 19:46 ` Benoît Canet 0 siblings, 0 replies; 10+ messages in thread From: Benoît Canet @ 2014-03-27 19:46 UTC (permalink / raw) To: Markus Armbruster Cc: Benoît Canet, lcapitulino, qemu-devel, anthony, wenchaoqemu The Thursday 27 Mar 2014 à 19:07:48 (+0100), Markus Armbruster wrote : > Benoît Canet <benoit.canet@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 <benoit@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 > > [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-28 8:27 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-27 15:33 [Qemu-devel] [PATCH V2 0/3] Create an include directive for QAPI JSON files Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 1/3] test-qapi: Make test-qapi.py spit useful error messages Benoît Canet 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 2/3] qapi: Change the qapi scripts to take their input as first argument Benoît Canet 2014-03-27 17:27 ` Eric Blake 2014-03-28 8:27 ` Markus Armbruster 2014-03-27 17:56 ` Markus Armbruster 2014-03-27 15:33 ` [Qemu-devel] [PATCH V2 3/3] qapi: Create an include directive for use in the JSON description files Benoît Canet 2014-03-27 17:39 ` Eric Blake 2014-03-27 18:07 ` Markus Armbruster 2014-03-27 19:46 ` Benoît Canet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).