Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protoc-gen-gofast generates different names #25

Open
tgulacsi opened this issue Sep 20, 2016 · 11 comments
Open

protoc-gen-gofast generates different names #25

tgulacsi opened this issue Sep 20, 2016 · 11 comments

Comments

@tgulacsi
Copy link
Contributor

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)
}
@awalterschulze
Copy link
Member

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.

@tgulacsi
Copy link
Contributor Author

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

@awalterschulze
Copy link
Member

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?

@tgulacsi
Copy link
Contributor Author

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.

@awalterschulze
Copy link
Member

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?

@tgulacsi
Copy link
Contributor Author

  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

@awalterschulze
Copy link
Member

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

@tgulacsi
Copy link
Contributor Author

Aye aye, Sir!
PTAL gogo/protobuf#203

Walter Schulze notifications@github.com 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
#25 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAPoSnJRiQ3EkN0jlhL_JZ0XlLa1uGFtks5qtO-4gaJpZM4KBj2u
.

@awalterschulze
Copy link
Member

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

@tgulacsi
Copy link
Contributor Author

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

@awalterschulze
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants