[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file
|
From: |
Markus Armbruster |
|
Subject: |
Re: [Qemu-devel] [PATCH v4 1/3] qapi: Use an explicit input file |
|
Date: |
Sat, 01 Mar 2014 09:35:52 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
Lluís Vilanova <address@hidden> writes:
> Use an explicit input file on the command-line instead of reading from
> standard input
>
> Signed-off-by: Lluís Vilanova <address@hidden>
> ---
> 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:
>
> address@hidden:~/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-"
> address@hidden:~/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:
>
> address@hidden:~/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-"
> address@hidden:~/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,
Markus Armbruster <=