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

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

[Benoît Canet]

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

[Qemu-devel] [PATCH v3] qapi: skip redundant includes

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

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Eric Blake]

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

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Luiz Capitulino]

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']}]
> +[]

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Eric Blake]

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

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Benoît Canet]

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
> 

[Qemu-devel] [PATCH v3] qapi: skip redundant includes

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

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Markus Armbruster]

Benoît Canet <benoit.canet@irqsave.net> writes:

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

Committed a couple of weeks ago as 24fd848.

Re: [Qemu-devel] [PATCH v3] qapi: skip redundant includes

[Benoît Canet]

The Friday 27 Jun 2014 à 17:14:33 (+0200), Markus Armbruster wrote :
> Benoît Canet <benoit.canet@irqsave.net> writes:
> 
> > 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>
> 
> Committed a couple of weeks ago as 24fd848.

Yes I took a git send-email line in my bash history and forgot to remove a directory to send
while adding one another.

Best regards

Benoît