letmegrpc icon indicating copy to clipboard operation
letmegrpc copied to clipboard

protoc-gen-gofast generates different names

Open tgulacsi opened this issue 8 years ago • 11 comments

db_dealer.zip protoc-gen-gofast generates DbDealer_UtolsoMod from the proto name db_dealer__utolso_mod.

The protoc-gen-letmegrpc plugin uses the names from the proto unchanged. I've tried to replicate what gofast does (vanity and vanity/command) but seems a little bit magic, and when it worked, it did not compile, missed the "Serve" function from the html plugin.

Please help me sort this out!

package main

import (
    "strings"

    "github.com/gogo/letmegrpc/html"
    "github.com/gogo/protobuf/proto"
    "github.com/gogo/protobuf/protoc-gen-gogo/generator"
    "github.com/gogo/protobuf/vanity/command"
)

func main() {
    req := command.Read()
    //files := req.GetProtoFile()
    //files = vanity.FilterFiles(files, vanity.NotGoogleProtobufDescriptorProto)
    //vanity.ForEachFile(files, vanity.TurnOnMarshalerAll)
    //vanity.ForEachFile(files, vanity.TurnOnSizerAll)
    //vanity.ForEachFile(files, vanity.TurnOnUnmarshalerAll)

    //resp := command.Generate(req)
    //command.Write(resp)

    gen := generator.New()
    gen.Request = req
    gen.CommandLineParameters(gen.Request.GetParameter())
    gen.WrapTypes()
    gen.SetPackageNames()
    gen.BuildTypeNameMap()
    gen.GeneratePlugin(html.New())
        gen.Response = resp

    for i := 0; i < len(gen.Response.File); i++ {
        gen.Response.File[i].Name = proto.String(strings.Replace(*gen.Response.File[i].Name, ".pb.go", ".letmegrpc.go", -1))
    }

    // Send back the results.
    command.Write(gen.Response)
}

tgulacsi avatar Sep 20 '16 12:09 tgulacsi

Ok so letmegrpc does not call the CamelCase function. Basically if you find the place where this function should be called you can fix it properly.

The easiest fix though is just to change you message name to match the camel cased name.

awalterschulze avatar Sep 22 '16 17:09 awalterschulze

I went back to proto-gen-go, and read & implemented the Protocol Buffers Style Guide (CamelCase for message and service names, snake_case for field names), and now letmegrpc starts flawlessly, it's only complaint is a missing GZIPDecompressor - I'll try to add that.

But very possible that the error is not in letmegrpc, but protoc-gen-gofast (gogoprotobuf), as it handled non-CamelCase names very inconsistently...

tgulacsi avatar Sep 23 '16 14:09 tgulacsi

protoc-gen-gofast should generate almost exactly the same code protoc-gen-go Could you maybe show me a the diff between the two code generators' generated code and the source proto file?

awalterschulze avatar Sep 23 '16 18:09 awalterschulze

I haven't said that protoc-gen-go produced different code, just that I went back to it to get as close to a working example as I can.

I've included the generated .pb.go in db_dealer.zip in the first comment - for example, db_dealer__utolso_mod becomes "/db_dealer.db_dealer__utolso_mod/DbDealer_UtolsoMod" in its Handler's UnaryServerInfo, but in the Client, Invoke calls "/db_dealer.db_dealer__utolso_mod/db_dealer__utolso_mod", which does not match...

As I'm generating the .proto file, the change to use CamelCase is not a problem.

tgulacsi avatar Sep 23 '16 19:09 tgulacsi

How did you go back to get as close to a working example as possible? Did you change your message names? Why did you need to go back to protoc-gen-go vs protoc-gen-gofast?

awalterschulze avatar Sep 24 '16 07:09 awalterschulze

  1. I've replaced protoc-gen-gofast with protoc-gen-go, with no help,
  2. changed service and message names to be CamelCase - this helped.
  3. now I've tried protoc-gen-gofast, and this works, too!

Now I created a minimal example .proto file, and you're right, gofast and go both create the same naming inconsistencies: Client Invokes "/db_dealer.db_dealer/db_dealer__utolso_mod", but Handler uses "/db_dealer.db_dealer/DbDealer_UtolsoMod" in UnaryServerInfo.FullMethod. db_dealer-proto.zip

tgulacsi avatar Sep 24 '16 09:09 tgulacsi

Ok so now that you've contributed do you think you are ready to fix this as well?

awalterschulze avatar Sep 24 '16 09:09 awalterschulze

Aye aye, Sir! PTAL https://github.com/gogo/protobuf/issues/203

Walter Schulze [email protected] ezt írta (időpont: 2016. szept. 24., Szo, 11:39):

Ok so now that you've contributed do you think you are ready to fix this as well?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/gogo/letmegrpc/issues/25#issuecomment-249355738, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPoSnJRiQ3EkN0jlhL_JZ0XlLa1uGFtks5qtO-4gaJpZM4KBj2u .

tgulacsi avatar Sep 24 '16 11:09 tgulacsi

I think this fix should happen in letmegrpc and not in gogoprotobuf.

awalterschulze avatar Sep 24 '16 11:09 awalterschulze

But that inconsistency is in gogoprotobuf's gprc code generator!

tgulacsi avatar Sep 24 '16 12:09 tgulacsi

This code is consistently being generated in the same way by go_out, gofast_out and gogo_out

awalterschulze avatar Sep 24 '16 15:09 awalterschulze