[Qemu-devel] [PATCH v4 0/3] qapi: Allow modularization of QAPI schema files

[Qemu-devel] [PATCH v4 0/3] qapi: Allow modularization of QAPI schema files

[Lluís Vilanova]

Adds the "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 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 (3):
      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                        |   12 +++++-
 scripts/qapi-commands.py                      |   10 ++++-
 scripts/qapi-types.py                         |    9 +++--
 scripts/qapi-visit.py                         |    9 +++--
 scripts/qapi.py                               |   47 +++++++++++++++++++++++--
 tests/Makefile                                |   18 +++++++---
 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-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 +
 42 files changed, 137 insertions(+), 38 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-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>

[Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Lluís Vilanova]

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                                      |   24 ++++++++++++++++++------
 docs/qapi-code-gen.txt                        |    4 ++--
 scripts/qapi-commands.py                      |   10 +++++++---
 scripts/qapi-types.py                         |    9 ++++++---
 scripts/qapi-visit.py                         |    9 ++++++---
 scripts/qapi.py                               |   15 +++++++++++----
 tests/Makefile                                |   14 ++++++++++----
 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 +-
 19 files changed, 73 insertions(+), 37 deletions(-)

diff --git a/Makefile b/Makefile
index 988438f..e82d49f 100644
--- a/Makefile
+++ b/Makefile
@@ -217,23 +217,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) -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   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+		$(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   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+		$(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   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
+		$(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   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+		$(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) -m -o "." < $<, "  GEN   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+		$(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)
 $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 0728f36..2e9f036 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -220,7 +220,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 */
 
@@ -290,7 +290,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 b12b696..1657f21 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -389,13 +389,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"
@@ -409,6 +411,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"):
@@ -440,7 +444,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 4a1652b..7304543 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -282,14 +282,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'
@@ -301,6 +302,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"):
@@ -381,7 +384,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 65f1a54..856f969 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -383,13 +383,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'
@@ -402,6 +403,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"):
@@ -480,7 +483,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 9b3de4c..59c2b9b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -12,6 +12,7 @@
 # See the COPYING.LIB file in the top-level directory.
 
 from ordereddict import OrderedDict
+import os
 import sys
 
 builtin_types = [
@@ -37,6 +38,7 @@ builtin_type_qtypes = {
 
 class QAPISchemaError(Exception):
     def __init__(self, schema, msg):
+        self.base = schema.error_base
         self.fp = schema.fp
         self.msg = msg
         self.line = self.col = 1
@@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
                 self.col += 1
 
     def __str__(self):
-        return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg)
+        name = os.path.relpath(self.fp.name, self.base)
+        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)
 
 class QAPISchema:
 
-    def __init__(self, fp):
+    def __init__(self, fp, error_base=None):
         self.fp = fp
+        if error_base is None:
+            self.error_base = os.getcwd()
+        else:
+            self.error_base = error_base
         self.src = fp.read()
         if self.src == '' or self.src[-1] != '\n':
             self.src += '\n'
@@ -158,9 +165,9 @@ class QAPISchema:
             raise QAPISchemaError(self, 'Expected "{", "[" or string')
         return expr
 
-def parse_schema(fp):
+def parse_schema(input_path, error_base=None):
     try:
-        schema = QAPISchema(fp)
+        schema = QAPISchema(open(input_path, "r"), error_base)
     except QAPISchemaError, e:
         print >>sys.stderr, e
         exit(1)
diff --git a/tests/Makefile b/tests/Makefile
index b17d41e..02b0dbc 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -192,13 +192,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) -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   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
+		$(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   $@")
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
+		$(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
 tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
@@ -331,7 +337,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/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 b3d1e1d..f97e886 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], os.path.dirname(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..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 "'"

Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Markus Armbruster]

Lluís Vilanova <vilanova@ac.upc.edu> writes:

> 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                                      |   24 ++++++++++++++++++------
>  docs/qapi-code-gen.txt                        |    4 ++--
>  scripts/qapi-commands.py                      |   10 +++++++---
>  scripts/qapi-types.py                         |    9 ++++++---
>  scripts/qapi-visit.py                         |    9 ++++++---
>  scripts/qapi.py                               |   15 +++++++++++----
>  tests/Makefile                                |   14 ++++++++++----
>  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 +-
>  19 files changed, 73 insertions(+), 37 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 988438f..e82d49f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -217,23 +217,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) -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   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
> +		$(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   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
> +		$(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   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
> +		$(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   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
> +		$(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) -m -o "." < $<, "  GEN   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
> +		$(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)
>  $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN)
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 0728f36..2e9f036 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -220,7 +220,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 */
>  
> @@ -290,7 +290,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 b12b696..1657f21 100644
> --- a/scripts/qapi-commands.py
> +++ b/scripts/qapi-commands.py
> @@ -389,13 +389,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"
> @@ -409,6 +411,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"):
> @@ -440,7 +444,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 4a1652b..7304543 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -282,14 +282,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'
> @@ -301,6 +302,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"):

Option -i isn't optional:

    $ python ../scripts/qapi-types.py -o tests
    Traceback (most recent call last):
      File "../scripts/qapi-types.py", line 387, in <module>
        exprs = parse_schema(input_file)
      File "/work/armbru/qemu/scripts/qapi.py", line 202, in parse_schema
        schema = QAPISchema(open(input_path, "r"), input_dir, error_base)
    IOError: [Errno 2] No such file or directory: ''

The error message is is atrocious, but it's what we get from python
programs when the authors can't be bothered to give a rat's ass on
usability.  Not your fault.

Either default the input file to standard input, so that -i is optional,
or make the input file a required non-option argument rather than an
option.  I'd prefer the latter.

Hmm, -o isn't optional, either.  And extra non-option arguments aren't
caught.  Okay, I declare this thing crap.  Your patch doesn't make it
crappier than it already is, so I'm retracting my request.

> @@ -381,7 +384,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 65f1a54..856f969 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -383,13 +383,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'
> @@ -402,6 +403,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"):
> @@ -480,7 +483,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 9b3de4c..59c2b9b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -12,6 +12,7 @@
>  # See the COPYING.LIB file in the top-level directory.
>  
>  from ordereddict import OrderedDict
> +import os
>  import sys
>  
>  builtin_types = [
> @@ -37,6 +38,7 @@ builtin_type_qtypes = {
>  
>  class QAPISchemaError(Exception):
>      def __init__(self, schema, msg):
> +        self.base = schema.error_base

Non-obvious identifiers.  Took me some reading on to figure out that
this is a directory.

>          self.fp = schema.fp
>          self.msg = msg
>          self.line = self.col = 1
> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
>                  self.col += 1
>  
>      def __str__(self):
> -        return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg)
> +        name = os.path.relpath(self.fp.name, self.base)
> +        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)

Can you explain what this change does, and why it's wanted?

>  
>  class QAPISchema:
>  
> -    def __init__(self, fp):
> +    def __init__(self, fp, error_base=None):
>          self.fp = fp
> +        if error_base is None:
> +            self.error_base = os.getcwd()
> +        else:
> +            self.error_base = error_base
>          self.src = fp.read()
>          if self.src == '' or self.src[-1] != '\n':
>              self.src += '\n'
> @@ -158,9 +165,9 @@ class QAPISchema:
>              raise QAPISchemaError(self, 'Expected "{", "[" or string')
>          return expr
>  
> -def parse_schema(fp):
> +def parse_schema(input_path, error_base=None):

The only caller that passes the optional argument is
tests/qapi-schema/test-qapi.py.  Is it really necessary?

>      try:
> -        schema = QAPISchema(fp)
> +        schema = QAPISchema(open(input_path, "r"), error_base)
>      except QAPISchemaError, e:
>          print >>sys.stderr, e
>          exit(1)
> diff --git a/tests/Makefile b/tests/Makefile
> index b17d41e..02b0dbc 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -192,13 +192,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) -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   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
> +		$(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   $@")
> +	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py \
> +		$(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
>  tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
> @@ -331,7 +337,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/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 b3d1e1d..f97e886 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], os.path.dirname(sys.argv[1]))
>  except SystemExit:
>      raise
>  except:

This is the caller that passes the optional argument.

> 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 "'"

Error messages improved :)

Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Lluís Vilanova]

Markus Armbruster writes:

> Lluís Vilanova <vilanova@ac.upc.edu> writes:
[...]
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 9b3de4c..59c2b9b 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -12,6 +12,7 @@
>> # See the COPYING.LIB file in the top-level directory.
>> 
>> from ordereddict import OrderedDict
>> +import os
>> import sys
>> 
>> builtin_types = [
>> @@ -37,6 +38,7 @@ builtin_type_qtypes = {
>> 
>> class QAPISchemaError(Exception):
>> def __init__(self, schema, msg):
>> +        self.base = schema.error_base

> Non-obvious identifiers.  Took me some reading on to figure out that
> this is a directory.

Will fix.


>> self.fp = schema.fp
>> self.msg = msg
>> self.line = self.col = 1
>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
>> self.col += 1
>> 
>> def __str__(self):
>> -        return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg)
>> +        name = os.path.relpath(self.fp.name, self.base)
>> +        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)

> Can you explain what this change does, and why it's wanted?

Paths are shown as relative so that the test outputs (stderr) can be verified
with diff. Otherwise the actual message depends on the path from which you're
running the tests.


>> 
>> class QAPISchema:
>> 
>> -    def __init__(self, fp):
>> +    def __init__(self, fp, error_base=None):
>> self.fp = fp
>> +        if error_base is None:
>> +            self.error_base = os.getcwd()
>> +        else:
>> +            self.error_base = error_base
>> self.src = fp.read()
>> if self.src == '' or self.src[-1] != '\n':
>> self.src += '\n'
>> @@ -158,9 +165,9 @@ class QAPISchema:
>> raise QAPISchemaError(self, 'Expected "{", "[" or string')
>> return expr
>> 
>> -def parse_schema(fp):
>> +def parse_schema(input_path, error_base=None):

> The only caller that passes the optional argument is
> tests/qapi-schema/test-qapi.py.  Is it really necessary?

It's part of the previous answer; that's also true for your next comment (which
I elided). When running tests, paths are shown as relative.


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

Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Markus Armbruster]

