| Message ID | 1399650502-25530-2-git-send-email-benoit.canet@irqsave.net |
|---|---|
| State | New |
| Headers | show |
On 05/09/2014 09:48 AM, Benoît Canet wrote: Subject line has problems. 'excepted' is not a word, and you are already asked to avoid trailing '.'. I suggest a much simpler: qapi: skip redundant includes > The purpose of this change is to help create a json file containing > common definitions; each bit of generated C code must be spitted s/spitted/emitted/ > only one time. > > A second history global to all QAPISchema instances has been added > to detect when a file is included more than one time and skip these > includes. > It does not act as a stack and the changes made to it by the > __init__ function are propagated back to the caller so it's really > a global state. > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/include-only-one-time.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/include-only-one-time.json b/tests/qapi-schema/include-only-one-time.json > new file mode 100644 > index 0000000..11772e3 > --- /dev/null > +++ b/tests/qapi-schema/include-only-one-time.json Long name; you could get by with: tests/qapi-schema/include-repetition.json > @@ -0,0 +1,3 @@ > +{ 'include': 'comments.json' } > +{ 'include': 'sub-include-only-one-time.json' } This naming doesn't preserve the 'include-' prefix used by all other files related to include. Maybe: { 'include': 'include-repetition-sub.json' }
The Monday 12 May 2014 à 11:11:41 (-0600), Eric Blake wrote : > On 05/09/2014 09:48 AM, Benoît Canet wrote: > > Subject line has problems. 'excepted' is not a word, and you are already > asked to avoid trailing '.'. I suggest a much simpler: > > qapi: skip redundant includes > > > The purpose of this change is to help create a json file containing > > common definitions; each bit of generated C code must be spitted > > s/spitted/emitted/ > > > only one time. > > > > A second history global to all QAPISchema instances has been added > > to detect when a file is included more than one time and skip these > > includes. > > It does not act as a stack and the changes made to it by the > > __init__ function are propagated back to the caller so it's really > > a global state. > > > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > > --- > > > index 0000000..573541a > > --- /dev/null > > +++ b/tests/qapi-schema/include-only-one-time.exit > > @@ -0,0 +1 @@ > > +0 > > diff --git a/tests/qapi-schema/include-only-one-time.json b/tests/qapi-schema/include-only-one-time.json > > new file mode 100644 > > index 0000000..11772e3 > > --- /dev/null > > +++ b/tests/qapi-schema/include-only-one-time.json > > Long name; you could get by with: > > tests/qapi-schema/include-repetition.json > > > @@ -0,0 +1,3 @@ > > +{ 'include': 'comments.json' } > > +{ 'include': 'sub-include-only-one-time.json' } > > This naming doesn't preserve the 'include-' prefix used by all other > files related to include. Maybe: > > { 'include': 'include-repetition-sub.json' } > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > Thanks for the review I'll iterate.
diff --git a/scripts/qapi.py b/scripts/qapi.py index ec806aa..0265b40 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -73,13 +73,18 @@ class QAPIExprError(Exception): class QAPISchema: - def __init__(self, fp, input_relname=None, include_hist=[], parent_info=None): + def __init__(self, fp, input_relname=None, include_hist=[], + previously_included=[], parent_info=None): + """ include_hist is a stack used to detect inclusion cycles + previously_included is a global state used to avoid multiple + inclusions of the same file""" input_fname = os.path.abspath(fp.name) if input_relname is None: input_relname = fp.name self.input_dir = os.path.dirname(input_fname) self.input_file = input_relname self.include_hist = include_hist + [(input_relname, input_fname)] + previously_included.append(input_fname) self.parent_info = parent_info self.src = fp.read() if self.src == '' or self.src[-1] != '\n': @@ -106,13 +111,16 @@ class QAPISchema: for elem in self.include_hist): raise QAPIExprError(expr_info, "Inclusion loop for %s" % include) + # skip multiple include of the same file + if include_path in previously_included: + continue try: fobj = open(include_path, 'r') except IOError as e: raise QAPIExprError(expr_info, '%s: %s' % (e.strerror, include)) - exprs_include = QAPISchema(fobj, include, - self.include_hist, expr_info) + exprs_include = QAPISchema(fobj, include, self.include_hist, + previously_included, expr_info) self.exprs.extend(exprs_include.exprs) else: expr_elem = {'expr': expr, diff --git a/tests/Makefile b/tests/Makefile index a8d45be..a0f12fb 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -182,7 +182,8 @@ check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ flat-union-string-discriminator.json \ include-simple.json include-relpath.json include-format-err.json \ include-non-file.json include-no-file.json include-before-err.json \ - include-nested-err.json include-self-cycle.json include-cycle.json) + include-nested-err.json include-self-cycle.json include-cycle.json \ + include-only-one-time.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-only-one-time.err b/tests/qapi-schema/include-only-one-time.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/include-only-one-time.exit b/tests/qapi-schema/include-only-one-time.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/include-only-one-time.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/include-only-one-time.json b/tests/qapi-schema/include-only-one-time.json new file mode 100644 index 0000000..11772e3 --- /dev/null +++ b/tests/qapi-schema/include-only-one-time.json @@ -0,0 +1,3 @@ +{ 'include': 'comments.json' } +{ 'include': 'sub-include-only-one-time.json' } +{ 'include': 'comments.json' } diff --git a/tests/qapi-schema/include-only-one-time.out b/tests/qapi-schema/include-only-one-time.out new file mode 100644 index 0000000..4ce3dcf --- /dev/null +++ b/tests/qapi-schema/include-only-one-time.out @@ -0,0 +1,3 @@ +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] +[] diff --git a/tests/qapi-schema/sub-include-only-one-time.json b/tests/qapi-schema/sub-include-only-one-time.json new file mode 100644 index 0000000..6bfffdf --- /dev/null +++ b/tests/qapi-schema/sub-include-only-one-time.json @@ -0,0 +1,2 @@ +{ 'include': 'comments.json' } +{ 'include': 'comments.json' }
The purpose of this change is to help create a json file containing common definitions; each bit of generated C code must be spitted only one time. A second history global to all QAPISchema instances has been added to detect when a file is included more than one time and skip these includes. It does not act as a stack and the changes made to it by the __init__ function are propagated back to the caller so it's really a global state. Signed-off-by: Benoit Canet <benoit@irqsave.net> --- scripts/qapi.py | 14 +++++++++++--- tests/Makefile | 3 ++- tests/qapi-schema/include-only-one-time.exit | 1 + tests/qapi-schema/include-only-one-time.json | 3 +++ tests/qapi-schema/include-only-one-time.out | 3 +++ tests/qapi-schema/sub-include-only-one-time.json | 2 ++ 6 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 tests/qapi-schema/include-only-one-time.err create mode 100644 tests/qapi-schema/include-only-one-time.exit create mode 100644 tests/qapi-schema/include-only-one-time.json create mode 100644 tests/qapi-schema/include-only-one-time.out create mode 100644 tests/qapi-schema/sub-include-only-one-time.json