Skip to content
New issue

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

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

Already on GitHub? # to your account

Migrate away from using the protoc-gen-go/generator package #1209

Closed
johanbrandhorst opened this issue Apr 14, 2020 · 10 comments · Fixed by #1260
Closed

Migrate away from using the protoc-gen-go/generator package #1209

johanbrandhorst opened this issue Apr 14, 2020 · 10 comments · Fixed by #1260

Comments

@johanbrandhorst
Copy link
Collaborator

This is internal and will be deprecated in a future release of golang/protobuf

@nashikb
Copy link

nashikb commented Apr 17, 2020

Yes please.

pull bot pushed a commit to BuildingRobotics/grpc-gateway that referenced this issue Apr 29, 2020
@Greg0x01
Copy link

Hi @johanbrandhorst ,
after getting warning WARNING: Package "github.com/golang/protobuf/protoc-gen-go/generator" is deprecated. I've found this issue and tested lastest master version and get same warning. I've found other places where deprecated package is still imported:

egrep -iR 'github.com/golang/protobuf/protoc-gen-go/generator' .

./protoc-gen-grpc-gateway/internal/gengateway/template.go:      generator2 "github.com/golang/protobuf/protoc-gen-go/generator"
./protoc-gen-grpc-gateway/descriptor/types.go:  gogen "github.com/golang/protobuf/protoc-gen-go/generator"
./examples/internal/server/fieldmask_helper.go: "github.com/golang/protobuf/protoc-gen-go/generator"

Should these places migrate away from package github.com/golang/protobuf/protoc-gen-go/generator too?

I'm concerned with case of a future release of golang/protobuf that will delete it and break go build process.

@johanbrandhorst
Copy link
Collaborator Author

This is indeed not resolved yet. We'd be happy to accept contributions to remove these dependencies.

@johanbrandhorst
Copy link
Collaborator Author

The case of migrating from github.com/golang/protobuf/protoc-gen-go/generator is going to be more complicated, but also something we want to do long term.

@achew22
Copy link
Collaborator

achew22 commented Apr 30, 2020

Since we have a partial implementation, could we move it into our internal/generator package and then change the imports?

@johanbrandhorst
Copy link
Collaborator Author

Sounds good 👍. We should be able to copy parts of the dependency as long as we include the LICENSE I believe?

@achew22
Copy link
Collaborator

achew22 commented Apr 30, 2020

Step one is always collect data:

$ ag github.com/golang/protobuf/protoc-gen-go/generator
protoc-gen-grpc-gateway/internal/gengateway/template.go
11:     generator2 "github.com/golang/protobuf/protoc-gen-go/generator"

protoc-gen-grpc-gateway/descriptor/types.go
8:      gogen "github.com/golang/protobuf/protoc-gen-go/generator"

examples/internal/server/fieldmask_helper.go
8:      "github.com/golang/protobuf/protoc-gen-go/generator"

Let's see what the usage looks like.

$ ag generator2 protoc-gen-grpc-gateway/internal/gengateway/template.go
11:     generator2 "github.com/golang/protobuf/protoc-gen-go/generator"
41:             return generator2.CamelCase(b.Body.FieldPath.String()), nil
125:            return generator2.CamelCase(fieldMaskField.GetName())
159:            msgName := generator2.CamelCase(*msg.Name)
164:            svcName := generator2.CamelCase(*svc.Name)
168:                    methName := generator2.CamelCase(*meth.Name)

$ ag gogen protoc-gen-grpc-gateway/descriptor/types.go
8:      gogen "github.com/golang/protobuf/protoc-gen-go/generator"
319:                    oneOfName := gogen.CamelCase(msg.GetOneofDecl()[*index].GetName())
355:    return gogen.CamelCase(c.Name)
361:            return fmt.Sprintf("Get%s()", gogen.CamelCase(c.Name))
363:    return gogen.CamelCase(c.Name)

$ ag generator examples/internal/server/fieldmask_helper.go
8:      "github.com/golang/protobuf/protoc-gen-go/generator"
43:             v = v.FieldByName(generator.CamelCase(s))

Looks like we have the following breakdown of usage:

  • 5 * generator.CamelCase (template.go)
  • 4 * generator.CamelCase (types.go)
  • 1 * generator.CamelCase (fieldmask_helper.go)

We already have an implementation of CamelCase in [protoc-gen-swagger/genswagger/template.go](https://github.com/grpc-ecosystem/grpc-gateway/pull/1213/files#diff-d419038d63db0fdabc61f0d6cc6683aeR2002). Is there any reason we can't move that implementation into a root level internal` package and then just invoke it and remove our dependency entirely?

@johanbrandhorst
Copy link
Collaborator Author

I must've confused it with something else, that sounds straightforward enough.

@eddycjy
Copy link

eddycjy commented May 5, 2020

@johanbrandhorst Is there any recent release plan for this issue?

@johanbrandhorst
Copy link
Collaborator Author

We'll make a minor version release soon to get this out 😃

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

Successfully merging a pull request may close this issue.

5 participants