Lluís Vilanova <vilanova@ac.upc.edu> writes:

> Markus Armbruster writes:
>
>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
> [...]
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index 9b3de4c..59c2b9b 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -12,6 +12,7 @@
>>> # See the COPYING.LIB file in the top-level directory.
>>> 
>>> from ordereddict import OrderedDict
>>> +import os
>>> import sys
>>> 
>>> builtin_types = [
>>> @@ -37,6 +38,7 @@ builtin_type_qtypes = {
>>> 
>>> class QAPISchemaError(Exception):
>>> def __init__(self, schema, msg):
>>> +        self.base = schema.error_base
>
>> Non-obvious identifiers.  Took me some reading on to figure out that
>> this is a directory.
>
> Will fix.
>
>
>>> self.fp = schema.fp
>>> self.msg = msg
>>> self.line = self.col = 1
>>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
>>> self.col += 1
>>> 
>>> def __str__(self):
>>> - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col,
>>> self.msg)
>>> +        name = os.path.relpath(self.fp.name, self.base)
>>> +        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)
>
>> Can you explain what this change does, and why it's wanted?
>
> Paths are shown as relative so that the test outputs (stderr) can be verified
> with diff. Otherwise the actual message depends on the path from which you're
> running the tests.

Hmm.  This is the applicable make rule:

$(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")
	@diff -q $(SRC_PATH)/$*.out $*.test.out
	@diff -q $(SRC_PATH)/$*.err $*.test.err
	@diff -q $(SRC_PATH)/$*.exit $*.test.exit

Since $^ is in $(SRC_PATH), it's like $(SRC_PATH)/foo.json.  If
$(SRC_PATH)/foo.json has an error, the error messages duly points to
$(SRC_PATH)/foo.json.

The "diff -q $(SRC_PATH)/$*.err $*.test.err" fails unless your SRC_PATH
matches the one that's encoded in tests/qapi-schema/foo.err.  Is that
the problem you're trying to solve?

[...]

Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Lluís Vilanova]

Markus Armbruster writes:
[...]
>>>> self.fp = schema.fp
>>>> self.msg = msg
>>>> self.line = self.col = 1
>>>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
>>>> self.col += 1
>>>> 
>>>> def __str__(self):
>>>> - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col,
>>>> self.msg)
>>>> +        name = os.path.relpath(self.fp.name, self.base)
>>>> +        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)
>> 
>>> Can you explain what this change does, and why it's wanted?
>> 
>> Paths are shown as relative so that the test outputs (stderr) can be verified
>> with diff. Otherwise the actual message depends on the path from which you're
>> running the tests.

> Hmm.  This is the applicable make rule:

> $(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")
> 	@diff -q $(SRC_PATH)/$*.out $*.test.out
> 	@diff -q $(SRC_PATH)/$*.err $*.test.err
> 	@diff -q $(SRC_PATH)/$*.exit $*.test.exit

> Since $^ is in $(SRC_PATH), it's like $(SRC_PATH)/foo.json.  If
> $(SRC_PATH)/foo.json has an error, the error messages duly points to
> $(SRC_PATH)/foo.json.

> The "diff -q $(SRC_PATH)/$*.err $*.test.err" fails unless your SRC_PATH
> matches the one that's encoded in tests/qapi-schema/foo.err.  Is that
> the problem you're trying to solve?

Right. Paths are internally stored as absolute in "qapi.py" (to check for
include cycles), and the "base directory" is only used to show them as
relative. I can try to change the code to retain this functionality but without
special-casing the tests.


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

Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Markus Armbruster]

Lluís Vilanova <vilanova@ac.upc.edu> writes:

