* [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
* [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 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 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 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
* 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
* 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
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.