[Spice-devel] [RFC PATCH spice-protocol] codegen: Filter not defined messages from protocol

[Marc-André Lureau]

Hi

On Fri, Feb 19, 2016 at 9:21 AM, Pavel Grunt <pgrunt at redhat.com> wrote:
> Do not create functions for a message which doesn't have a definition
> in the included header file.
>
> Make spice-protocol again compatible with older spice-common.
> ---
> Hi, this is an attemp to make spice-protocol compatible with older releases of spice / spice-gtk
>
> Currently released versions of spice and spice-gtk are failing to build
> using spice-protocol from git master:
>   CC       libspice_common_server_la-generated_server_demarshallers.lo
> generated_server_demarshallers.c: In function 'parse_msgc_display_gl_draw_done':
> generated_server_demarshallers.c:767:23: error: 'SpiceMsgcDisplayGlDrawDone' undeclared (first use in this function)
>      mem_size = sizeof(SpiceMsgcDisplayGlDrawDone);
>                        ^
> It is due to the fact that spice-common calls spice_codegen.py from spice-protocol
> to generated marshallers from spice.proto (which is included in spice-protocol).
> However structures defining messages from spice.proto are in "common/messages.h"
> which is in spice-common. So generated code contains new messages but the header doesn't.
>
> To fix the issue I took advantage of spice-common calling the codegen with the header file,
> see spice-common/common/Makefile.am:
>
>         spice_codegen.py --generate-demarshallers --client --include common/messages.h
>
> Codegen then opens the header, gets its content and checks if messages defined in spice.proto
> are also defined in the header. If not the message is discarded.
>
> The code needs some improvements (the find_file() function), but what do you think about the approach?

This is a hack. But the problem exists and is fairly urgent imho. So I
think we should apply it asap, and add FIXME/TODO.

>
> Thanks for any comments,
> Pavel
>
> ---
>  spice_codegen.py | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>
> diff --git a/spice_codegen.py b/spice_codegen.py
> index 569cccc..06f8fe1 100755
> --- a/spice_codegen.py
> +++ b/spice_codegen.py
> @@ -105,6 +105,30 @@ def write_enums(writer, describe=False):
>
>      writer.writeln("#endif /* _H_SPICE_ENUMS */")
>
> +def filter_protocol_messages(proto, headers):
> +    """ Remove messages which don't have the definition in a header"""
> +    def find_file(name):
> +        fname = os.path.basename(name)
> +        for root, dirs, files in os.walk(os.getcwd()):
> +            if fname in files:
> +                return os.path.join(root, fname)
> +        return None
> +
> +    if headers == None:
> +        return
> +
> +    filesstr = ""
> +    for f in headers:
> +        path_to_file = find_file(f)
> +        if path_to_file != None:
> +            with open(path_to_file) as content_file:
> +                filesstr += content_file.read()
> +
> +    filter_func = lambda msg : msg.message_type.c_type().replace("Temp","Spice") in filesstr
> +    for channel in proto.channels:
> +        channel.channel_type.client_messages = filter(filter_func, channel.channel_type.client_messages)
> +        channel.channel_type.server_messages = filter(filter_func, channel.channel_type.server_messages)
> +
>  parser = OptionParser(usage="usage: %prog [options] <protocol_file> <destination file>")
>  parser.add_option("-e", "--generate-enums",
>                    action="store_true", dest="generate_enums", default=False,
> @@ -209,6 +233,7 @@ if options.print_error:
>      writer.set_option("print_error")
>
>  if options.includes:
> +    filter_protocol_messages(proto, options.includes)
>      for i in options.includes:
>          writer.header.writeln('#include <%s>' % i)
>          writer.writeln('#include <%s>' % i)
> --
> 2.5.0
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



-- 
Marc-André Lureau