> Markus Armbruster writes:
> [...]
>>>>> self.fp = schema.fp
>>>>> self.msg = msg
>>>>> self.line = self.col = 1
>>>>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
>>>>> self.col += 1
>>>>> 
>>>>> def __str__(self):
>>>>> - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col,
>>>>> self.msg)
>>>>> +        name = os.path.relpath(self.fp.name, self.base)
>>>>> +        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)
>>> 
>>>> Can you explain what this change does, and why it's wanted?
>>> 
>>> Paths are shown as relative so that the test outputs (stderr) can be verified
>>> with diff. Otherwise the actual message depends on the path from which you're
>>> running the tests.
>
>> Hmm.  This is the applicable make rule:
>
>> $(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")
>> 	@diff -q $(SRC_PATH)/$*.out $*.test.out
>> 	@diff -q $(SRC_PATH)/$*.err $*.test.err
>> 	@diff -q $(SRC_PATH)/$*.exit $*.test.exit
>
>> Since $^ is in $(SRC_PATH), it's like $(SRC_PATH)/foo.json.  If
>> $(SRC_PATH)/foo.json has an error, the error messages duly points to
>> $(SRC_PATH)/foo.json.
>
>> The "diff -q $(SRC_PATH)/$*.err $*.test.err" fails unless your SRC_PATH
>> matches the one that's encoded in tests/qapi-schema/foo.err.  Is that
>> the problem you're trying to solve?
>
> Right. Paths are internally stored as absolute in "qapi.py" (to check for
> include cycles), and the "base directory" is only used to show them as
> relative. I can try to change the code to retain this functionality but without
> special-casing the tests.

Well, my fav method to check for include cycles is really simple:

1. Estimate how many levels of inclusion you're going to need.

2. Shift left a couple of times.

3. Error out when inclusion depth hits that limit.

Obviously 100% reliable.  File name comparisons tend to be unreliable or
unobvious :)

