[PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)

Markus Armbruster posted 12 patches 2 years, 7 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210831123809.1107782-1-armbru@redhat.com
Maintainers: Michael Roth <michael.roth@amd.com>, Markus Armbruster <armbru@redhat.com>
scripts/qapi/common.py                  | 51 ++++++++++++++-----------
scripts/qapi/expr.py                    | 32 +++++++---------
scripts/qapi/gen.py                     |  6 +--
scripts/qapi/introspect.py              |  6 +--
scripts/qapi/schema.py                  | 12 +++++-
scripts/qapi/types.py                   | 22 +++++------
scripts/qapi/visit.py                   | 14 +++----
tests/qapi-schema/bad-if-key.err        |  2 +-
tests/qapi-schema/bad-if-keys.err       |  2 +-
tests/qapi-schema/doc-good.json         |  2 +-
tests/qapi-schema/doc-good.out          |  6 +--
tests/qapi-schema/doc-good.txt          |  8 ++--
tests/qapi-schema/double-type.err       |  4 +-
tests/qapi-schema/enum-if-invalid.err   |  2 +-
tests/qapi-schema/missing-type.err      |  2 +-
tests/qapi-schema/qapi-schema-test.json |  9 +++--
tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
tests/qapi-schema/test-qapi.py          | 11 +++++-
18 files changed, 118 insertions(+), 104 deletions(-)
[PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by Markus Armbruster 2 years, 7 months ago
Markus Armbruster (12):
  qapi: Simplify QAPISchemaIfCond's interface for generating C
  qapi: Simplify how QAPISchemaIfCond represents "no condition"
  tests/qapi-schema: Correct two 'if' conditionals
  tests/qapi-schema: Demonstrate broken C code for 'if'
  qapi: Fix C code generation for 'if'
  qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
  qapi: Avoid redundant parens in code generated for conditionals
  qapi: Use "not COND" instead of "!COND" for generated documentation
  qapi: Use re.fullmatch() where appropriate
  tests/qapi-schema: Hide OrderedDict in test output
  qapi: Tweak error messages for missing / conflicting meta-type
  qapi: Tweak error messages for unknown / conflicting 'if' keys

 scripts/qapi/common.py                  | 51 ++++++++++++++-----------
 scripts/qapi/expr.py                    | 32 +++++++---------
 scripts/qapi/gen.py                     |  6 +--
 scripts/qapi/introspect.py              |  6 +--
 scripts/qapi/schema.py                  | 12 +++++-
 scripts/qapi/types.py                   | 22 +++++------
 scripts/qapi/visit.py                   | 14 +++----
 tests/qapi-schema/bad-if-key.err        |  2 +-
 tests/qapi-schema/bad-if-keys.err       |  2 +-
 tests/qapi-schema/doc-good.json         |  2 +-
 tests/qapi-schema/doc-good.out          |  6 +--
 tests/qapi-schema/doc-good.txt          |  8 ++--
 tests/qapi-schema/double-type.err       |  4 +-
 tests/qapi-schema/enum-if-invalid.err   |  2 +-
 tests/qapi-schema/missing-type.err      |  2 +-
 tests/qapi-schema/qapi-schema-test.json |  9 +++--
 tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
 tests/qapi-schema/test-qapi.py          | 11 +++++-
 18 files changed, 118 insertions(+), 104 deletions(-)

-- 
2.31.1


Re: [PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by Markus Armbruster 2 years, 6 months ago
Queued.


Re: [PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by Marc-André Lureau 2 years, 7 months ago
On Tue, Aug 31, 2021 at 4:38 PM Markus Armbruster <armbru@redhat.com> wrote:

> Markus Armbruster (12):
>   qapi: Simplify QAPISchemaIfCond's interface for generating C
>   qapi: Simplify how QAPISchemaIfCond represents "no condition"
>   tests/qapi-schema: Correct two 'if' conditionals
>   tests/qapi-schema: Demonstrate broken C code for 'if'
>   qapi: Fix C code generation for 'if'
>   qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
>   qapi: Avoid redundant parens in code generated for conditionals
>   qapi: Use "not COND" instead of "!COND" for generated documentation
>   qapi: Use re.fullmatch() where appropriate
>   tests/qapi-schema: Hide OrderedDict in test output
>   qapi: Tweak error messages for missing / conflicting meta-type
>   qapi: Tweak error messages for unknown / conflicting 'if' keys
>
>  scripts/qapi/common.py                  | 51 ++++++++++++++-----------
>  scripts/qapi/expr.py                    | 32 +++++++---------
>  scripts/qapi/gen.py                     |  6 +--
>  scripts/qapi/introspect.py              |  6 +--
>  scripts/qapi/schema.py                  | 12 +++++-
>  scripts/qapi/types.py                   | 22 +++++------
>  scripts/qapi/visit.py                   | 14 +++----
>  tests/qapi-schema/bad-if-key.err        |  2 +-
>  tests/qapi-schema/bad-if-keys.err       |  2 +-
>  tests/qapi-schema/doc-good.json         |  2 +-
>  tests/qapi-schema/doc-good.out          |  6 +--
>  tests/qapi-schema/doc-good.txt          |  8 ++--
>  tests/qapi-schema/double-type.err       |  4 +-
>  tests/qapi-schema/enum-if-invalid.err   |  2 +-
>  tests/qapi-schema/missing-type.err      |  2 +-
>  tests/qapi-schema/qapi-schema-test.json |  9 +++--
>  tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
>  tests/qapi-schema/test-qapi.py          | 11 +++++-
>  18 files changed, 118 insertions(+), 104 deletions(-)
>
>
The first patch, you should apply isort (we should have a check for that
and linters I suppose).

Series:
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

-- 
> 2.31.1
>
>
Re: [PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by Markus Armbruster 2 years, 7 months ago
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> On Tue, Aug 31, 2021 at 4:38 PM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Markus Armbruster (12):
>>   qapi: Simplify QAPISchemaIfCond's interface for generating C
>>   qapi: Simplify how QAPISchemaIfCond represents "no condition"
>>   tests/qapi-schema: Correct two 'if' conditionals
>>   tests/qapi-schema: Demonstrate broken C code for 'if'
>>   qapi: Fix C code generation for 'if'
>>   qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
>>   qapi: Avoid redundant parens in code generated for conditionals
>>   qapi: Use "not COND" instead of "!COND" for generated documentation
>>   qapi: Use re.fullmatch() where appropriate
>>   tests/qapi-schema: Hide OrderedDict in test output
>>   qapi: Tweak error messages for missing / conflicting meta-type
>>   qapi: Tweak error messages for unknown / conflicting 'if' keys
>>
>>  scripts/qapi/common.py                  | 51 ++++++++++++++-----------
>>  scripts/qapi/expr.py                    | 32 +++++++---------
>>  scripts/qapi/gen.py                     |  6 +--
>>  scripts/qapi/introspect.py              |  6 +--
>>  scripts/qapi/schema.py                  | 12 +++++-
>>  scripts/qapi/types.py                   | 22 +++++------
>>  scripts/qapi/visit.py                   | 14 +++----
>>  tests/qapi-schema/bad-if-key.err        |  2 +-
>>  tests/qapi-schema/bad-if-keys.err       |  2 +-
>>  tests/qapi-schema/doc-good.json         |  2 +-
>>  tests/qapi-schema/doc-good.out          |  6 +--
>>  tests/qapi-schema/doc-good.txt          |  8 ++--
>>  tests/qapi-schema/double-type.err       |  4 +-
>>  tests/qapi-schema/enum-if-invalid.err   |  2 +-
>>  tests/qapi-schema/missing-type.err      |  2 +-
>>  tests/qapi-schema/qapi-schema-test.json |  9 +++--
>>  tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
>>  tests/qapi-schema/test-qapi.py          | 11 +++++-
>>  18 files changed, 118 insertions(+), 104 deletions(-)
>>
>>
> The first patch, you should apply isort

Will fix.

> The first patch, you should apply isort (we should have a check for that
> and linters I suppose).

John will get us there.

> Series:
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Thanks!


Re: [PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by John Snow 2 years, 6 months ago
On Tue, Aug 31, 2021 at 10:28 AM Markus Armbruster <armbru@redhat.com>
wrote:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > On Tue, Aug 31, 2021 at 4:38 PM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> Markus Armbruster (12):
> >>   qapi: Simplify QAPISchemaIfCond's interface for generating C
> >>   qapi: Simplify how QAPISchemaIfCond represents "no condition"
> >>   tests/qapi-schema: Correct two 'if' conditionals
> >>   tests/qapi-schema: Demonstrate broken C code for 'if'
> >>   qapi: Fix C code generation for 'if'
> >>   qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
> >>   qapi: Avoid redundant parens in code generated for conditionals
> >>   qapi: Use "not COND" instead of "!COND" for generated documentation
> >>   qapi: Use re.fullmatch() where appropriate
> >>   tests/qapi-schema: Hide OrderedDict in test output
> >>   qapi: Tweak error messages for missing / conflicting meta-type
> >>   qapi: Tweak error messages for unknown / conflicting 'if' keys
> >>
> >>  scripts/qapi/common.py                  | 51 ++++++++++++++-----------
> >>  scripts/qapi/expr.py                    | 32 +++++++---------
> >>  scripts/qapi/gen.py                     |  6 +--
> >>  scripts/qapi/introspect.py              |  6 +--
> >>  scripts/qapi/schema.py                  | 12 +++++-
> >>  scripts/qapi/types.py                   | 22 +++++------
> >>  scripts/qapi/visit.py                   | 14 +++----
> >>  tests/qapi-schema/bad-if-key.err        |  2 +-
> >>  tests/qapi-schema/bad-if-keys.err       |  2 +-
> >>  tests/qapi-schema/doc-good.json         |  2 +-
> >>  tests/qapi-schema/doc-good.out          |  6 +--
> >>  tests/qapi-schema/doc-good.txt          |  8 ++--
> >>  tests/qapi-schema/double-type.err       |  4 +-
> >>  tests/qapi-schema/enum-if-invalid.err   |  2 +-
> >>  tests/qapi-schema/missing-type.err      |  2 +-
> >>  tests/qapi-schema/qapi-schema-test.json |  9 +++--
> >>  tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
> >>  tests/qapi-schema/test-qapi.py          | 11 +++++-
> >>  18 files changed, 118 insertions(+), 104 deletions(-)
> >>
> >>
> > The first patch, you should apply isort
>
> Will fix.
>
> > The first patch, you should apply isort (we should have a check for that
> > and linters I suppose).
>
> John will get us there.
>
>
The goal is to move scripts/qapi to python/qemu/qapi where it will be
covered by the tests that exist there.
Try going to the python/ directory and run 'make' to see help output on
what tests are available and how to invoke them, and what they actually
test.

"make check-dev" is the target that requires the least amount of
dependencies and environment setup to run, and should be nearly push-button
in most environments.
'make check-tox' and 'make check-pipenv' are executed by GitlabCI as
check-python-tox and check-python-pipenv, respectively.

In the meantime, I've been running tests against qapi manually -- with the
scripts I included in my 'python-qapi-cleanup-pt0' branch.

--js
Re: [PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by Marc-André Lureau 2 years, 6 months ago
Hi

On Wed, Sep 15, 2021 at 6:51 PM John Snow <jsnow@redhat.com> wrote:

>
>
> On Tue, Aug 31, 2021 at 10:28 AM Markus Armbruster <armbru@redhat.com>
> wrote:
>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > On Tue, Aug 31, 2021 at 4:38 PM Markus Armbruster <armbru@redhat.com>
>> wrote:
>> >
>> >> Markus Armbruster (12):
>> >>   qapi: Simplify QAPISchemaIfCond's interface for generating C
>> >>   qapi: Simplify how QAPISchemaIfCond represents "no condition"
>> >>   tests/qapi-schema: Correct two 'if' conditionals
>> >>   tests/qapi-schema: Demonstrate broken C code for 'if'
>> >>   qapi: Fix C code generation for 'if'
>> >>   qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
>> >>   qapi: Avoid redundant parens in code generated for conditionals
>> >>   qapi: Use "not COND" instead of "!COND" for generated documentation
>> >>   qapi: Use re.fullmatch() where appropriate
>> >>   tests/qapi-schema: Hide OrderedDict in test output
>> >>   qapi: Tweak error messages for missing / conflicting meta-type
>> >>   qapi: Tweak error messages for unknown / conflicting 'if' keys
>> >>
>> >>  scripts/qapi/common.py                  | 51 ++++++++++++++-----------
>> >>  scripts/qapi/expr.py                    | 32 +++++++---------
>> >>  scripts/qapi/gen.py                     |  6 +--
>> >>  scripts/qapi/introspect.py              |  6 +--
>> >>  scripts/qapi/schema.py                  | 12 +++++-
>> >>  scripts/qapi/types.py                   | 22 +++++------
>> >>  scripts/qapi/visit.py                   | 14 +++----
>> >>  tests/qapi-schema/bad-if-key.err        |  2 +-
>> >>  tests/qapi-schema/bad-if-keys.err       |  2 +-
>> >>  tests/qapi-schema/doc-good.json         |  2 +-
>> >>  tests/qapi-schema/doc-good.out          |  6 +--
>> >>  tests/qapi-schema/doc-good.txt          |  8 ++--
>> >>  tests/qapi-schema/double-type.err       |  4 +-
>> >>  tests/qapi-schema/enum-if-invalid.err   |  2 +-
>> >>  tests/qapi-schema/missing-type.err      |  2 +-
>> >>  tests/qapi-schema/qapi-schema-test.json |  9 +++--
>> >>  tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
>> >>  tests/qapi-schema/test-qapi.py          | 11 +++++-
>> >>  18 files changed, 118 insertions(+), 104 deletions(-)
>> >>
>> >>
>> > The first patch, you should apply isort
>>
>> Will fix.
>>
>> > The first patch, you should apply isort (we should have a check for that
>> > and linters I suppose).
>>
>> John will get us there.
>>
>>
> The goal is to move scripts/qapi to python/qemu/qapi where it will be
> covered by the tests that exist there.
> Try going to the python/ directory and run 'make' to see help output on
> what tests are available and how to invoke them, and what they actually
> test.
>
> "make check-dev" is the target that requires the least amount of
> dependencies and environment setup to run, and should be nearly push-button
> in most environments.
> 'make check-tox' and 'make check-pipenv' are executed by GitlabCI as
> check-python-tox and check-python-pipenv, respectively.
>
>
What about a pre-commit hook?


> In the meantime, I've been running tests against qapi manually -- with the
> scripts I included in my 'python-qapi-cleanup-pt0' branch.
>
> --js
>
Re: [PATCH 00/12] qapi: Fixes and cleanups for recent work (mostly)
Posted by John Snow 2 years, 6 months ago
On Wed, Sep 15, 2021 at 11:08 AM Marc-André Lureau <
marcandre.lureau@redhat.com> wrote:

> Hi
>
> On Wed, Sep 15, 2021 at 6:51 PM John Snow <jsnow@redhat.com> wrote:
>
>>
>>
>> On Tue, Aug 31, 2021 at 10:28 AM Markus Armbruster <armbru@redhat.com>
>> wrote:
>>
>>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>>
>>> > On Tue, Aug 31, 2021 at 4:38 PM Markus Armbruster <armbru@redhat.com>
>>> wrote:
>>> >
>>> >> Markus Armbruster (12):
>>> >>   qapi: Simplify QAPISchemaIfCond's interface for generating C
>>> >>   qapi: Simplify how QAPISchemaIfCond represents "no condition"
>>> >>   tests/qapi-schema: Correct two 'if' conditionals
>>> >>   tests/qapi-schema: Demonstrate broken C code for 'if'
>>> >>   qapi: Fix C code generation for 'if'
>>> >>   qapi: Factor common recursion out of cgen_ifcond(), docgen_ifcond()
>>> >>   qapi: Avoid redundant parens in code generated for conditionals
>>> >>   qapi: Use "not COND" instead of "!COND" for generated documentation
>>> >>   qapi: Use re.fullmatch() where appropriate
>>> >>   tests/qapi-schema: Hide OrderedDict in test output
>>> >>   qapi: Tweak error messages for missing / conflicting meta-type
>>> >>   qapi: Tweak error messages for unknown / conflicting 'if' keys
>>> >>
>>> >>  scripts/qapi/common.py                  | 51
>>> ++++++++++++++-----------
>>> >>  scripts/qapi/expr.py                    | 32 +++++++---------
>>> >>  scripts/qapi/gen.py                     |  6 +--
>>> >>  scripts/qapi/introspect.py              |  6 +--
>>> >>  scripts/qapi/schema.py                  | 12 +++++-
>>> >>  scripts/qapi/types.py                   | 22 +++++------
>>> >>  scripts/qapi/visit.py                   | 14 +++----
>>> >>  tests/qapi-schema/bad-if-key.err        |  2 +-
>>> >>  tests/qapi-schema/bad-if-keys.err       |  2 +-
>>> >>  tests/qapi-schema/doc-good.json         |  2 +-
>>> >>  tests/qapi-schema/doc-good.out          |  6 +--
>>> >>  tests/qapi-schema/doc-good.txt          |  8 ++--
>>> >>  tests/qapi-schema/double-type.err       |  4 +-
>>> >>  tests/qapi-schema/enum-if-invalid.err   |  2 +-
>>> >>  tests/qapi-schema/missing-type.err      |  2 +-
>>> >>  tests/qapi-schema/qapi-schema-test.json |  9 +++--
>>> >>  tests/qapi-schema/qapi-schema-test.out  | 31 ++++++++-------
>>> >>  tests/qapi-schema/test-qapi.py          | 11 +++++-
>>> >>  18 files changed, 118 insertions(+), 104 deletions(-)
>>> >>
>>> >>
>>> > The first patch, you should apply isort
>>>
>>> Will fix.
>>>
>>> > The first patch, you should apply isort (we should have a check for
>>> that
>>> > and linters I suppose).
>>>
>>> John will get us there.
>>>
>>>
>> The goal is to move scripts/qapi to python/qemu/qapi where it will be
>> covered by the tests that exist there.
>> Try going to the python/ directory and run 'make' to see help output on
>> what tests are available and how to invoke them, and what they actually
>> test.
>>
>> "make check-dev" is the target that requires the least amount of
>> dependencies and environment setup to run, and should be nearly push-button
>> in most environments.
>> 'make check-tox' and 'make check-pipenv' are executed by GitlabCI as
>> check-python-tox and check-python-pipenv, respectively.
>>
>>
> What about a pre-commit hook?
>

"patches welcome" ? There are other python tests likely to go here in the
future too, so it might be too costly to run as a commit hook. YMMV.

--js