Re: [PATCH v3 1/8] qapi: golang: Generate enum type

[Victor Toso]

Hi,

On Tue, Jan 14, 2025 at 09:52:23AM +0100, Markus Armbruster wrote:
> Victor Toso <victortoso@redhat.com> writes:
> 
> > This patch handles QAPI enum types and generates its equivalent in Go.
> > We sort the output based on enum's type name.
> 
> Any particular reason for sorting?

It was a request from Daniel that I've accepted.
https://lists.gnu.org/archive/html/qemu-devel/2023-09/msg07042.html

We did the same thing in the code generator in libvirt-go-module
and the result helps with navigating the code and also with the
diff itself when we regenerate.
 
> The existing backends generate output it source order, on the (bold?)
> assumption that developers care to pick an order that makes sense.
> 
> > Enums are being handled as strings in Golang.
> >
> > 1. For each QAPI enum, we will define a string type in Go to be the
> >    assigned type of this specific enum.
> >
> > 2. Naming: CamelCase will be used in any identifier that we want to
> >    export, which is everything.
> >
> > Example:
> >
> > qapi:
> >   | ##
> >   | # @DisplayProtocol:
> >   | #
> >   | # Display protocols which support changing password options.
> >   | #
> >   | # Since: 7.0
> >   | ##
> >   | { 'enum': 'DisplayProtocol',
> >   |   'data': [ 'vnc', 'spice' ] }
> >
> > go:
> >   | // Display protocols which support changing password options.
> >   | //
> >   | // Since: 7.0
> >   | type DisplayProtocol string
> >   |
> >   | const (
> >   |         DisplayProtocolVnc   DisplayProtocol = "vnc"
> >   |         DisplayProtocolSpice DisplayProtocol = "spice"
> >   | )
> >
> > Signed-off-by: Victor Toso <victortoso@redhat.com>
> > ---
> >  scripts/qapi/golang.py | 266 +++++++++++++++++++++++++++++++++++++++++
> >  scripts/qapi/main.py   |   3 +
> >  2 files changed, 269 insertions(+)
> >  create mode 100644 scripts/qapi/golang.py
> >
> > diff --git a/scripts/qapi/golang.py b/scripts/qapi/golang.py
> > new file mode 100644
> > index 0000000000..1e04c99f1c
> > --- /dev/null
> > +++ b/scripts/qapi/golang.py
> 
> [...]
> 
> > +class QAPISchemaGenGolangVisitor(QAPISchemaVisitor):
> 
> [...]
> 
> > +    def write(self, output_dir: str) -> None:
> > +        for module_name, content in self.target.items():
> > +            go_module = module_name + "s.go"
> > +            go_dir = "go"
> > +            pathname = os.path.join(output_dir, go_dir, go_module)
> > +            odir = os.path.dirname(pathname)
> > +            os.makedirs(odir, exist_ok=True)
> > +
> > +            with open(pathname, "w", encoding="utf8") as outfile:
> > +                outfile.write(content)
> 
> Your write() serves the same purpose as QAPIGen.write().  The latter
> touches output files only when their contents actually changes.
> 
> Have you considered use of QAPIGen?

I have not, didn't know that actually.
I have to investigate if it'd be beneficial. Considering this is
once per release execution, shouldn't be a big problem either
way.
 
> The backends generating C use QAPISchemaMonolithicCVisitor or
> QAPISchemaModularCVisitor, which use QAPIGenC, QAPIGenH and
> QAPIGenTrace, all specializations of QAPIGen.
> 
> > diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
> > index 316736b6a2..f1f813b466 100644
> > --- a/scripts/qapi/main.py
> > +++ b/scripts/qapi/main.py
> > @@ -15,6 +15,7 @@
> >  from .common import must_match
> >  from .error import QAPIError
> >  from .events import gen_events
> > +from .golang import gen_golang
> >  from .introspect import gen_introspect
> >  from .schema import QAPISchema
> >  from .types import gen_types
> > @@ -54,6 +55,8 @@ def generate(schema_file: str,
> >      gen_events(schema, output_dir, prefix)
> >      gen_introspect(schema, output_dir, prefix, unmask)
> >  
> > +    gen_golang(schema, output_dir, prefix)
> > +
> >  
> >  def main() -> int:
> >      """

Thanks for the review,
Victor

signature.asc
Description: PGP signature