* [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done
@ 2014-05-14 14:07 Benoît Canet
2014-05-14 14:07 ` [Qemu-devel] [PATCH v3] qapi: skip redundant includes Benoît Canet
0 siblings, 1 reply; 8+ messages in thread
From: Benoît Canet @ 2014-05-14 14:07 UTC (permalink / raw)
To: qemu-devel; +Cc: Benoît Canet, armbru, vilanova, lcapitulino
in V3:
change commit title [Eric]
s/spitted/emitted/
change tests names [Eric]
Benoît Canet (1):
qapi: skip redundant includes
scripts/qapi.py | 14 +++++++++++---
tests/Makefile | 3 ++-
tests/qapi-schema/include-repetition-sub.json | 2 ++
tests/qapi-schema/include-repetition.err | 0
tests/qapi-schema/include-repetition.exit | 1 +
tests/qapi-schema/include-repetition.json | 3 +++
tests/qapi-schema/include-repetition.out | 3 +++
7 files changed, 22 insertions(+), 4 deletions(-)
create mode 100644 tests/qapi-schema/include-repetition-sub.json
create mode 100644 tests/qapi-schema/include-repetition.err
create mode 100644 tests/qapi-schema/include-repetition.exit
create mode 100644 tests/qapi-schema/include-repetition.json
create mode 100644 tests/qapi-schema/include-repetition.out
--
2.0.0.rc2
^ permalink raw reply [flat|nested] 8+ messages in thread* [Qemu-devel] [PATCH v3] qapi: skip redundant includes 2014-05-14 14:07 [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done Benoît Canet @ 2014-05-14 14:07 ` Benoît Canet 2014-05-14 15:04 ` Eric Blake 2014-05-15 19:13 ` Luiz Capitulino 0 siblings, 2 replies; 8+ messages in thread From: Benoît Canet @ 2014-05-14 14:07 UTC (permalink / raw) To: qemu-devel; +Cc: Benoît Canet, Benoit Canet, armbru, lcapitulino, vilanova The purpose of this change is to help create a json file containing common definitions; each bit of generated C code must be 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> --- scripts/qapi.py | 14 +++++++++++--- tests/Makefile | 3 ++- tests/qapi-schema/include-repetition-sub.json | 2 ++ tests/qapi-schema/include-repetition.err | 0 tests/qapi-schema/include-repetition.exit | 1 + tests/qapi-schema/include-repetition.json | 3 +++ tests/qapi-schema/include-repetition.out | 3 +++ 7 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 tests/qapi-schema/include-repetition-sub.json create mode 100644 tests/qapi-schema/include-repetition.err create mode 100644 tests/qapi-schema/include-repetition.exit create mode 100644 tests/qapi-schema/include-repetition.json create mode 100644 tests/qapi-schema/include-repetition.out 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..24c3d05 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-repetition.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-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json new file mode 100644 index 0000000..6bfffdf --- /dev/null +++ b/tests/qapi-schema/include-repetition-sub.json @@ -0,0 +1,2 @@ +{ 'include': 'comments.json' } +{ 'include': 'comments.json' } diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err new file mode 100644 index 0000000..e69de29 diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit new file mode 100644 index 0000000..573541a --- /dev/null +++ b/tests/qapi-schema/include-repetition.exit @@ -0,0 +1 @@ +0 diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json new file mode 100644 index 0000000..ec329dd --- /dev/null +++ b/tests/qapi-schema/include-repetition.json @@ -0,0 +1,3 @@ +{ 'include': 'comments.json' } +{ 'include': 'include-repetition-sub.json' } +{ 'include': 'comments.json' } diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out new file mode 100644 index 0000000..4ce3dcf --- /dev/null +++ b/tests/qapi-schema/include-repetition.out @@ -0,0 +1,3 @@ +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] +[] -- 2.0.0.rc2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes 2014-05-14 14:07 ` [Qemu-devel] [PATCH v3] qapi: skip redundant includes Benoît Canet @ 2014-05-14 15:04 ` Eric Blake 2014-05-15 19:13 ` Luiz Capitulino 1 sibling, 0 replies; 8+ messages in thread From: Eric Blake @ 2014-05-14 15:04 UTC (permalink / raw) To: Benoît Canet, qemu-devel; +Cc: armbru, Benoit Canet, vilanova, lcapitulino [-- Attachment #1: Type: text/plain, Size: 1537 bytes --] On 05/14/2014 08:07 AM, Benoît Canet wrote: > The purpose of this change is to help create a json file containing > common definitions; each bit of generated C code must be 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> > --- > scripts/qapi.py | 14 +++++++++++--- > tests/Makefile | 3 ++- > tests/qapi-schema/include-repetition-sub.json | 2 ++ > tests/qapi-schema/include-repetition.err | 0 > tests/qapi-schema/include-repetition.exit | 1 + > tests/qapi-schema/include-repetition.json | 3 +++ > tests/qapi-schema/include-repetition.out | 3 +++ > 7 files changed, 22 insertions(+), 4 deletions(-) > create mode 100644 tests/qapi-schema/include-repetition-sub.json > create mode 100644 tests/qapi-schema/include-repetition.err > create mode 100644 tests/qapi-schema/include-repetition.exit > create mode 100644 tests/qapi-schema/include-repetition.json > create mode 100644 tests/qapi-schema/include-repetition.out > Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes 2014-05-14 14:07 ` [Qemu-devel] [PATCH v3] qapi: skip redundant includes Benoît Canet 2014-05-14 15:04 ` Eric Blake @ 2014-05-15 19:13 ` Luiz Capitulino 2014-05-15 19:39 ` Eric Blake 1 sibling, 1 reply; 8+ messages in thread From: Luiz Capitulino @ 2014-05-15 19:13 UTC (permalink / raw) To: Benoît Canet; +Cc: armbru, Benoit Canet, qemu-devel, vilanova On Wed, 14 May 2014 16:07:49 +0200 Benoît Canet <benoit.canet@irqsave.net> wrote: > The purpose of this change is to help create a json file containing > common definitions; each bit of generated C code must be 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. Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt or qapi-code-gen.txt? > > Signed-off-by: Benoit Canet <benoit@irqsave.net> > --- > scripts/qapi.py | 14 +++++++++++--- > tests/Makefile | 3 ++- > tests/qapi-schema/include-repetition-sub.json | 2 ++ > tests/qapi-schema/include-repetition.err | 0 > tests/qapi-schema/include-repetition.exit | 1 + > tests/qapi-schema/include-repetition.json | 3 +++ > tests/qapi-schema/include-repetition.out | 3 +++ > 7 files changed, 22 insertions(+), 4 deletions(-) > create mode 100644 tests/qapi-schema/include-repetition-sub.json > create mode 100644 tests/qapi-schema/include-repetition.err > create mode 100644 tests/qapi-schema/include-repetition.exit > create mode 100644 tests/qapi-schema/include-repetition.json > create mode 100644 tests/qapi-schema/include-repetition.out > > 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..24c3d05 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-repetition.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-repetition-sub.json b/tests/qapi-schema/include-repetition-sub.json > new file mode 100644 > index 0000000..6bfffdf > --- /dev/null > +++ b/tests/qapi-schema/include-repetition-sub.json > @@ -0,0 +1,2 @@ > +{ 'include': 'comments.json' } > +{ 'include': 'comments.json' } > diff --git a/tests/qapi-schema/include-repetition.err b/tests/qapi-schema/include-repetition.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/include-repetition.exit b/tests/qapi-schema/include-repetition.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/include-repetition.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/include-repetition.json b/tests/qapi-schema/include-repetition.json > new file mode 100644 > index 0000000..ec329dd > --- /dev/null > +++ b/tests/qapi-schema/include-repetition.json > @@ -0,0 +1,3 @@ > +{ 'include': 'comments.json' } > +{ 'include': 'include-repetition-sub.json' } > +{ 'include': 'comments.json' } > diff --git a/tests/qapi-schema/include-repetition.out b/tests/qapi-schema/include-repetition.out > new file mode 100644 > index 0000000..4ce3dcf > --- /dev/null > +++ b/tests/qapi-schema/include-repetition.out > @@ -0,0 +1,3 @@ > +[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])] > +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}] > +[] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes 2014-05-15 19:13 ` Luiz Capitulino @ 2014-05-15 19:39 ` Eric Blake 2014-05-15 20:03 ` Benoît Canet 0 siblings, 1 reply; 8+ messages in thread From: Eric Blake @ 2014-05-15 19:39 UTC (permalink / raw) To: Luiz Capitulino, Benoît Canet Cc: armbru, Benoit Canet, qemu-devel, vilanova [-- Attachment #1: Type: text/plain, Size: 1007 bytes --] On 05/15/2014 01:13 PM, Luiz Capitulino wrote: > On Wed, 14 May 2014 16:07:49 +0200 > Benoît Canet <benoit.canet@irqsave.net> wrote: > >> The purpose of this change is to help create a json file containing >> common definitions; each bit of generated C code must be 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. > > Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt > or qapi-code-gen.txt? dosc/qapi-code-gen.txt already covers the {'include':'file'} syntax, so yes, that should be updated to mention that multiple includes of the same file are now safe. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes 2014-05-15 19:39 ` Eric Blake @ 2014-05-15 20:03 ` Benoît Canet 0 siblings, 0 replies; 8+ messages in thread From: Benoît Canet @ 2014-05-15 20:03 UTC (permalink / raw) To: Eric Blake Cc: Benoît Canet, armbru, vilanova, qemu-devel, Luiz Capitulino The Thursday 15 May 2014 à 13:39:02 (-0600), Eric Blake wrote : > On 05/15/2014 01:13 PM, Luiz Capitulino wrote: > > On Wed, 14 May 2014 16:07:49 +0200 > > Benoît Canet <benoit.canet@irqsave.net> wrote: > > > >> The purpose of this change is to help create a json file containing > >> common definitions; each bit of generated C code must be 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. > > > > Shouldn't this behavior be noted somewhere? Eg. in writing-qmp-commands.txt > > or qapi-code-gen.txt? > > dosc/qapi-code-gen.txt already covers the {'include':'file'} syntax, so > yes, that should be updated to mention that multiple includes of the > same file are now safe. ok, I will do this. Best regards Benoît > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done
@ 2014-06-27 13:41 Benoît Canet
2014-06-27 15:15 ` Benoît Canet
0 siblings, 1 reply; 8+ messages in thread
From: Benoît Canet @ 2014-06-27 13:41 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Benoît Canet, stefanha
in V3:
change commit title [Eric]
s/spitted/emitted/
change tests names [Eric]
Benoît Canet (1):
qapi: skip redundant includes
scripts/qapi.py | 14 +++++++++++---
tests/Makefile | 3 ++-
tests/qapi-schema/include-repetition-sub.json | 2 ++
tests/qapi-schema/include-repetition.err | 0
tests/qapi-schema/include-repetition.exit | 1 +
tests/qapi-schema/include-repetition.json | 3 +++
tests/qapi-schema/include-repetition.out | 3 +++
7 files changed, 22 insertions(+), 4 deletions(-)
create mode 100644 tests/qapi-schema/include-repetition-sub.json
create mode 100644 tests/qapi-schema/include-repetition.err
create mode 100644 tests/qapi-schema/include-repetition.exit
create mode 100644 tests/qapi-schema/include-repetition.json
create mode 100644 tests/qapi-schema/include-repetition.out
--
2.0.0.rc2
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done 2014-06-27 13:41 [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done Benoît Canet @ 2014-06-27 15:15 ` Benoît Canet 0 siblings, 0 replies; 8+ messages in thread From: Benoît Canet @ 2014-06-27 15:15 UTC (permalink / raw) To: Benoît Canet; +Cc: kwolf, qemu-devel, stefanha The Friday 27 Jun 2014 à 15:41:50 (+0200), Benoît Canet wrote : > in V3: > change commit title [Eric] > s/spitted/emitted/ > change tests names [Eric] > > Benoît Canet (1): > qapi: skip redundant includes > > scripts/qapi.py | 14 +++++++++++--- > tests/Makefile | 3 ++- > tests/qapi-schema/include-repetition-sub.json | 2 ++ > tests/qapi-schema/include-repetition.err | 0 > tests/qapi-schema/include-repetition.exit | 1 + > tests/qapi-schema/include-repetition.json | 3 +++ > tests/qapi-schema/include-repetition.out | 3 +++ > 7 files changed, 22 insertions(+), 4 deletions(-) > create mode 100644 tests/qapi-schema/include-repetition-sub.json > create mode 100644 tests/qapi-schema/include-repetition.err > create mode 100644 tests/qapi-schema/include-repetition.exit > create mode 100644 tests/qapi-schema/include-repetition.json > create mode 100644 tests/qapi-schema/include-repetition.out Please forgot this one is send two separate directories at once. > > -- > 2.0.0.rc2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-06-27 15:16 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-14 14:07 [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done Benoît Canet 2014-05-14 14:07 ` [Qemu-devel] [PATCH v3] qapi: skip redundant includes Benoît Canet 2014-05-14 15:04 ` Eric Blake 2014-05-15 19:13 ` Luiz Capitulino 2014-05-15 19:39 ` Eric Blake 2014-05-15 20:03 ` Benoît Canet -- strict thread matches above, loose matches on Subject: below -- 2014-06-27 13:41 [Qemu-devel] [PATCH v3] Let the C generated bits of QAPI be generated only once when identical includes are done Benoît Canet 2014-06-27 15:15 ` Benoît Canet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).