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

Fetch gRPC go v1.5.1 #1388

Merged
merged 8 commits into from
Jul 31, 2024
Merged

Fetch gRPC go v1.5.1 #1388

merged 8 commits into from
Jul 31, 2024

Conversation

oliversun9
Copy link
Contributor

@oliversun9 oliversun9 commented Jul 29, 2024

This fetches gRPC go v1.5.1 (skipping v1.5.0), updates the patch and re-enables auto fetch for this plugin.

Diff in patch:

$ diff plugins/grpc/go/v1.5.1/separate-package.patch  plugins/grpc/go/v1.4.0/separate-package.patch
2c2
< index abc21602..7d9dd638 100644
---
> index 9088bc2a..290d012e 100644
13c13
< @@ -136,8 +137,27 @@ func generateFile(gen *protogen.Plugin, file *protogen.File) *protogen.Generated
---
> @@ -132,8 +133,27 @@ func generateFile(gen *protogen.Plugin, file *protogen.File) *protogen.Generated
44c44
< index 183ba697..d0e1be60 100644
---
> index 0cb6a021..69055d3e 100644
47c47
< @@ -46,6 +46,7 @@ const version = "1.5.1"
---
> @@ -45,6 +45,7 @@ const version = "1.4.0"
56d55
<  
58a58
>       useGenericStreams = flags.Bool("use_generic_streams_experimental", false, "set to true to use generic types for streaming client and server objects; this flag is EXPERIMENTAL and may be changed or removed in a future release")
60d59
<       useGenericStreams = flags.Bool("use_generic_streams_experimental", true, "set to true to use generic types for streaming client and server objects; this flag is EXPERIMENTAL and may be changed or removed in a future release")
62a62
>               ParamFunc: flags.Set,

Diff between gPRC go v1.5.0 and v1.4.0

diff --git a/cmd/protoc-gen-go-grpc/README.md b/cmd/protoc-gen-go-grpc/README.md
index 711d20ed..95635ce7 100644
--- a/cmd/protoc-gen-go-grpc/README.md
+++ b/cmd/protoc-gen-go-grpc/README.md
@@ -19,10 +19,3 @@ E.g.:
 
 Note that this is not recommended, and the option is only provided to restore
 backward compatibility with previously-generated code.
-
-When embedding the `Unimplemented<ServiceName>Server` in a struct that
-implements the service, it should be embedded by _value_ instead of as a
-_pointer_.  If it is embedded as a pointer, it must be assigned to a valid,
-non-nil pointer or else unimplemented methods would panic when called.  This is
-tested at service registration time, and will lead to a panic in
-`Register<ServiceName>Server` if it is not embedded properly.
diff --git a/cmd/protoc-gen-go-grpc/go.mod b/cmd/protoc-gen-go-grpc/go.mod
index 7c21ffd6..4583705c 100644
--- a/cmd/protoc-gen-go-grpc/go.mod
+++ b/cmd/protoc-gen-go-grpc/go.mod
@@ -2,14 +2,4 @@ module google.golang.org/grpc/cmd/protoc-gen-go-grpc
 
 go 1.21
 
-require (
-       google.golang.org/grpc v1.65.0
-       google.golang.org/protobuf v1.34.1
-)
-
-require (
-       golang.org/x/net v0.26.0 // indirect
-       golang.org/x/sys v0.21.0 // indirect
-       golang.org/x/text v0.16.0 // indirect
-       google.golang.org/genproto/googleapis/rpc v0.0.0-20240604185151-ef581f913117 // indirect
-)
+require google.golang.org/protobuf v1.34.1
diff --git a/cmd/protoc-gen-go-grpc/go.sum b/cmd/protoc-gen-go-grpc/go.sum
index 2d5e359e..33bede0b 100644
--- a/cmd/protoc-gen-go-grpc/go.sum
+++ b/cmd/protoc-gen-go-grpc/go.sum
@@ -1,14 +1,6 @@
-github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
-github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
-golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ=
-golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE=
-golang.org/x/sys v0.21.0 h1:rF+pYz3DAGSQAxAu1CbC7catZg4ebC4UIeIhKxBZvws=
-golang.org/x/sys v0.21.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/text v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
-golang.org/x/text v0.16.0/go.mod h1:GhwF1Be+LQoKShO3cGOHzqOgRrGaYc9AvblQOmPVHnI=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20240604185151-ef581f913117 h1:1GBuWVLM/KMVUv1t1En5Gs+gFZCNd360GGb4sSxtrhU=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20240604185151-ef581f913117/go.mod h1:EfXuqaE1J41VCDicxHzUDm+8rk+7ZdXzHV0IhO/I6s0=
-google.golang.org/grpc v1.65.0 h1:bs/cUb4lp1G5iImFFd3u5ixQzweKizoZJAwBNLR42lc=
-google.golang.org/grpc v1.65.0/go.mod h1:WgYC2ypjlB0EiQi6wdKixMqukr6lBc0Vo+oOgjrM5ZQ=
+github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
+github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
+golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 google.golang.org/protobuf v1.34.1 h1:9ddQBjfCyZPOHPUiPxpYESBLc+T8P3E+Vo4IbKZgFWg=
 google.golang.org/protobuf v1.34.1/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos=
