* [Qemu-devel] [PATCH v6 0/4] qapi: Allow modularization of QAPI schema files
@ 2014-03-31 19:16 Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines Lluís Vilanova
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Lluís Vilanova @ 2014-03-31 19:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
Adds an include primitive to the syntax of QAPI schema files, allowing these to
be modularized into multiple per-topic files in the future.
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Changes in v6:
* Split changes for long-line breaking in makefiles.
* Put documentation on a separate section; reference recursiveness.
* Check (and test) for non-string include arguments (tanks to Benoît Canet).
Changes in v5:
* Rebase on b3706fa.
* Remove 'error_base' argument in 'parse_schema'; fix test checks instead.
* Implement include directive using JSON syntax.
Changes in v4:
* Rebase on 3e890c7.
* Minor cosmetic changes.
* Fix recording of included files in case of a cycle error.
* Add a more complex include cycle test.
Changes in v3:
* Fix documentation examples regarding how the input file is passed to the
scripts.
* Add documentation for the 'include' directive.
* Detect inclusion loops.
* Fix "tests/qapi-schema/test-qapi.py" and "tests/Makefile" to use an explicit
input file when running tests.
* Fix QAPI tests to cope with an explicit input file.
* Add tests for the "include" directive.
Changes in v2:
* Change the scripts to use an explicit input file instead of standard input.
* Fix "tests/Makefile" to use the new argument.
* Get the input directory for the "include" directive from the input file
dirname.
Lluís Vilanova (4):
qapi: [trivial] Break long command lines
qapi: Use an explicit input file
qapi: Add a primitive to include other files from a QAPI schema file
qapi: Add tests for the "include" directive
Makefile | 24 ++++++++---
docs/qapi-code-gen.txt | 15 ++++++-
scripts/qapi-commands.py | 10 +++--
scripts/qapi-types.py | 9 +++-
scripts/qapi-visit.py | 9 +++-
scripts/qapi.py | 44 +++++++++++++++++---
tests/Makefile | 19 ++++++---
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-cycle-b.json | 1
tests/qapi-schema/include-cycle-c.json | 1
tests/qapi-schema/include-cycle.err | 1
tests/qapi-schema/include-cycle.exit | 1
tests/qapi-schema/include-cycle.json | 1
tests/qapi-schema/include-cycle.out | 0
tests/qapi-schema/include-nested-err.err | 1
tests/qapi-schema/include-nested-err.exit | 1
tests/qapi-schema/include-nested-err.json | 1
tests/qapi-schema/include-nested-err.out | 0
tests/qapi-schema/include-no-file.err | 1
tests/qapi-schema/include-no-file.exit | 1
tests/qapi-schema/include-no-file.json | 1
tests/qapi-schema/include-no-file.out | 0
tests/qapi-schema/include-non-file.err | 1
tests/qapi-schema/include-non-file.exit | 1
tests/qapi-schema/include-non-file.json | 1
tests/qapi-schema/include-non-file.out | 0
tests/qapi-schema/include-self-cycle.err | 1
tests/qapi-schema/include-self-cycle.exit | 1
tests/qapi-schema/include-self-cycle.json | 1
tests/qapi-schema/include-self-cycle.out | 0
tests/qapi-schema/include-simple-sub.json | 2 +
tests/qapi-schema/include-simple.err | 0
tests/qapi-schema/include-simple.exit | 1
tests/qapi-schema/include-simple.json | 1
tests/qapi-schema/include-simple.out | 3 +
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 | 3 +
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 -
52 files changed, 144 insertions(+), 47 deletions(-)
create mode 100644 tests/qapi-schema/include-cycle-b.json
create mode 100644 tests/qapi-schema/include-cycle-c.json
create mode 100644 tests/qapi-schema/include-cycle.err
create mode 100644 tests/qapi-schema/include-cycle.exit
create mode 100644 tests/qapi-schema/include-cycle.json
create mode 100644 tests/qapi-schema/include-cycle.out
create mode 100644 tests/qapi-schema/include-nested-err.err
create mode 100644 tests/qapi-schema/include-nested-err.exit
create mode 100644 tests/qapi-schema/include-nested-err.json
create mode 100644 tests/qapi-schema/include-nested-err.out
create mode 100644 tests/qapi-schema/include-no-file.err
create mode 100644 tests/qapi-schema/include-no-file.exit
create mode 100644 tests/qapi-schema/include-no-file.json
create mode 100644 tests/qapi-schema/include-no-file.out
create mode 100644 tests/qapi-schema/include-non-file.err
create mode 100644 tests/qapi-schema/include-non-file.exit
create mode 100644 tests/qapi-schema/include-non-file.json
create mode 100644 tests/qapi-schema/include-non-file.out
create mode 100644 tests/qapi-schema/include-self-cycle.err
create mode 100644 tests/qapi-schema/include-self-cycle.exit
create mode 100644 tests/qapi-schema/include-self-cycle.json
create mode 100644 tests/qapi-schema/include-self-cycle.out
create mode 100644 tests/qapi-schema/include-simple-sub.json
create mode 100644 tests/qapi-schema/include-simple.err
create mode 100644 tests/qapi-schema/include-simple.exit
create mode 100644 tests/qapi-schema/include-simple.json
create mode 100644 tests/qapi-schema/include-simple.out
To: qemu-devel@nongnu.org
Cc: Luiz Capitulino <lcapitulino@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Benoît Canet <benoit.canet@irqsave.net>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines
2014-03-31 19:16 [Qemu-devel] [PATCH v6 0/4] qapi: Allow modularization of QAPI schema files Lluís Vilanova
@ 2014-03-31 19:16 ` Lluís Vilanova
2014-03-31 19:32 ` Eric Blake
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file Lluís Vilanova
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Lluís Vilanova @ 2014-03-31 19:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Makefile | 24 ++++++++++++++++++------
tests/Makefile | 12 +++++++++---
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index ec74039..84345ee 100644
--- a/Makefile
+++ b/Makefile
@@ -237,23 +237,35 @@ 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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+ $(gen-out-type) -o "." -m < $<, \
+ " 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/tests/Makefile b/tests/Makefile
index 2d021fb..5a2e130 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -216,13 +216,19 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+ $(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 $@")
+ $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+ $(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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file
2014-03-31 19:16 [Qemu-devel] [PATCH v6 0/4] qapi: Allow modularization of QAPI schema files Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines Lluís Vilanova
@ 2014-03-31 19:16 ` Lluís Vilanova
2014-03-31 19:42 ` Eric Blake
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive Lluís Vilanova
3 siblings, 1 reply; 13+ messages in thread
From: Lluís Vilanova @ 2014-03-31 19:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
Use an explicit input file on the command-line instead of reading from standard input
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
Makefile | 12 ++++++------
docs/qapi-code-gen.txt | 4 ++--
scripts/qapi-commands.py | 10 +++++++---
scripts/qapi-types.py | 9 ++++++---
scripts/qapi-visit.py | 9 ++++++---
scripts/qapi.py | 5 +++--
tests/Makefile | 9 +++++----
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 | 3 ++-
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 +-
25 files changed, 54 insertions(+), 41 deletions(-)
diff --git a/Makefile b/Makefile
index 84345ee..c7cec01 100644
--- a/Makefile
+++ b/Makefile
@@ -238,33 +238,33 @@ 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-out-type) -i "$<" -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-out-type) -i "$<" -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-out-type) -i "$<" -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-out-type) -i "$<" -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-out-type) -i "$<" -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) -o "." -m < $<, \
+ $(gen-out-type) -i "$<" -o "." -m, \
" GEN $@")
QGALIB_GEN=$(addprefix qga/qapi-generated/, qga-qapi-types.h qga-qapi-visit.h qga-qmp-commands.h)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index d78921f..824f6e5 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -221,7 +221,7 @@ created code.
Example:
mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-types.py \
- --output-dir="qapi-generated" --prefix="example-" < example-schema.json
+ --input-file=example-schema.json --output-dir="qapi-generated" --prefix="example-"
mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-types.c
/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
@@ -291,7 +291,7 @@ $(prefix)qapi-visit.h: declarations for previously mentioned visitor
Example:
mdroth@illuin:~/w/qemu2.git$ python scripts/qapi-visit.py \
- --output-dir="qapi-generated" --prefix="example-" < example-schema.json
+ --input-file=example-schema.json --output-dir="qapi-generated" --prefix="example-"
mdroth@illuin:~/w/qemu2.git$ cat qapi-generated/example-qapi-visit.c
/* THIS FILE IS AUTOMATICALLY GENERATED, DO NOT MODIFY */
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 9734ab0..910e867 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -369,13 +369,15 @@ 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[1:], "chp:i:o:m",
["source", "header", "prefix=",
- "output-dir=", "type=", "middle"])
+ "input-file=", "output-dir=",
+ "type=", "middle"])
except getopt.GetoptError, err:
print str(err)
sys.exit(1)
+input_file = ""
output_dir = ""
prefix = ""
dispatch_type = "sync"
@@ -389,6 +391,8 @@ do_h = False
for o, a in opts:
if o in ("-p", "--prefix"):
prefix = a
+ elif o in ("-i", "--input-file"):
+ input_file = a
elif o in ("-o", "--output-dir"):
output_dir = a + "/"
elif o in ("-t", "--type"):
@@ -420,7 +424,7 @@ except os.error, e:
if e.errno != errno.EEXIST:
raise
-exprs = parse_schema(sys.stdin)
+exprs = parse_schema(input_file)
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..b463232 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -279,14 +279,15 @@ 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[1:], "chbp:i:o:",
["source", "header", "builtins",
- "prefix=", "output-dir="])
+ "prefix=", "input-file=", "output-dir="])
except getopt.GetoptError, err:
print str(err)
sys.exit(1)
output_dir = ""
+input_file = ""
prefix = ""
c_file = 'qapi-types.c'
h_file = 'qapi-types.h'
@@ -298,6 +299,8 @@ do_builtins = False
for o, a in opts:
if o in ("-p", "--prefix"):
prefix = a
+ elif o in ("-i", "--input-file"):
+ input_file = a
elif o in ("-o", "--output-dir"):
output_dir = a + "/"
elif o in ("-c", "--source"):
@@ -378,7 +381,7 @@ fdecl.write(mcgen('''
''',
guard=guardname(h_file)))
-exprs = parse_schema(sys.stdin)
+exprs = parse_schema(input_file)
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..c6579be 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -397,13 +397,14 @@ 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[1:], "chbp:i:o:",
["source", "header", "builtins", "prefix=",
- "output-dir="])
+ "input-file=", "output-dir="])
except getopt.GetoptError, err:
print str(err)
sys.exit(1)
+input_file = ""
output_dir = ""
prefix = ""
c_file = 'qapi-visit.c'
@@ -416,6 +417,8 @@ do_builtins = False
for o, a in opts:
if o in ("-p", "--prefix"):
prefix = a
+ elif o in ("-i", "--input-file"):
+ input_file = a
elif o in ("-o", "--output-dir"):
output_dir = a + "/"
elif o in ("-c", "--source"):
@@ -494,7 +497,7 @@ fdecl.write(mcgen('''
''',
prefix=prefix, guard=guardname(h_file)))
-exprs = parse_schema(sys.stdin)
+exprs = parse_schema(input_file)
# 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..3a38e27 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 = [
@@ -263,9 +264,9 @@ def check_exprs(schema):
if expr.has_key('union'):
check_union(expr, expr_elem['info'])
-def parse_schema(fp):
+def parse_schema(input_path):
try:
- schema = QAPISchema(fp)
+ schema = QAPISchema(open(input_path, "r"))
except QAPISchemaError, e:
print >>sys.stderr, e
exit(1)
diff --git a/tests/Makefile b/tests/Makefile
index 5a2e130..f2beb1b 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -217,17 +217,17 @@ tests/test-vmstate$(EXESUF): tests/test-vmstate.o \
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-out-type) -i "$<" -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-out-type) -i "$<" -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-out-type) -i "$<" -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
@@ -368,7 +368,8 @@ 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")
+ @sed -i -e "s|$(SRC_PATH)/||g" $*.test.err
@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..768b276 100644
--- a/tests/qapi-schema/duplicate-key.err
+++ b/tests/qapi-schema/duplicate-key.err
@@ -1 +1 @@
-<stdin>:2:10: Duplicate key "key"
+tests/qapi-schema/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..ccf72d2 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'
+tests/qapi-schema/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..790b675 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'
+tests/qapi-schema/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..a59749e 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
+tests/qapi-schema/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..200016b 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
+tests/qapi-schema/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..bfc890c 100644
--- a/tests/qapi-schema/funny-char.err
+++ b/tests/qapi-schema/funny-char.err
@@ -1 +1 @@
-<stdin>:2:36: Stray ";"
+tests/qapi-schema/funny-char.json:2:36: Stray ";"
diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err
index 9f2a355..d9d66b3 100644
--- a/tests/qapi-schema/missing-colon.err
+++ b/tests/qapi-schema/missing-colon.err
@@ -1 +1 @@
-<stdin>:1:10: Expected ":"
+tests/qapi-schema/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..e73d277 100644
--- a/tests/qapi-schema/missing-comma-list.err
+++ b/tests/qapi-schema/missing-comma-list.err
@@ -1 +1 @@
-<stdin>:2:20: Expected "," or "]"
+tests/qapi-schema/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..52b3a8a 100644
--- a/tests/qapi-schema/missing-comma-object.err
+++ b/tests/qapi-schema/missing-comma-object.err
@@ -1 +1 @@
-<stdin>:2:3: Expected "," or "}"
+tests/qapi-schema/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..334f0c9 100644
--- a/tests/qapi-schema/non-objects.err
+++ b/tests/qapi-schema/non-objects.err
@@ -1 +1 @@
-<stdin>:1:1: Expected "{"
+tests/qapi-schema/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..9b18384 100644
--- a/tests/qapi-schema/quoted-structural-chars.err
+++ b/tests/qapi-schema/quoted-structural-chars.err
@@ -1 +1 @@
-<stdin>:1:1: Expected "{"
+tests/qapi-schema/quoted-structural-chars.json:1:1: Expected "{"
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index b3d1e1d..cabf95c 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -12,10 +12,11 @@
from qapi import *
from pprint import pprint
+import os
import sys
try:
- exprs = parse_schema(sys.stdin)
+ exprs = parse_schema(sys.argv[1])
except SystemExit:
raise
except:
diff --git a/tests/qapi-schema/trailing-comma-list.err b/tests/qapi-schema/trailing-comma-list.err
index ff839a3..24c24b0 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
+tests/qapi-schema/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..30bce5e 100644
--- a/tests/qapi-schema/trailing-comma-object.err
+++ b/tests/qapi-schema/trailing-comma-object.err
@@ -1 +1 @@
-<stdin>:2:38: Expected string
+tests/qapi-schema/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..fb41a86 100644
--- a/tests/qapi-schema/unclosed-list.err
+++ b/tests/qapi-schema/unclosed-list.err
@@ -1 +1 @@
-<stdin>:1:20: Expected "," or "]"
+tests/qapi-schema/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..db3deed 100644
--- a/tests/qapi-schema/unclosed-object.err
+++ b/tests/qapi-schema/unclosed-object.err
@@ -1 +1 @@
-<stdin>:1:21: Expected "," or "}"
+tests/qapi-schema/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..12b1870 100644
--- a/tests/qapi-schema/unclosed-string.err
+++ b/tests/qapi-schema/unclosed-string.err
@@ -1 +1 @@
-<stdin>:1:11: Missing terminating "'"
+tests/qapi-schema/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..938f969 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
+tests/qapi-schema/union-invalid-base.json:7: Base 'TestBaseWrong' is not a valid type
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file
2014-03-31 19:16 [Qemu-devel] [PATCH v6 0/4] qapi: Allow modularization of QAPI schema files Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file Lluís Vilanova
@ 2014-03-31 19:16 ` Lluís Vilanova
2014-03-31 20:00 ` Eric Blake
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive Lluís Vilanova
3 siblings, 1 reply; 13+ messages in thread
From: Lluís Vilanova @ 2014-03-31 19:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
docs/qapi-code-gen.txt | 11 +++++++++++
scripts/qapi.py | 39 ++++++++++++++++++++++++++++++++++-----
2 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 824f6e5..70b4eeb 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -40,6 +40,17 @@ enumeration types and union types.
Generally speaking, types definitions should always use CamelCase for the type
names. Command names should be all lower case with words separated by a hyphen.
+
+=== Includes ===
+
+The QAPI schema definitions can be modularized using the 'include' directive:
+
+ { 'include': 'path/to/file.json'}
+
+The directive is evaluated recursively, and include paths are relative to the
+file using the directive.
+
+
=== Complex types ===
A complex type is a dictionary containing a single key whose value is a
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3a38e27..9f73426 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,6 +11,8 @@
# This work is licensed under the terms of the GNU GPL, version 2.
# See the COPYING file in the top-level directory.
+import os
+import re
from ordereddict import OrderedDict
import os
import sys
@@ -62,8 +64,11 @@ class QAPIExprError(Exception):
class QAPISchema:
- def __init__(self, fp):
+ def __init__(self, fp, included=[]):
self.fp = fp
+ input_path = os.path.abspath(fp.name)
+ self.included = included + [input_path]
+ self.input_dir = os.path.dirname(input_path)
self.src = fp.read()
if self.src == '' or self.src[-1] != '\n':
self.src += '\n'
@@ -75,9 +80,33 @@ class QAPISchema:
while self.tok != None:
expr_info = {'fp': fp, 'line': self.line}
- expr_elem = {'expr': self.get_expr(False),
- 'info': expr_info}
- self.exprs.append(expr_elem)
+ expr = self.get_expr(False)
+ if isinstance(expr, dict) and "include" in expr:
+ if len(expr) != 1:
+ raise QAPIExprError(expr_info, "Invalid 'include' directive")
+ include_path = expr["include"]
+ if not isinstance(include_path, str):
+ raise QAPIExprError(expr_info,
+ 'Expected a file path (string), got: %s'
+ % include_path)
+ if not os.path.isabs(include_path):
+ include_path = os.path.join(self.input_dir, include_path)
+ if not os.path.isfile(include_path):
+ raise QAPIExprError(
+ expr_info,
+ 'Non-existing included file "%s"' %
+ include_path)
+ if include_path in self.included:
+ raise QAPIExprError(expr_info, "Infinite inclusion loop: %s"
+ % " -> ".join(self.included +
+ [include_path]))
+ exprs_include = QAPISchema(open(include_path, "r"),
+ self.included)
+ self.exprs.extend(exprs_include.exprs)
+ else:
+ expr_elem = {'expr': expr,
+ 'info': expr_info}
+ self.exprs.append(expr_elem)
def accept(self):
while True:
@@ -267,7 +296,7 @@ def check_exprs(schema):
def parse_schema(input_path):
try:
schema = QAPISchema(open(input_path, "r"))
- except QAPISchemaError, e:
+ except (QAPISchemaError, QAPIExprError), e:
print >>sys.stderr, e
exit(1)
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive
2014-03-31 19:16 [Qemu-devel] [PATCH v6 0/4] qapi: Allow modularization of QAPI schema files Lluís Vilanova
` (2 preceding siblings ...)
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file Lluís Vilanova
@ 2014-03-31 19:16 ` Lluís Vilanova
2014-03-31 20:17 ` Eric Blake
3 siblings, 1 reply; 13+ messages in thread
From: Lluís Vilanova @ 2014-03-31 19:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
tests/Makefile | 4 +++-
tests/qapi-schema/include-cycle-b.json | 1 +
tests/qapi-schema/include-cycle-c.json | 1 +
tests/qapi-schema/include-cycle.err | 1 +
tests/qapi-schema/include-cycle.exit | 1 +
tests/qapi-schema/include-cycle.json | 1 +
tests/qapi-schema/include-cycle.out | 0
tests/qapi-schema/include-nested-err.err | 1 +
tests/qapi-schema/include-nested-err.exit | 1 +
tests/qapi-schema/include-nested-err.json | 1 +
tests/qapi-schema/include-nested-err.out | 0
tests/qapi-schema/include-no-file.err | 1 +
tests/qapi-schema/include-no-file.exit | 1 +
tests/qapi-schema/include-no-file.json | 1 +
tests/qapi-schema/include-no-file.out | 0
tests/qapi-schema/include-non-file.err | 1 +
tests/qapi-schema/include-non-file.exit | 1 +
tests/qapi-schema/include-non-file.json | 1 +
tests/qapi-schema/include-non-file.out | 0
tests/qapi-schema/include-self-cycle.err | 1 +
tests/qapi-schema/include-self-cycle.exit | 1 +
tests/qapi-schema/include-self-cycle.json | 1 +
tests/qapi-schema/include-self-cycle.out | 0
tests/qapi-schema/include-simple-sub.json | 2 ++
tests/qapi-schema/include-simple.err | 0
tests/qapi-schema/include-simple.exit | 1 +
tests/qapi-schema/include-simple.json | 1 +
tests/qapi-schema/include-simple.out | 3 +++
28 files changed, 27 insertions(+), 1 deletion(-)
create mode 100644 tests/qapi-schema/include-cycle-b.json
create mode 100644 tests/qapi-schema/include-cycle-c.json
create mode 100644 tests/qapi-schema/include-cycle.err
create mode 100644 tests/qapi-schema/include-cycle.exit
create mode 100644 tests/qapi-schema/include-cycle.json
create mode 100644 tests/qapi-schema/include-cycle.out
create mode 100644 tests/qapi-schema/include-nested-err.err
create mode 100644 tests/qapi-schema/include-nested-err.exit
create mode 100644 tests/qapi-schema/include-nested-err.json
create mode 100644 tests/qapi-schema/include-nested-err.out
create mode 100644 tests/qapi-schema/include-no-file.err
create mode 100644 tests/qapi-schema/include-no-file.exit
create mode 100644 tests/qapi-schema/include-no-file.json
create mode 100644 tests/qapi-schema/include-no-file.out
create mode 100644 tests/qapi-schema/include-non-file.err
create mode 100644 tests/qapi-schema/include-non-file.exit
create mode 100644 tests/qapi-schema/include-non-file.json
create mode 100644 tests/qapi-schema/include-non-file.out
create mode 100644 tests/qapi-schema/include-self-cycle.err
create mode 100644 tests/qapi-schema/include-self-cycle.exit
create mode 100644 tests/qapi-schema/include-self-cycle.json
create mode 100644 tests/qapi-schema/include-self-cycle.out
create mode 100644 tests/qapi-schema/include-simple-sub.json
create mode 100644 tests/qapi-schema/include-simple.err
create mode 100644 tests/qapi-schema/include-simple.exit
create mode 100644 tests/qapi-schema/include-simple.json
create mode 100644 tests/qapi-schema/include-simple.out
diff --git a/tests/Makefile b/tests/Makefile
index f2beb1b..4fcbc55 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -164,7 +164,9 @@ 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-simple.json include-non-file.json include-no-file.json \
+ include-nested-err.json include-self-cycle.json include-cycle.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-cycle-b.json b/tests/qapi-schema/include-cycle-b.json
new file mode 100644
index 0000000..4fa985d
--- /dev/null
+++ b/tests/qapi-schema/include-cycle-b.json
@@ -0,0 +1 @@
+{ 'include': 'include-cycle-c.json' }
diff --git a/tests/qapi-schema/include-cycle-c.json b/tests/qapi-schema/include-cycle-c.json
new file mode 100644
index 0000000..d12b592
--- /dev/null
+++ b/tests/qapi-schema/include-cycle-c.json
@@ -0,0 +1 @@
+{ 'include': 'include-cycle.json' }
diff --git a/tests/qapi-schema/include-cycle.err b/tests/qapi-schema/include-cycle.err
new file mode 100644
index 0000000..7a74655
--- /dev/null
+++ b/tests/qapi-schema/include-cycle.err
@@ -0,0 +1 @@
+tests/qapi-schema/include-cycle-c.json:1: Infinite inclusion loop: tests/qapi-schema/include-cycle.json -> tests/qapi-schema/include-cycle-b.json -> tests/qapi-schema/include-cycle-c.json -> tests/qapi-schema/include-cycle.json
diff --git a/tests/qapi-schema/include-cycle.exit b/tests/qapi-schema/include-cycle.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/include-cycle.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/include-cycle.json b/tests/qapi-schema/include-cycle.json
new file mode 100644
index 0000000..6fcf1eb
--- /dev/null
+++ b/tests/qapi-schema/include-cycle.json
@@ -0,0 +1 @@
+{ 'include': 'include-cycle-b.json' }
diff --git a/tests/qapi-schema/include-cycle.out b/tests/qapi-schema/include-cycle.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-nested-err.err b/tests/qapi-schema/include-nested-err.err
new file mode 100644
index 0000000..d9d66b3
--- /dev/null
+++ b/tests/qapi-schema/include-nested-err.err
@@ -0,0 +1 @@
+tests/qapi-schema/missing-colon.json:1:10: Expected ":"
diff --git a/tests/qapi-schema/include-nested-err.exit b/tests/qapi-schema/include-nested-err.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/include-nested-err.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/include-nested-err.json b/tests/qapi-schema/include-nested-err.json
new file mode 100644
index 0000000..5631e56
--- /dev/null
+++ b/tests/qapi-schema/include-nested-err.json
@@ -0,0 +1 @@
+{ 'include': 'missing-colon.json' }
diff --git a/tests/qapi-schema/include-nested-err.out b/tests/qapi-schema/include-nested-err.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-no-file.err b/tests/qapi-schema/include-no-file.err
new file mode 100644
index 0000000..28adaee
--- /dev/null
+++ b/tests/qapi-schema/include-no-file.err
@@ -0,0 +1 @@
+tests/qapi-schema/include-no-file.json:1: Non-existing included file "tests/qapi-schema/include-no-file-sub.json"
diff --git a/tests/qapi-schema/include-no-file.exit b/tests/qapi-schema/include-no-file.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/include-no-file.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/include-no-file.json b/tests/qapi-schema/include-no-file.json
new file mode 100644
index 0000000..9249ebd
--- /dev/null
+++ b/tests/qapi-schema/include-no-file.json
@@ -0,0 +1 @@
+{ 'include': 'include-no-file-sub.json' }
diff --git a/tests/qapi-schema/include-no-file.out b/tests/qapi-schema/include-no-file.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-non-file.err b/tests/qapi-schema/include-non-file.err
new file mode 100644
index 0000000..6418482
--- /dev/null
+++ b/tests/qapi-schema/include-non-file.err
@@ -0,0 +1 @@
+tests/qapi-schema/include-non-file.json:1: Expected a file path (string), got: ['foo', 'bar']
diff --git a/tests/qapi-schema/include-non-file.exit b/tests/qapi-schema/include-non-file.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/include-non-file.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/include-non-file.json b/tests/qapi-schema/include-non-file.json
new file mode 100644
index 0000000..cd43c3f
--- /dev/null
+++ b/tests/qapi-schema/include-non-file.json
@@ -0,0 +1 @@
+{ 'include': [ 'foo', 'bar' ] }
diff --git a/tests/qapi-schema/include-non-file.out b/tests/qapi-schema/include-non-file.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-self-cycle.err b/tests/qapi-schema/include-self-cycle.err
new file mode 100644
index 0000000..7a72174
--- /dev/null
+++ b/tests/qapi-schema/include-self-cycle.err
@@ -0,0 +1 @@
+tests/qapi-schema/include-self-cycle.json:1: Infinite inclusion loop: tests/qapi-schema/include-self-cycle.json -> tests/qapi-schema/include-self-cycle.json
diff --git a/tests/qapi-schema/include-self-cycle.exit b/tests/qapi-schema/include-self-cycle.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/include-self-cycle.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/include-self-cycle.json b/tests/qapi-schema/include-self-cycle.json
new file mode 100644
index 0000000..55fb1b5
--- /dev/null
+++ b/tests/qapi-schema/include-self-cycle.json
@@ -0,0 +1 @@
+{ 'include': 'include-self-cycle.json' }
diff --git a/tests/qapi-schema/include-self-cycle.out b/tests/qapi-schema/include-self-cycle.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-simple-sub.json b/tests/qapi-schema/include-simple-sub.json
new file mode 100644
index 0000000..4bd4af4
--- /dev/null
+++ b/tests/qapi-schema/include-simple-sub.json
@@ -0,0 +1,2 @@
+{ 'enum': 'Status',
+ 'data': [ 'good', 'bad', 'ugly' ] }
diff --git a/tests/qapi-schema/include-simple.err b/tests/qapi-schema/include-simple.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/include-simple.exit b/tests/qapi-schema/include-simple.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/include-simple.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/include-simple.json b/tests/qapi-schema/include-simple.json
new file mode 100644
index 0000000..1dd391a
--- /dev/null
+++ b/tests/qapi-schema/include-simple.json
@@ -0,0 +1 @@
+{ 'include': 'include-simple-sub.json' }
diff --git a/tests/qapi-schema/include-simple.out b/tests/qapi-schema/include-simple.out
new file mode 100644
index 0000000..4ce3dcf
--- /dev/null
+++ b/tests/qapi-schema/include-simple.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
+[]
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines Lluís Vilanova
@ 2014-03-31 19:32 ` Eric Blake
0 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2014-03-31 19:32 UTC (permalink / raw)
To: Lluís Vilanova, qemu-devel
Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 411 bytes --]
On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> Makefile | 24 ++++++++++++++++++------
> tests/Makefile | 12 +++++++++---
> 2 files changed, 27 insertions(+), 9 deletions(-)
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] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file Lluís Vilanova
@ 2014-03-31 19:42 ` Eric Blake
2014-04-01 14:05 ` Lluís Vilanova
0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2014-03-31 19:42 UTC (permalink / raw)
To: Lluís Vilanova, qemu-devel
Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 3174 bytes --]
On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
> Use an explicit input file on the command-line instead of reading from standard input
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> +++ b/Makefile
> @@ -238,33 +238,33 @@ 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-out-type) -i "$<" -o qga/qapi-generated -p "qga-", \
I still wonder if:
-o qga/qapi-generated -p "qga-" -i "$<"
is easier to read than injecting -i up front. But it's not something
that I will block the patch for.
> @@ -368,7 +368,8 @@ 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")
Why is this line still long? Shouldn't it have been split in 1/4? And
why is this form using an argument of the input file, instead of adding
a -i option like all the others?
> + @sed -i -e "s|$(SRC_PATH)/||g" $*.test.err
'sed -i' is a GNU-ism, and not portable to all sed. It's not the first
use of 'sed -i' in the code base, but most of those uses so far are for
situations that are Linux-specific where GNU sed can be assumed.
> @diff -q $(SRC_PATH)/$*.out $*.test.out
> @diff -q $(SRC_PATH)/$*.err $*.test.err
You could instead have done:
@sed "s|$(SRC_PATH)/||g" $*.test.err \
| diff -q $(SRC_PATH_/$*.err -
to avoid the portability question.
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -12,10 +12,11 @@
>
> from qapi import *
> from pprint import pprint
> +import os
> import sys
>
> try:
> - exprs = parse_schema(sys.stdin)
> + exprs = parse_schema(sys.argv[1])
Here's where I found it inconsistent with the rest of the patch. It
seems it would be better to either use -i everywhere, or use sys.argv[1]
everywhere.
> +++ b/tests/qapi-schema/trailing-comma-list.err
> @@ -1 +1 @@
> -<stdin>:2:36: Expected "{", "[" or string
> +tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or string
These are some long error messages; is it also worth trimming the
leading "tests/qapi-schema/" in the sed script where you massage the
data before doing the diff on expected errors?
It's too late for this to go into 2.0, but I think we're getting close
to having a solution worth using at the start of the 2.1 devel cycle.
--
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] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file Lluís Vilanova
@ 2014-03-31 20:00 ` Eric Blake
2014-04-01 13:46 ` Lluís Vilanova
0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2014-03-31 20:00 UTC (permalink / raw)
To: Lluís Vilanova, qemu-devel
Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 3359 bytes --]
On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> docs/qapi-code-gen.txt | 11 +++++++++++
> scripts/qapi.py | 39 ++++++++++++++++++++++++++++++++++-----
> 2 files changed, 45 insertions(+), 5 deletions(-)
I would consider squashing 3 and 4 together, but not the end of the
world to keep them separate.
> @@ -75,9 +80,33 @@ class QAPISchema:
>
> while self.tok != None:
> expr_info = {'fp': fp, 'line': self.line}
> - expr_elem = {'expr': self.get_expr(False),
> - 'info': expr_info}
> - self.exprs.append(expr_elem)
> + expr = self.get_expr(False)
> + if isinstance(expr, dict) and "include" in expr:
> + if len(expr) != 1:
> + raise QAPIExprError(expr_info, "Invalid 'include' directive")
Not covered in patch 4. Ideally, ALL new error messages also come with
a test that exposes the message.
> + include_path = expr["include"]
> + if not isinstance(include_path, str):
> + raise QAPIExprError(expr_info,
> + 'Expected a file path (string), got: %s'
> + % include_path)
s/path/name/ ("path" implies a series of directories to some people)
Good, this one is covered by include-non-file.err
> + if not os.path.isabs(include_path):
> + include_path = os.path.join(self.input_dir, include_path)
> + if not os.path.isfile(include_path):
> + raise QAPIExprError(
> + expr_info,
> + 'Non-existing included file "%s"' %
> + include_path)
Is this error necessary, or would you get a sane error message by just
trying to open the file?
s/included/include/
Good, covered by include-no-file.err
> + if include_path in self.included:
> + raise QAPIExprError(expr_info, "Infinite inclusion loop: %s"
> + % " -> ".join(self.included +
> + [include_path]))
Good, include-self-cycle.err covers a one-file loop, and
include-cycle.err covers a 3-file loop.
Not so good: your error message is an extremely long line:
+tests/qapi-schema/include-cycle-c.json:1: Infinite inclusion loop:
tests/qapi-schema/include-cycle.json ->
tests/qapi-schema/include-cycle-b.json ->
tests/qapi-schema/include-cycle-c.json ->
tests/qapi-schema/include-cycle.json
The formatting in Benoît's series was a little nicer aesthetically:
+Inclusion loop detected with file: multi_file_loop_include.json
+Path to the broken include is:
+ multi_file_loop_include.json
+ multi_loop.json
Furthermore, it had the benefit of using the spelling provided by the
user, rather than the absolute path to the files. You want to track
canonical paths for detecting the loop, but do NOT want to report
absolute paths back to the user - instead, it's nicer to report back the
names as they spelled it.
--
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] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive Lluís Vilanova
@ 2014-03-31 20:17 ` Eric Blake
0 siblings, 0 replies; 13+ messages in thread
From: Eric Blake @ 2014-03-31 20:17 UTC (permalink / raw)
To: Lluís Vilanova, qemu-devel
Cc: Benoît Canet, Markus Armbruster, Luiz Capitulino
[-- Attachment #1: Type: text/plain, Size: 1906 bytes --]
On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
> tests/Makefile | 4 +++-
> tests/qapi-schema/include-cycle-b.json | 1 +
...
> tests/qapi-schema/include-simple.out | 3 +++
Hmm, no tests of cross-directory inclusion. In Benoît's series, there
was a test that "a" includes "dir/b" includes "../c" finds the correct
file "c" (well, the filenames were spelled differently, but the idea is
to make sure that we are fully exercising the "relative to the file that
has the include statement").
> +++ b/tests/qapi-schema/include-nested-err.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/missing-colon.json:1:10: Expected ":"
> +++ b/tests/qapi-schema/include-nested-err.json
> @@ -0,0 +1 @@
> +{ 'include': 'missing-colon.json' }
Nice - it proves the error reporting got the right line number in the
nested file.
Not so nice - it doesn't tell how we got there. I like gcc's notion of
telling you what files were included along the way to an error, as in:
$ echo '#include "foo.h"' > foo.c
$ echo 'choke me' > foo.h
$ gcc -c -o /dev/null -Wall foo.c
In file included from foo.c:1:0:
foo.h:1:1: error: unknown type name ‘choke’
choke me
^
foo.c:1:0: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ at
end of input
#include "foo.h"
^
Note how it mentions "In file included from foo.c:1:0:" before
proceeding to tell me about the bug in "foo.h".
Also might be worth a test that an error after an include directive
correctly has the right context back in the source file, as in:
{ 'include': 'good-sub.json' }
{ 'command' 'missing-colon' }
having the correct line number information about the missing colon.
--
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] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file
2014-03-31 20:00 ` Eric Blake
@ 2014-04-01 13:46 ` Lluís Vilanova
2014-04-01 14:47 ` Eric Blake
0 siblings, 1 reply; 13+ messages in thread
From: Lluís Vilanova @ 2014-04-01 13:46 UTC (permalink / raw)
To: Eric Blake
Cc: Benoît Canet, Markus Armbruster, qemu-devel, Luiz Capitulino
Eric Blake writes:
> On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
[...]
>> + if not os.path.isabs(include_path):
>> + include_path = os.path.join(self.input_dir, include_path)
>> + if not os.path.isfile(include_path):
>> + raise QAPIExprError(
>> + expr_info,
>> + 'Non-existing included file "%s"' %
>> + include_path)
> Is this error necessary, or would you get a sane error message by just
> trying to open the file?
Not catching it would throw a Python exception, with no context of how the user
got there.
[...]
>> + if include_path in self.included:
>> + raise QAPIExprError(expr_info, "Infinite inclusion loop: %s"
>> + % " -> ".join(self.included +
>> + [include_path]))
> Good, include-self-cycle.err covers a one-file loop, and
> include-cycle.err covers a 3-file loop.
> Not so good: your error message is an extremely long line:
> +tests/qapi-schema/include-cycle-c.json:1: Infinite inclusion loop:
> tests/qapi-schema/include-cycle.json ->
> tests/qapi-schema/include-cycle-b.json ->
> tests/qapi-schema/include-cycle-c.json ->
> tests/qapi-schema/include-cycle.json
> The formatting in Benoît's series was a little nicer aesthetically:
> +Inclusion loop detected with file: multi_file_loop_include.json
> +Path to the broken include is:
> + multi_file_loop_include.json
> + multi_loop.json
> Furthermore, it had the benefit of using the spelling provided by the
> user, rather than the absolute path to the files. You want to track
> canonical paths for detecting the loop, but do NOT want to report
> absolute paths back to the user - instead, it's nicer to report back the
> names as they spelled it.
I think it's better reporting absolute paths, otherwise the user has to mentally
keep track of the relative paths to get to the file.
Thanks,
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file
2014-03-31 19:42 ` Eric Blake
@ 2014-04-01 14:05 ` Lluís Vilanova
0 siblings, 0 replies; 13+ messages in thread
From: Lluís Vilanova @ 2014-04-01 14:05 UTC (permalink / raw)
To: Eric Blake
Cc: Benoît Canet, Markus Armbruster, qemu-devel, Luiz Capitulino
Eric Blake writes:
> On 03/31/2014 01:16 PM, Lluís Vilanova wrote:
[...]
>> @@ -368,7 +368,8 @@ 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")
> Why is this line still long? Shouldn't it have been split in 1/4? And
> why is this form using an argument of the input file, instead of adding
> a -i option like all the others?
The "test-qapi.py" was already not using arguments, so I did not add support for
it, since it has one single mandatory argument.
[...]
>> +++ b/tests/qapi-schema/test-qapi.py
>> @@ -12,10 +12,11 @@
>>
>> from qapi import *
>> from pprint import pprint
>> +import os
>> import sys
>>
>> try:
>> - exprs = parse_schema(sys.stdin)
>> + exprs = parse_schema(sys.argv[1])
> Here's where I found it inconsistent with the rest of the patch. It
> seems it would be better to either use -i everywhere, or use sys.argv[1]
> everywhere.
I just did not want to add argument parsing when there's only one mandatory
argument.
>> +++ b/tests/qapi-schema/trailing-comma-list.err
>> @@ -1 +1 @@
>> -<stdin>:2:36: Expected "{", "[" or string
>> +tests/qapi-schema/trailing-comma-list.json:2:36: Expected "{", "[" or string
> These are some long error messages; is it also worth trimming the
> leading "tests/qapi-schema/" in the sed script where you massage the
> data before doing the diff on expected errors?
I'm neutral about this, since the user will barely ever look at the test
contents.
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file
2014-04-01 13:46 ` Lluís Vilanova
@ 2014-04-01 14:47 ` Eric Blake
2014-04-01 16:05 ` Lluís Vilanova
0 siblings, 1 reply; 13+ messages in thread
From: Eric Blake @ 2014-04-01 14:47 UTC (permalink / raw)
To: qemu-devel, Luiz Capitulino, Markus Armbruster, Benoît Canet
[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]
On 04/01/2014 07:46 AM, Lluís Vilanova wrote:
>> The formatting in Benoît's series was a little nicer aesthetically:
>
>> +Inclusion loop detected with file: multi_file_loop_include.json
>> +Path to the broken include is:
>> + multi_file_loop_include.json
>> + multi_loop.json
>
>> Furthermore, it had the benefit of using the spelling provided by the
>> user, rather than the absolute path to the files. You want to track
>> canonical paths for detecting the loop, but do NOT want to report
>> absolute paths back to the user - instead, it's nicer to report back the
>> names as they spelled it.
>
> I think it's better reporting absolute paths, otherwise the user has to mentally
> keep track of the relative paths to get to the file.
If you display the entire chain of paths that you opened as spelled by
the user, the user shouldn't have that hard a time finding the right
file. Furthermore, I seriously doubt we will be including files from
very many subdirectories in our use of this feature - the error is there
to aid a developer adding a new include, and won't ever trigger in
normal usage. If a developer triggers the error, they KNOW that the
error was caused by the very file that they were working on.
--
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] 13+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file
2014-04-01 14:47 ` Eric Blake
@ 2014-04-01 16:05 ` Lluís Vilanova
0 siblings, 0 replies; 13+ messages in thread
From: Lluís Vilanova @ 2014-04-01 16:05 UTC (permalink / raw)
To: Eric Blake
Cc: Benoît Canet, Markus Armbruster, qemu-devel, Luiz Capitulino
Eric Blake writes:
> On 04/01/2014 07:46 AM, Lluís Vilanova wrote:
>>> The formatting in Benoît's series was a little nicer aesthetically:
>>
>>> +Inclusion loop detected with file: multi_file_loop_include.json
>>> +Path to the broken include is:
>>> + multi_file_loop_include.json
>>> + multi_loop.json
>>
>>> Furthermore, it had the benefit of using the spelling provided by the
>>> user, rather than the absolute path to the files. You want to track
>>> canonical paths for detecting the loop, but do NOT want to report
>>> absolute paths back to the user - instead, it's nicer to report back the
>>> names as they spelled it.
>>
>> I think it's better reporting absolute paths, otherwise the user has to mentally
>> keep track of the relative paths to get to the file.
> If you display the entire chain of paths that you opened as spelled by
> the user, the user shouldn't have that hard a time finding the right
> file. Furthermore, I seriously doubt we will be including files from
> very many subdirectories in our use of this feature - the error is there
> to aid a developer adding a new include, and won't ever trigger in
> normal usage. If a developer triggers the error, they KNOW that the
> error was caused by the very file that they were working on.
Ok, then I can send a v8 with that change. Please check v7 to see if there are
more changes required.
Thanks,
Lluis
--
"And it's much the same thing with knowledge, for whenever you learn
something new, the whole world becomes that much richer."
-- The Princess of Pure Reason, as told by Norton Juster in The Phantom
Tollbooth
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-04-01 16:05 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-31 19:16 [Qemu-devel] [PATCH v6 0/4] qapi: Allow modularization of QAPI schema files Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 1/4] qapi: [trivial] Break long command lines Lluís Vilanova
2014-03-31 19:32 ` Eric Blake
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 2/4] qapi: Use an explicit input file Lluís Vilanova
2014-03-31 19:42 ` Eric Blake
2014-04-01 14:05 ` Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 3/4] qapi: Add a primitive to include other files from a QAPI schema file Lluís Vilanova
2014-03-31 20:00 ` Eric Blake
2014-04-01 13:46 ` Lluís Vilanova
2014-04-01 14:47 ` Eric Blake
2014-04-01 16:05 ` Lluís Vilanova
2014-03-31 19:16 ` [Qemu-devel] [PATCH v6 4/4] qapi: Add tests for the "include" directive Lluís Vilanova
2014-03-31 20:17 ` Eric Blake
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).