[Qemu-devel] [PATCH V2] Let the C generated bits of QAPI be generated only once when identical includes are done

[Qemu-devel] [PATCH V2] Let the C generated bits of QAPI be generated only once when identical includes are done

[Benoît Canet]

in V2:
        Keep the cycle detection [Markus]
        Don't talk about idempottent [Markus]

Benoît Canet (1):
  qapi: Let redundant includes be skipped excepted the first
    occurrence.

 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

-- 
1.7.10.4

[Qemu-devel] [PATCH V2] qapi: Let redundant includes be skipped excepted the first occurrence.

[Benoît Canet]

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

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' }
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH V2] qapi: Let redundant includes be skipped excepted the first occurrence.

[Eric Blake]

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH V2] qapi: Let redundant includes be skipped excepted the first occurrence.

[Benoît Canet]

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.