With this realpath business out of the way, file names in tests should
all be of the form $(SRC_PATH)/tests/qapi-schema/*.json, shouldn't they?

The $(SRC_PATH) prefix depends on where the user's build tree is, the
rest is fixed.  We could strip the prefix from error messages with a
simple filter:

    perl -pe 's,\Q$(SRC_PATH)/tests/qapi-schema/\E,tests/qapi-schema/,'

Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file

[Lluís Vilanova]

Markus Armbruster writes:

> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Markus Armbruster writes:
>> [...]
>>>>>> self.fp = schema.fp
>>>>>> self.msg = msg
>>>>>> self.line = self.col = 1
>>>>>> @@ -50,12 +52,17 @@ class QAPISchemaError(Exception):
>>>>>> self.col += 1
>>>>>> 
>>>>>> def __str__(self):
>>>>>> - return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col,
>>>>>> self.msg)
>>>>>> +        name = os.path.relpath(self.fp.name, self.base)
>>>>>> +        return "%s:%s:%s: %s" % (name, self.line, self.col, self.msg)
>>>> 
>>>>> Can you explain what this change does, and why it's wanted?
>>>> 
>>>> Paths are shown as relative so that the test outputs (stderr) can be verified
>>>> with diff. Otherwise the actual message depends on the path from which you're
>>>> running the tests.
>> 
>>> Hmm.  This is the applicable make rule:
>> 
>>> $(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")
>>> @diff -q $(SRC_PATH)/$*.out $*.test.out
>>> @diff -q $(SRC_PATH)/$*.err $*.test.err
>>> @diff -q $(SRC_PATH)/$*.exit $*.test.exit
>> 
>>> Since $^ is in $(SRC_PATH), it's like $(SRC_PATH)/foo.json.  If
>>> $(SRC_PATH)/foo.json has an error, the error messages duly points to
>>> $(SRC_PATH)/foo.json.
>> 
>>> The "diff -q $(SRC_PATH)/$*.err $*.test.err" fails unless your SRC_PATH
>>> matches the one that's encoded in tests/qapi-schema/foo.err.  Is that
>>> the problem you're trying to solve?
>> 
>> Right. Paths are internally stored as absolute in "qapi.py" (to check for
>> include cycles), and the "base directory" is only used to show them as
>> relative. I can try to change the code to retain this functionality but without
>> special-casing the tests.

> Well, my fav method to check for include cycles is really simple:

> 1. Estimate how many levels of inclusion you're going to need.

> 2. Shift left a couple of times.

> 3. Error out when inclusion depth hits that limit.

> Obviously 100% reliable.  File name comparisons tend to be unreliable or
> unobvious :)

Yes, I just wanted to error-out in a more helpful way. I really hate to have
some arbitrary limit and an unhelpful (or unnecessarily long) message.


> With this realpath business out of the way, file names in tests should
> all be of the form $(SRC_PATH)/tests/qapi-schema/*.json, shouldn't they?

> The $(SRC_PATH) prefix depends on where the user's build tree is, the
> rest is fixed.  We could strip the prefix from error messages with a
> simple filter:

>     perl -pe 's,\Q$(SRC_PATH)/tests/qapi-schema/\E,tests/qapi-schema/,'

Right, post-processing is the other option, silly me.


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

[Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Lluís Vilanova]

Adds the "include(...)" primitive to the syntax of QAPI schema files.

Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
 docs/qapi-code-gen.txt |    8 ++++++++
 scripts/qapi.py        |   36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 2e9f036..e007807 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -40,6 +40,14 @@ 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.
 
+The QAPI schema definitions can be modularized using the 'include' directive:
+
+ include("sub-system/qapi.json")
+
+All paths are interpreted as relative to the initial input file passed to the
+QAPI parsing scripts.
+
+
 === 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 59c2b9b..eddbf25 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -11,10 +11,14 @@
 # This work is licensed under the terms of the GNU GPLv2.
 # See the COPYING.LIB file in the top-level directory.
 
+import os
+import re
 from ordereddict import OrderedDict
 import os
 import sys
 
+include_cre = re.compile("include\(\"([^\"]*)\"\)")
+
 builtin_types = [
     'str', 'int', 'number', 'bool',
     'int8', 'int16', 'int32', 'int64',
@@ -57,12 +61,16 @@ class QAPISchemaError(Exception):
 
 class QAPISchema:
 
-    def __init__(self, fp, error_base=None):
+    def __init__(self, fp, input_dir, error_base=None, included=[]):
         self.fp = fp
         if error_base is None:
             self.error_base = os.getcwd()
         else:
             self.error_base = error_base
+        input_path = os.path.abspath(fp.name)
+        input_path = os.path.relpath(input_path, self.error_base)
+        self.included = included + [input_path]
+        self.input_dir = input_dir
         self.src = fp.read()
         if self.src == '' or self.src[-1] != '\n':
             self.src += '\n'
@@ -107,6 +115,29 @@ class QAPISchema:
                 if self.cursor == len(self.src):
                     self.tok = None
                     return
+            elif self.tok == 'i':
+                include_src = self.src[self.cursor-1:]
+                include_match = include_cre.match(include_src)
+                if include_match is not None:
+                    include_path = os.path.join(self.input_dir,
+                                                include_match.group(1))
+                    include_path = os.path.abspath(include_path)
+                    if not os.path.isfile(include_path):
+                        raise QAPISchemaError(
+                            self,
+                            'Non-existing included file "%s"' %
+                            include_match.group(1))
+                    include_path_rel = os.path.relpath(include_path,
+                                                       self.error_base)
+                    if include_path_rel in self.included:
+                        raise QAPISchemaError(self, "Infinite inclusion loop: %s"
+                                              % " -> ".join(self.included +
+                                                            [include_path_rel]))
+                    include_schema = QAPISchema(open(include_path, "r"),
+                                                self.input_dir, self.error_base,
+                                                self.included)
+                    self.exprs += include_schema.exprs
+                    self.cursor += include_match.span()[1] - 1
             elif not self.tok.isspace():
                 raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
 
@@ -166,8 +197,9 @@ class QAPISchema:
         return expr
 
 def parse_schema(input_path, error_base=None):
+    input_dir = os.path.dirname(input_path)
     try:
-        schema = QAPISchema(open(input_path, "r"), error_base)
+        schema = QAPISchema(open(input_path, "r"), input_dir, error_base)
     except QAPISchemaError, e:
         print >>sys.stderr, e
         exit(1)

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Markus Armbruster]

Lluís Vilanova <vilanova@ac.upc.edu> writes:

> Adds the "include(...)" primitive to the syntax of QAPI schema files.
>
> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
> ---
>  docs/qapi-code-gen.txt |    8 ++++++++
>  scripts/qapi.py        |   36 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 2e9f036..e007807 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -40,6 +40,14 @@ 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.
>  
> +The QAPI schema definitions can be modularized using the 'include' directive:
> +
> + include("sub-system/qapi.json")

And now it isn't JSON anymore.

To keep it JSON, use syntax like

    { "include": "sub-system/qapi.json" }

If you absolutely must make it non-JSON, you better rename the .json
files.

Hmm, we already are non-JSON, because we use ' instead of " for no sane
reason.

Our JSON parser accepts ' as an extension, to save us quoting in C
strings.  That reason doesn't apply to .json files.

> +
> +All paths are interpreted as relative to the initial input file passed to the
> +QAPI parsing scripts.

Really?

Consider foo.json includes lib/a.json, which wants to include
lib/b.json.

foo.json:       include("lib/a.json")
lib/a.json:     include("lib/b.json")   # relative to foo.json's directory

Now throw in bar/bar.json including lib/a.json:

bar/bar.json:   include("../lib/a.json")
lib/a.json:     include("lib/b.json")   # relative to bar/ -> ENOENT

Make it relative to the file with the include directive.

> +
> +
>  === Complex types ===
>  
>  A complex type is a dictionary containing a single key whose value is a
[...]

Are you aware of Wenchao Xia's "[PATCH V8 00/10] qapi script: support
enum as discriminator and better enum name"?  I'm afraid there's a
(semantic?) conflict.  With include files, "[PATCH V8 03/10] qapi
script: remember line number in schema parsing" needs to remember the
source file, too.

Wenchao's series is likely go in first.  Perhaps you want to base on it
now.

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Lluís Vilanova]

Markus Armbruster writes:

> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Adds the "include(...)" primitive to the syntax of QAPI schema files.
>> 
>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>> ---
>> docs/qapi-code-gen.txt |    8 ++++++++
>> scripts/qapi.py        |   36 ++++++++++++++++++++++++++++++++++--
>> 2 files changed, 42 insertions(+), 2 deletions(-)
>> 
>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>> index 2e9f036..e007807 100644
>> --- a/docs/qapi-code-gen.txt
>> +++ b/docs/qapi-code-gen.txt
>> @@ -40,6 +40,14 @@ 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.
>> 
>> +The QAPI schema definitions can be modularized using the 'include' directive:
>> +
>> + include("sub-system/qapi.json")

> And now it isn't JSON anymore.

> To keep it JSON, use syntax like

>     { "include": "sub-system/qapi.json" }

> If you absolutely must make it non-JSON, you better rename the .json
> files.

> Hmm, we already are non-JSON, because we use ' instead of " for no sane
> reason.

> Our JSON parser accepts ' as an extension, to save us quoting in C
> strings.  That reason doesn't apply to .json files.

Is it a problem if they are not pure JSON? In the end, they are parsed by
qapi.py (which already knows about file syntax), and having a separate syntax
for includes makes it somewhat easier to spot when that happens.


>> +
>> +All paths are interpreted as relative to the initial input file passed to the
>> +QAPI parsing scripts.

> Really?

> Consider foo.json includes lib/a.json, which wants to include
> lib/b.json.

> foo.json:       include("lib/a.json")
> lib/a.json:     include("lib/b.json")   # relative to foo.json's directory

> Now throw in bar/bar.json including lib/a.json:

> bar/bar.json:   include("../lib/a.json")
> lib/a.json:     include("lib/b.json")   # relative to bar/ -> ENOENT

> Make it relative to the file with the include directive.

Right, sorry. The documentation was not updated after removing the explicit
include directory argument. What I'm not sure though is what would be a better
option, having current-file-relative includes, or resuscitating the old explicit
include argument.


>> +
>> +
>> === Complex types ===
>> 
>> A complex type is a dictionary containing a single key whose value is a
> [...]

> Are you aware of Wenchao Xia's "[PATCH V8 00/10] qapi script: support
> enum as discriminator and better enum name"?  I'm afraid there's a
> (semantic?) conflict.  With include files, "[PATCH V8 03/10] qapi
> script: remember line number in schema parsing" needs to remember the
> source file, too.

> Wenchao's series is likely go in first.  Perhaps you want to base on it
> now.

I was not aware of that. Since I'm managing multiple series on a single branch
with stgit (and extracting that is somewhat tedious), I will redo this series
once Xia's is merged upstream. Is the merge going to happen anytime soon?


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

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Markus Armbruster]

Lluís Vilanova <vilanova@ac.upc.edu> writes:

> Markus Armbruster writes:
>
>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>>> Adds the "include(...)" primitive to the syntax of QAPI schema files.
>>> 
>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>> ---
>>> docs/qapi-code-gen.txt |    8 ++++++++
>>> scripts/qapi.py        |   36 ++++++++++++++++++++++++++++++++++--
>>> 2 files changed, 42 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>>> index 2e9f036..e007807 100644
>>> --- a/docs/qapi-code-gen.txt
>>> +++ b/docs/qapi-code-gen.txt
>>> @@ -40,6 +40,14 @@ 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.
>>> 
>>> +The QAPI schema definitions can be modularized using the 'include'
>>> directive:
>>> +
>>> + include("sub-system/qapi.json")
>
>> And now it isn't JSON anymore.
>
>> To keep it JSON, use syntax like
>
>>     { "include": "sub-system/qapi.json" }
>
>> If you absolutely must make it non-JSON, you better rename the .json
>> files.
>
>> Hmm, we already are non-JSON, because we use ' instead of " for no sane
>> reason.
>
>> Our JSON parser accepts ' as an extension, to save us quoting in C
>> strings.  That reason doesn't apply to .json files.
>
> Is it a problem if they are not pure JSON? In the end, they are parsed by
> qapi.py (which already knows about file syntax), and having a separate syntax
> for includes makes it somewhat easier to spot when that happens.

I don't particularly care whether schema syntax is pure JSON, some
bastardized variation of JSON, or something else entirely.  But as long
as we advertize schema files it as .json, they better contain JSON.  If
they contain something else, they should be called something else.

>>> +
>>> +All paths are interpreted as relative to the initial input file
>>> passed to the
>>> +QAPI parsing scripts.
>
>> Really?
>
>> Consider foo.json includes lib/a.json, which wants to include
>> lib/b.json.
>
>> foo.json:       include("lib/a.json")
>> lib/a.json:     include("lib/b.json")   # relative to foo.json's directory
>
>> Now throw in bar/bar.json including lib/a.json:
>
>> bar/bar.json:   include("../lib/a.json")
>> lib/a.json:     include("lib/b.json")   # relative to bar/ -> ENOENT
>
>> Make it relative to the file with the include directive.
>
> Right, sorry. The documentation was not updated after removing the explicit
> include directory argument. What I'm not sure though is what would be a better
> option, having current-file-relative includes, or resuscitating the old explicit
> include argument.

Include relative to the including file is simple.  If we needed
not-so-simple semantics, I suspect we'd be better off piping through
cpp.

>>> +
>>> +
>>> === Complex types ===
>>> 
>>> A complex type is a dictionary containing a single key whose value is a
>> [...]
>
>> Are you aware of Wenchao Xia's "[PATCH V8 00/10] qapi script: support
>> enum as discriminator and better enum name"?  I'm afraid there's a
>> (semantic?) conflict.  With include files, "[PATCH V8 03/10] qapi
>> script: remember line number in schema parsing" needs to remember the
>> source file, too.
>
>> Wenchao's series is likely go in first.  Perhaps you want to base on it
>> now.
>
> I was not aware of that. Since I'm managing multiple series on a single branch
> with stgit (and extracting that is somewhat tedious), I will redo this series
> once Xia's is merged upstream. Is the merge going to happen anytime soon?

I reviewed v7, and it looked almost ready to me.  If v8 is ready, it'll
hopefully get merged fairly quickly (days, not weeks).

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Lluís Vilanova]

Markus Armbruster writes:

> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>> Markus Armbruster writes:
>> 
>>> Lluís Vilanova <vilanova@ac.upc.edu> writes:
>>>> Adds the "include(...)" primitive to the syntax of QAPI schema files.
>>>> 
>>>> Signed-off-by: Lluís Vilanova <vilanova@ac.upc.edu>
>>>> ---
>>>> docs/qapi-code-gen.txt |    8 ++++++++
>>>> scripts/qapi.py        |   36 ++++++++++++++++++++++++++++++++++--
>>>> 2 files changed, 42 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>>>> index 2e9f036..e007807 100644
>>>> --- a/docs/qapi-code-gen.txt
>>>> +++ b/docs/qapi-code-gen.txt
>>>> @@ -40,6 +40,14 @@ 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.
>>>> 
>>>> +The QAPI schema definitions can be modularized using the 'include'
>>>> directive:
>>>> +
>>>> + include("sub-system/qapi.json")
>> 
>>> And now it isn't JSON anymore.
>> 
>>> To keep it JSON, use syntax like
>> 
>>> { "include": "sub-system/qapi.json" }
>> 
>>> If you absolutely must make it non-JSON, you better rename the .json
>>> files.
>> 
>>> Hmm, we already are non-JSON, because we use ' instead of " for no sane
>>> reason.
>> 
>>> Our JSON parser accepts ' as an extension, to save us quoting in C
>>> strings.  That reason doesn't apply to .json files.
>> 
>> Is it a problem if they are not pure JSON? In the end, they are parsed by
>> qapi.py (which already knows about file syntax), and having a separate syntax
>> for includes makes it somewhat easier to spot when that happens.

> I don't particularly care whether schema syntax is pure JSON, some
> bastardized variation of JSON, or something else entirely.  But as long
> as we advertize schema files it as .json, they better contain JSON.  If
> they contain something else, they should be called something else.

Both are simple enough to implement. Is there any consensus on what's preferred?


>>>> +
>>>> +All paths are interpreted as relative to the initial input file
>>>> passed to the
>>>> +QAPI parsing scripts.
>> 
>>> Really?
>> 
>>> Consider foo.json includes lib/a.json, which wants to include
>>> lib/b.json.
>> 
>>> foo.json:       include("lib/a.json")
>>> lib/a.json:     include("lib/b.json")   # relative to foo.json's directory
>> 
>>> Now throw in bar/bar.json including lib/a.json:
>> 
>>> bar/bar.json:   include("../lib/a.json")
>>> lib/a.json:     include("lib/b.json")   # relative to bar/ -> ENOENT
>> 
>>> Make it relative to the file with the include directive.
>> 
>> Right, sorry. The documentation was not updated after removing the explicit
>> include directory argument. What I'm not sure though is what would be a better
>> option, having current-file-relative includes, or resuscitating the old explicit
>> include argument.

> Include relative to the including file is simple.  If we needed
> not-so-simple semantics, I suspect we'd be better off piping through
> cpp.

I tried, but it chokes on the comment syntax.


>>>> +
>>>> +
>>>> === Complex types ===
>>>> 
>>>> A complex type is a dictionary containing a single key whose value is a
>>> [...]
>> 
>>> Are you aware of Wenchao Xia's "[PATCH V8 00/10] qapi script: support
>>> enum as discriminator and better enum name"?  I'm afraid there's a
>>> (semantic?) conflict.  With include files, "[PATCH V8 03/10] qapi
>>> script: remember line number in schema parsing" needs to remember the
>>> source file, too.
>> 
>>> Wenchao's series is likely go in first.  Perhaps you want to base on it
>>> now.
>> 
>> I was not aware of that. Since I'm managing multiple series on a single branch
>> with stgit (and extracting that is somewhat tedious), I will redo this series
>> once Xia's is merged upstream. Is the merge going to happen anytime soon?

> I reviewed v7, and it looked almost ready to me.  If v8 is ready, it'll
> hopefully get merged fairly quickly (days, not weeks).

Excellent.


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

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]

On 03/03/2014 08:27 AM, Markus Armbruster wrote:

>>>> +The QAPI schema definitions can be modularized using the 'include'
>>>> directive:
>>>> +
>>>> + include("sub-system/qapi.json")
>>
>>> And now it isn't JSON anymore.
>>
>>> To keep it JSON, use syntax like
>>
>>>     { "include": "sub-system/qapi.json" }

I actually think this looks nicer - makes the file more consistent.

>>
>>> If you absolutely must make it non-JSON, you better rename the .json
>>> files.
>>
>>> Hmm, we already are non-JSON, because we use ' instead of " for no sane
>>> reason.

A weak argument: ' is easier than " to type (at least on US keyboards -
no shift key required).

Another weak argument: using ' in the qapi files vs. " in actual QMP
makes it easy to interleave discussions about semantics vs. examples of
those semantics in use (you can see whether a code snippet is talking
about qapi or wire format based on what quoting it used)

Our files are already non-JSON due to comments (JSON has no notion of #
introducing a comment to ignore text to the next newline).  But both our
use of comments and our use of ' instead of " can be remedied in a
one-pass sed script to get a true JSON output if such is needed, at
least as long as we don't need to quote any " characters in the schema.

Therefore, I agree that making the include syntax closer to true JSON is
desirable, whether or not we also decide to use " in the files to begin
with.  I don't see any way around the fact that JSON doesn't define
comments, vs. our absolute need for comments in our schema files, though.

>>
>>> Our JSON parser accepts ' as an extension, to save us quoting in C
>>> strings.  That reason doesn't apply to .json files.
>>
>> Is it a problem if they are not pure JSON? In the end, they are parsed by
>> qapi.py (which already knows about file syntax), and having a separate syntax
>> for includes makes it somewhat easier to spot when that happens.
> 
> I don't particularly care whether schema syntax is pure JSON, some
> bastardized variation of JSON, or something else entirely.  But as long
> as we advertize schema files it as .json, they better contain JSON.  If
> they contain something else, they should be called something else.

Maybe .qapi? But the name qapi-schema.qapi sounds redundant...

-- 
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 --]

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 03/03/2014 08:27 AM, Markus Armbruster wrote:
>
>>>>> +The QAPI schema definitions can be modularized using the 'include'
>>>>> directive:
>>>>> +
>>>>> + include("sub-system/qapi.json")
>>>
>>>> And now it isn't JSON anymore.
>>>
>>>> To keep it JSON, use syntax like
>>>
>>>>     { "include": "sub-system/qapi.json" }
>
> I actually think this looks nicer - makes the file more consistent.

I suspect qapi.py would look nicer, too :)

>>>> If you absolutely must make it non-JSON, you better rename the .json
>>>> files.
>>>
>>>> Hmm, we already are non-JSON, because we use ' instead of " for no sane
>>>> reason.
>
> A weak argument: ' is easier than " to type (at least on US keyboards -
> no shift key required).
>
> Another weak argument: using ' in the qapi files vs. " in actual QMP
> makes it easy to interleave discussions about semantics vs. examples of
> those semantics in use (you can see whether a code snippet is talking
> about qapi or wire format based on what quoting it used)
>
> Our files are already non-JSON due to comments (JSON has no notion of #
> introducing a comment to ignore text to the next newline).  But both our
> use of comments and our use of ' instead of " can be remedied in a
> one-pass sed script to get a true JSON output if such is needed, at
> least as long as we don't need to quote any " characters in the schema.
>
> Therefore, I agree that making the include syntax closer to true JSON is
> desirable, whether or not we also decide to use " in the files to begin
> with.  I don't see any way around the fact that JSON doesn't define
> comments, vs. our absolute need for comments in our schema files, though.

We certainly can't do without comments.

JSON is designed for easy data exchange, but we use it as programming
language syntax.  Its restrictions make sense for easy data exchange,
but hurt our use.  We're not the first ones experiencing that pain:
http://json5.org/

No idea how much momentum this JSON5 thingy has...

>>>> Our JSON parser accepts ' as an extension, to save us quoting in C
>>>> strings.  That reason doesn't apply to .json files.
>>>
>>> Is it a problem if they are not pure JSON? In the end, they are parsed by
>>> qapi.py (which already knows about file syntax), and having a separate syntax
>>> for includes makes it somewhat easier to spot when that happens.
>> 
>> I don't particularly care whether schema syntax is pure JSON, some
>> bastardized variation of JSON, or something else entirely.  But as long
>> as we advertize schema files it as .json, they better contain JSON.  If
>> they contain something else, they should be called something else.
>
> Maybe .qapi? But the name qapi-schema.qapi sounds redundant...

schema.qapi?

Switch to JSON5 and call it qapi-schema.json5?

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Benoît Canet]

The Tuesday 04 Mar 2014 à 09:02:57 (+0100), Markus Armbruster wrote :
> Eric Blake <eblake@redhat.com> writes:
> 
> > On 03/03/2014 08:27 AM, Markus Armbruster wrote:
> >
> >>>>> +The QAPI schema definitions can be modularized using the 'include'
> >>>>> directive:
> >>>>> +
> >>>>> + include("sub-system/qapi.json")
> >>>
> >>>> And now it isn't JSON anymore.
> >>>
> >>>> To keep it JSON, use syntax like
> >>>
> >>>>     { "include": "sub-system/qapi.json" }
> >
> > I actually think this looks nicer - makes the file more consistent.
> 
> I suspect qapi.py would look nicer, too :)
> 
> >>>> If you absolutely must make it non-JSON, you better rename the .json
> >>>> files.
> >>>
> >>>> Hmm, we already are non-JSON, because we use ' instead of " for no sane
> >>>> reason.
> >
> > A weak argument: ' is easier than " to type (at least on US keyboards -
> > no shift key required).
> >
> > Another weak argument: using ' in the qapi files vs. " in actual QMP
> > makes it easy to interleave discussions about semantics vs. examples of
> > those semantics in use (you can see whether a code snippet is talking
> > about qapi or wire format based on what quoting it used)
> >
> > Our files are already non-JSON due to comments (JSON has no notion of #
> > introducing a comment to ignore text to the next newline).  But both our
> > use of comments and our use of ' instead of " can be remedied in a
> > one-pass sed script to get a true JSON output if such is needed, at
> > least as long as we don't need to quote any " characters in the schema.
> >
> > Therefore, I agree that making the include syntax closer to true JSON is
> > desirable, whether or not we also decide to use " in the files to begin
> > with.  I don't see any way around the fact that JSON doesn't define
> > comments, vs. our absolute need for comments in our schema files, though.
> 
> We certainly can't do without comments.
> 
> JSON is designed for easy data exchange, but we use it as programming
> language syntax.  Its restrictions make sense for easy data exchange,
> but hurt our use.  We're not the first ones experiencing that pain:
> http://json5.org/
> 
> No idea how much momentum this JSON5 thingy has...
> 
> >>>> Our JSON parser accepts ' as an extension, to save us quoting in C
> >>>> strings.  That reason doesn't apply to .json files.
> >>>
> >>> Is it a problem if they are not pure JSON? In the end, they are parsed by
> >>> qapi.py (which already knows about file syntax), and having a separate syntax
> >>> for includes makes it somewhat easier to spot when that happens.
> >> 
> >> I don't particularly care whether schema syntax is pure JSON, some
> >> bastardized variation of JSON, or something else entirely.  But as long
> >> as we advertize schema files it as .json, they better contain JSON.  If
> >> they contain something else, they should be called something else.
> >
> > Maybe .qapi? But the name qapi-schema.qapi sounds redundant...
> 
> schema.qapi?
> 
> Switch to JSON5 and call it qapi-schema.json5?
> 

Hmm don't we want something that python and other language know how to parse out
of the box ? Or will we write yet another delicate work of art to parse it ?

Best regards

Benoît

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1448 bytes --]

On 03/13/2014 09:33 AM, Benoît Canet wrote:

>> We certainly can't do without comments.
>>
>> JSON is designed for easy data exchange, but we use it as programming
>> language syntax.  Its restrictions make sense for easy data exchange,
>> but hurt our use.  We're not the first ones experiencing that pain:
>> http://json5.org/
>>
>> No idea how much momentum this JSON5 thingy has...

If we 's,#,//,', our comments magically fall in line with JSON5 syntax;
everything else in our files is already compliant with JSON5.

>>
>> Switch to JSON5 and call it qapi-schema.json5?

This actually seems like a rather nice idea - but due to our choice of
comments, it means rewriting the bulk of the file and tweaking our parser.

>>
> 
> Hmm don't we want something that python and other language know how to parse out
> of the box ? Or will we write yet another delicate work of art to parse it ?

Our existing parser would only need to learn a new comment syntax to
parse the subset of JSON5 that we currently actually use.  Parsing FULL
JSON5 would mean also learning about trailing commas, unquoted names in
name:value pairs, multiline strings, and alternative numeric
representations.  But a point made on the JSON5 page is that ES5
JavaScript already parses JSON5, just as it already parses original JSON.

-- 
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 --]

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Lluís Vilanova]

Eric Blake writes:

> On 03/13/2014 09:33 AM, Benoît Canet wrote:
>>> We certainly can't do without comments.
>>> 
>>> JSON is designed for easy data exchange, but we use it as programming
>>> language syntax.  Its restrictions make sense for easy data exchange,
>>> but hurt our use.  We're not the first ones experiencing that pain:
>>> http://json5.org/
>>> 
>>> No idea how much momentum this JSON5 thingy has...

> If we 's,#,//,', our comments magically fall in line with JSON5 syntax;
> everything else in our files is already compliant with JSON5.

>>> 
>>> Switch to JSON5 and call it qapi-schema.json5?

> This actually seems like a rather nice idea - but due to our choice of
> comments, it means rewriting the bulk of the file and tweaking our parser.

>>> 
>> 
>> Hmm don't we want something that python and other language know how to parse out
>> of the box ? Or will we write yet another delicate work of art to parse it ?

> Our existing parser would only need to learn a new comment syntax to
> parse the subset of JSON5 that we currently actually use.  Parsing FULL
> JSON5 would mean also learning about trailing commas, unquoted names in
> name:value pairs, multiline strings, and alternative numeric
> representations.  But a point made on the JSON5 page is that ES5
> JavaScript already parses JSON5, just as it already parses original JSON.

Another option is to bump QEMU requirements to python 2.6 or later. Then we can
use the json parser that comes with python. A simple pre-processing could
eliminate the comments before passing them to the json package for loading into
python structures. The commands/enums/etc should also be elements of a list for
it to work (that must be either changed on the qapi files, or hackishly
"injected" before parsing).


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

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Benoît Canet]

The Thursday 13 Mar 2014 à 19:05:12 (+0100), Lluís Vilanova wrote :
> Eric Blake writes:
> 
> > On 03/13/2014 09:33 AM, Benoît Canet wrote:
> >>> We certainly can't do without comments.
> >>> 
> >>> JSON is designed for easy data exchange, but we use it as programming
> >>> language syntax.  Its restrictions make sense for easy data exchange,
> >>> but hurt our use.  We're not the first ones experiencing that pain:
> >>> http://json5.org/
> >>> 
> >>> No idea how much momentum this JSON5 thingy has...
> 
> > If we 's,#,//,', our comments magically fall in line with JSON5 syntax;
> > everything else in our files is already compliant with JSON5.
> 
> >>> 
> >>> Switch to JSON5 and call it qapi-schema.json5?
> 
> > This actually seems like a rather nice idea - but due to our choice of
> > comments, it means rewriting the bulk of the file and tweaking our parser.
> 
> >>> 
> >> 
> >> Hmm don't we want something that python and other language know how to parse out
> >> of the box ? Or will we write yet another delicate work of art to parse it ?
> 
> > Our existing parser would only need to learn a new comment syntax to
> > parse the subset of JSON5 that we currently actually use.  Parsing FULL
> > JSON5 would mean also learning about trailing commas, unquoted names in
> > name:value pairs, multiline strings, and alternative numeric
> > representations.  But a point made on the JSON5 page is that ES5
> > JavaScript already parses JSON5, just as it already parses original JSON.
> 
> Another option is to bump QEMU requirements to python 2.6 or later. Then we can
> use the json parser that comes with python. A simple pre-processing could
> eliminate the comments before passing them to the json package for loading into
> python structures. The commands/enums/etc should also be elements of a list for
> it to work (that must be either changed on the qapi files, or hackishly
> "injected" before parsing).
> 
> 
> Lluis

I have an use case for this series.

Lluis: Do you plan to respin this series ? Or should I do it ?

Best regards

Benoît

> 
> -- 
>  "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
> 

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Lluís Vilanova]

Benoît Canet writes:

> The Thursday 13 Mar 2014 à 19:05:12 (+0100), Lluís Vilanova wrote :
>> Eric Blake writes:
>> 
>> > On 03/13/2014 09:33 AM, Benoît Canet wrote:
>> >>> We certainly can't do without comments.
>> >>> 
>> >>> JSON is designed for easy data exchange, but we use it as programming
>> >>> language syntax.  Its restrictions make sense for easy data exchange,
>> >>> but hurt our use.  We're not the first ones experiencing that pain:
>> >>> http://json5.org/
>> >>> 
>> >>> No idea how much momentum this JSON5 thingy has...
>> 
>> > If we 's,#,//,', our comments magically fall in line with JSON5 syntax;
>> > everything else in our files is already compliant with JSON5.
>> 
>> >>> 
>> >>> Switch to JSON5 and call it qapi-schema.json5?
>> 
>> > This actually seems like a rather nice idea - but due to our choice of
>> > comments, it means rewriting the bulk of the file and tweaking our parser.
>> 
>> >>> 
>> >> 
>> >> Hmm don't we want something that python and other language know how to parse out
>> >> of the box ? Or will we write yet another delicate work of art to parse it ?
>> 
>> > Our existing parser would only need to learn a new comment syntax to
>> > parse the subset of JSON5 that we currently actually use.  Parsing FULL
>> > JSON5 would mean also learning about trailing commas, unquoted names in
>> > name:value pairs, multiline strings, and alternative numeric
>> > representations.  But a point made on the JSON5 page is that ES5
>> > JavaScript already parses JSON5, just as it already parses original JSON.
>> 
>> Another option is to bump QEMU requirements to python 2.6 or later. Then we can
>> use the json parser that comes with python. A simple pre-processing could
>> eliminate the comments before passing them to the json package for loading into
>> python structures. The commands/enums/etc should also be elements of a list for
>> it to work (that must be either changed on the qapi files, or hackishly
>> "injected" before parsing).
>> 
>> 
>> Lluis

> I have an use case for this series.

> Lluis: Do you plan to respin this series ? Or should I do it ?

I was waiting for some other series to get merged, since they conflict. But I
still did not change the "include" syntax.

I will probably not be able to get to this until May 1st.


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

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Benoît Canet]

The Friday 14 Mar 2014 à 21:24:38 (+0100), Lluís Vilanova wrote :
> Benoît Canet writes:
> 
> > The Thursday 13 Mar 2014 à 19:05:12 (+0100), Lluís Vilanova wrote :
> >> Eric Blake writes:
> >> 
> >> > On 03/13/2014 09:33 AM, Benoît Canet wrote:
> >> >>> We certainly can't do without comments.
> >> >>> 
> >> >>> JSON is designed for easy data exchange, but we use it as programming
> >> >>> language syntax.  Its restrictions make sense for easy data exchange,
> >> >>> but hurt our use.  We're not the first ones experiencing that pain:
> >> >>> http://json5.org/
> >> >>> 
> >> >>> No idea how much momentum this JSON5 thingy has...
> >> 
> >> > If we 's,#,//,', our comments magically fall in line with JSON5 syntax;
> >> > everything else in our files is already compliant with JSON5.
> >> 
> >> >>> 
> >> >>> Switch to JSON5 and call it qapi-schema.json5?
> >> 
> >> > This actually seems like a rather nice idea - but due to our choice of
> >> > comments, it means rewriting the bulk of the file and tweaking our parser.
> >> 
> >> >>> 
> >> >> 
> >> >> Hmm don't we want something that python and other language know how to parse out
> >> >> of the box ? Or will we write yet another delicate work of art to parse it ?
> >> 
> >> > Our existing parser would only need to learn a new comment syntax to
> >> > parse the subset of JSON5 that we currently actually use.  Parsing FULL
> >> > JSON5 would mean also learning about trailing commas, unquoted names in
> >> > name:value pairs, multiline strings, and alternative numeric
> >> > representations.  But a point made on the JSON5 page is that ES5
> >> > JavaScript already parses JSON5, just as it already parses original JSON.
> >> 
> >> Another option is to bump QEMU requirements to python 2.6 or later. Then we can
> >> use the json parser that comes with python. A simple pre-processing could
> >> eliminate the comments before passing them to the json package for loading into
> >> python structures. The commands/enums/etc should also be elements of a list for
> >> it to work (that must be either changed on the qapi files, or hackishly
> >> "injected" before parsing).
> >> 
> >> 
> >> Lluis
> 
> > I have an use case for this series.
> 
> > Lluis: Do you plan to respin this series ? Or should I do it ?
> 
> I was waiting for some other series to get merged, since they conflict. But I
> still did not change the "include" syntax.
> 
> I will probably not be able to get to this until May 1st.

Ok, I think I need the include feature more badly than you I will do it.

Best regards

Benoît

> 
> 
> 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

Re: [Qemu-devel] [PATCH v4 2/3] qapi: Add a primitive to include other files from a QAPI schema file

[Benoît Canet]

The Thursday 13 Mar 2014 à 09:54:15 (-0600), Eric Blake wrote :
> On 03/13/2014 09:33 AM, Benoît Canet wrote:
> 
> >> We certainly can't do without comments.
> >>
> >> JSON is designed for easy data exchange, but we use it as programming
> >> language syntax.  Its restrictions make sense for easy data exchange,
> >> but hurt our use.  We're not the first ones experiencing that pain:
> >> http://json5.org/
> >>
> >> No idea how much momentum this JSON5 thingy has...
> 
> If we 's,#,//,', our comments magically fall in line with JSON5 syntax;
> everything else in our files is already compliant with JSON5.

Not really qapi-schema.json is missing comas between types to be a
valid json file.

The fragment:

{ 'type'  : 'InputBtnEvent',
  'data'  : { 'button'  : 'InputButton',
              'down'    : 'bool' } }

{ 'type'  : 'InputMoveEvent',
  'data'  : { 'axis'    : 'InputAxis',
              'value'   : 'int' } }

Should be:

[

{ 'type'  : 'InputBtnEvent',
  'data'  : { 'button'  : 'InputButton',
              'down'    : 'bool' } },

{ 'type'  : 'InputMoveEvent',
  'data'  : { 'axis'    : 'InputAxis',
              'value'   : 'int' } }

]

to hope being a valid json file.

Best regards

Benoît

> 
> >>
> >> Switch to JSON5 and call it qapi-schema.json5?
> 
> This actually seems like a rather nice idea - but due to our choice of
> comments, it means rewriting the bulk of the file and tweaking our parser.
> 
> >>
> > 
> > Hmm don't we want something that python and other language know how to parse out
> > of the box ? Or will we write yet another delicate work of art to parse it ?
> 
> Our existing parser would only need to learn a new comment syntax to
> parse the subset of JSON5 that we currently actually use.  Parsing FULL
> JSON5 would mean also learning about trailing commas, unquoted names in
> name:value pairs, multiline strings, and alternative numeric
> representations.  But a point made on the JSON5 page is that ES5
> JavaScript already parses JSON5, just as it already parses original JSON.
> 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

[Qemu-devel] [PATCH v4 3/3] qapi: Add tests for the "include" directive

[Lluís Vilanova]

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-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 +++
 24 files changed, 24 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-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 02b0dbc..f4aae8c 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -142,7 +142,9 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \
         missing-comma-object.json non-objects.json \
         qapi-schema-test.json quoted-structural-chars.json \
         trailing-comma-list.json trailing-comma-object.json \
-        unclosed-list.json unclosed-object.json unclosed-string.json)
+        unclosed-list.json unclosed-object.json unclosed-string.json \
+        include-simple.json include-nested-err.json include-no-file.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..baf4282
--- /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..90ffa6a
--- /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..48003d3
--- /dev/null
+++ b/tests/qapi-schema/include-cycle.err
@@ -0,0 +1 @@
+include-cycle-c.json:1:1: Infinite inclusion loop: include-cycle.json -> include-cycle-b.json -> include-cycle-c.json -> 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..4f00e7e
--- /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..676cce5
--- /dev/null
+++ b/tests/qapi-schema/include-nested-err.err
@@ -0,0 +1 @@
+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..ff53883
--- /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..24832c1
--- /dev/null
+++ b/tests/qapi-schema/include-no-file.err
@@ -0,0 +1 @@
+include-no-file.json:1:1: Non-existing included file "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..df762c2
--- /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-self-cycle.err b/tests/qapi-schema/include-self-cycle.err
new file mode 100644
index 0000000..c2b216a
--- /dev/null
+++ b/tests/qapi-schema/include-self-cycle.err
@@ -0,0 +1 @@
+include-self-cycle.json:1:1: Infinite inclusion loop: include-self-cycle.json -> 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..1c418c0
--- /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..31c81d1
--- /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..e3bd904
--- /dev/null
+++ b/tests/qapi-schema/include-simple.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+['Status']
+[]

Re: [Qemu-devel] [PATCH v4 0/3] qapi: Allow modularization of QAPI schema files

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On 02/28/2014 08:53 AM, Lluís Vilanova wrote:
> Adds the "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>
> ---

Series: 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 --]