diff --git a/cmd/protoc-gen-go-grpc/grpc.go b/cmd/protoc-gen-go-grpc/grpc.go
index abc21602..9088bc2a 100644
--- a/cmd/protoc-gen-go-grpc/grpc.go
+++ b/cmd/protoc-gen-go-grpc/grpc.go
@@ -84,12 +84,9 @@ func (serviceGenerateHelper) generateUnimplementedServerType(gen *protogen.Plugi
                mustOrShould = "should"
        }
        // Server Unimplemented struct for forward compatibility.
-       g.P("// Unimplemented", serverType, " ", mustOrShould, " be embedded to have")
-       g.P("// forward compatible implementations.")
-       g.P("//")
-       g.P("// NOTE: this should be embedded by value instead of pointer to avoid a nil")
-       g.P("// pointer dereference when methods are called.")
-       g.P("type Unimplemented", serverType, " struct {}")
+       g.P("// Unimplemented", serverType, " ", mustOrShould, " be embedded to have forward compatible implementations.")
+       g.P("type Unimplemented", serverType, " struct {")
+       g.P("}")
        g.P()
        for _, method := range service.Methods {
                nilArg := ""
@@ -103,7 +100,6 @@ func (serviceGenerateHelper) generateUnimplementedServerType(gen *protogen.Plugi
        if *requireUnimplemented {
                g.P("func (Unimplemented", serverType, ") mustEmbedUnimplemented", serverType, "() {}")
        }
-       g.P("func (Unimplemented", serverType, ") testEmbeddedByValue() {}")
        g.P()
 }
 
@@ -268,7 +264,7 @@ func genService(gen *protogen.Plugin, file *protogen.File, g *protogen.Generated
        serverType := service.GoName + "Server"
        g.P("// ", serverType, " is the server API for ", service.GoName, " service.")
        g.P("// All implementations ", mustOrShould, " embed Unimplemented", serverType)
-       g.P("// for forward compatibility.")
+       g.P("// for forward compatibility")
 
        // Copy comments from proto file.
        genServiceComments(g, service)
@@ -310,13 +306,6 @@ func genService(gen *protogen.Plugin, file *protogen.File, g *protogen.Generated
        }
        serviceDescVar := service.GoName + "_ServiceDesc"
        g.P("func Register", service.GoName, "Server(s ", grpcPackage.Ident("ServiceRegistrar"), ", srv ", serverType, ") {")
-       g.P("// If the following call pancis, it indicates Unimplemented", serverType, " was")
-       g.P("// embedded by pointer and is nil.  This will cause panics if an")
-       g.P("// unimplemented method is ever invoked, so we test this at initialization")
-       g.P("// time to prevent it from happening at runtime later due to I/O.")
-       g.P("if t, ok := srv.(interface { testEmbeddedByValue() }); ok {")
-       g.P("t.testEmbeddedByValue()")
-       g.P("}")
        g.P("s.RegisterService(&", serviceDescVar, `, srv)`)
        g.P("}")
        g.P()
diff --git a/cmd/protoc-gen-go-grpc/main.go b/cmd/protoc-gen-go-grpc/main.go
index 183ba697..0cb6a021 100644
--- a/cmd/protoc-gen-go-grpc/main.go
+++ b/cmd/protoc-gen-go-grpc/main.go
@@ -38,11 +38,10 @@ import (
        "fmt"
 
        "google.golang.org/protobuf/compiler/protogen"
-       "google.golang.org/protobuf/types/descriptorpb"
        "google.golang.org/protobuf/types/pluginpb"
 )
 
-const version = "1.5.1"
+const version = "1.4.0"
 
 var requireUnimplemented *bool
 var useGenericStreams *bool
@@ -57,14 +56,12 @@ func main() {
 
        var flags flag.FlagSet
        requireUnimplemented = flags.Bool("require_unimplemented_servers", true, "set to false to match legacy behavior")
-       useGenericStreams = flags.Bool("use_generic_streams_experimental", true, "set to true to use generic types for streaming client and server objects; this flag is EXPERIMENTAL and may be changed or removed in a future release")
+       useGenericStreams = flags.Bool("use_generic_streams_experimental", false, "set to true to use generic types for streaming client and server objects; this flag is EXPERIMENTAL and may be changed or removed in a future release")
 
        protogen.Options{
                ParamFunc: flags.Set,
        }.Run(func(gen *protogen.Plugin) error {
-               gen.SupportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL) | uint64(pluginpb.CodeGeneratorResponse_FEATURE_SUPPORTS_EDITIONS)
-               gen.SupportedEditionsMinimum = descriptorpb.Edition_EDITION_PROTO2
-               gen.SupportedEditionsMaximum = descriptorpb.Edition_EDITION_2023
+               gen.SupportedFeatures = uint64(pluginpb.CodeGeneratorResponse_FEATURE_PROTO3_OPTIONAL)
                for _, f := range gen.Files {
                        if !f.Generate {
                                continue
diff --git a/cmd/protoc-gen-go-grpc/unimpl_test.go b/cmd/protoc-gen-go-grpc/unimpl_test.go
deleted file mode 100644
index 4938ec91..00000000
--- a/cmd/protoc-gen-go-grpc/unimpl_test.go
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- *
- * Copyright 2024 gRPC authors.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- *
- */
-
-package main_test
-
-import (
-       "testing"
-
-       "google.golang.org/grpc"
-       testgrpc "google.golang.org/grpc/interop/grpc_testing"
-)
-
-type unimplEmbeddedByPointer struct {
-       *testgrpc.UnimplementedTestServiceServer
-}
-
-type unimplEmbeddedByValue struct {
-       testgrpc.UnimplementedTestServiceServer
-}
-
-func TestUnimplementedEmbedding(t *testing.T) {
-       t.Skip("Skip until next grpc release includes panic during registration.")
-
-       // Embedded by value, this should succeed.
-       testgrpc.RegisterTestServiceServer(grpc.NewServer(), &unimplEmbeddedByValue{})
-       defer func() {
-               if recover() == nil {
-                       t.Fatalf("Expected panic; received none")
-               }
-       }()
-
-       // Embedded by pointer, this should panic.
-       testgrpc.RegisterTestServiceServer(grpc.NewServer(), &unimplEmbeddedByPointer{})
-}

@oliversun9 oliversun9 requested a review from mfridman July 29, 2024 14:53
Co-authored-by: Michael Fridman <mfridman@buf.build>
@oliversun9 oliversun9 requested a review from mfridman July 29, 2024 15:01
oliversun9 and others added 2 commits July 30, 2024 11:12
Co-authored-by: Philip K. Warren <pkwarren@users.noreply.github.com>
@mfridman
Copy link
Member

Can update this to v1.5.1, they did a release yesterday

https://github.com/grpc/grpc-go/releases/tag/cmd%2Fprotoc-gen-go-grpc%2Fv1.5.1

@oliversun9 oliversun9 changed the title Fetch gRPC go v1.5.0 Fetch gRPC go v1.5.1 Jul 30, 2024
@oliversun9 oliversun9 merged commit 7cb8142 into main Jul 31, 2024
7 checks passed
@oliversun9 oliversun9 deleted the osun/fetch-grpc-go-v1.5.0 branch July 31, 2024 15:19
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

Successfully merging this pull request may close these issues.

3 participants