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

Upgrade protobuf to 1.4.0 #1165

Merged
merged 1 commit into from Apr 22, 2020
Merged

Upgrade protobuf to 1.4.0 #1165

merged 1 commit into from Apr 22, 2020

Conversation

johanbrandhorst
Copy link
Collaborator

No description provided.

@johanbrandhorst johanbrandhorst force-pushed the upgrade-protobuf branch 2 times, most recently from 556f48d to 9214d79 Compare March 9, 2020 21:34
@johanbrandhorst
Copy link
Collaborator Author

Yeah so this might be a no-go until we can do a v2. Basically all the proto packages in the public API have incompatible changes, and some function signatures are now incompatible:
https://app.circleci.com/jobs/github/grpc-ecosystem/grpc-gateway/5804

github.com/grpc-ecosystem/grpc-gateway/runtime
----------------------------------------------
Incompatible changes:
- (*ServeMux).GetForwardResponseOptions: changed from func() []func(context.Context, net/http.ResponseWriter, github.com/golang/protobuf/proto.Message) error to func() []func(context.Context, net/http.ResponseWriter, google.golang.org/protobuf/runtime/protoiface.MessageV1) error
- ForwardResponseMessage: changed from func(context.Context, *ServeMux, Marshaler, net/http.ResponseWriter, *net/http.Request, github.com/golang/protobuf/proto.Message, ...func(context.Context, net/http.ResponseWriter, github.com/golang/protobuf/proto.Message) error) to func(context.Context, *ServeMux, Marshaler, net/http.ResponseWriter, *net/http.Request, google.golang.org/protobuf/runtime/protoiface.MessageV1, ...func(context.Context, net/http.ResponseWriter, google.golang.org/protobuf/runtime/protoiface.MessageV1) error)
- ForwardResponseStream: changed from func(context.Context, *ServeMux, Marshaler, net/http.ResponseWriter, *net/http.Request, func() (github.com/golang/protobuf/proto.Message, error), ...func(context.Context, net/http.ResponseWriter, github.com/golang/protobuf/proto.Message) error) to func(context.Context, *ServeMux, Marshaler, net/http.ResponseWriter, *net/http.Request, func() (google.golang.org/protobuf/runtime/protoiface.MessageV1, error), ...func(context.Context, net/http.ResponseWriter, google.golang.org/protobuf/runtime/protoiface.MessageV1) error)
- PopulateFieldFromPath: changed from func(github.com/golang/protobuf/proto.Message, string, string) error to func(google.golang.org/protobuf/runtime/protoiface.MessageV1, string, string) error
- PopulateQueryParameters: changed from func(github.com/golang/protobuf/proto.Message, net/url.Values, *github.com/grpc-ecosystem/grpc-gateway/utilities.DoubleArray) error to func(google.golang.org/protobuf/runtime/protoiface.MessageV1, net/url.Values, *github.com/grpc-ecosystem/grpc-gateway/utilities.DoubleArray) error
- QueryParameterParser.Parse: changed from func(github.com/golang/protobuf/proto.Message, net/url.Values, *github.com/grpc-ecosystem/grpc-gateway/utilities.DoubleArray) error to func(google.golang.org/protobuf/runtime/protoiface.MessageV1, net/url.Values, *github.com/grpc-ecosystem/grpc-gateway/utilities.DoubleArray) error
- StreamError.XXX_NoUnkeyedLiteral: removed
- StreamError.XXX_sizecache: removed
- StreamError.XXX_unrecognized: removed
- WithForwardResponseOption: changed from func(func(context.Context, net/http.ResponseWriter, github.com/golang/protobuf/proto.Message) error) ServeMuxOption to func(func(context.Context, net/http.ResponseWriter, google.golang.org/protobuf/runtime/protoiface.MessageV1) error) ServeMuxOption

etc.

@johanbrandhorst
Copy link
Collaborator Author

Rebased on v2

@johanbrandhorst johanbrandhorst changed the base branch from master to v2 April 18, 2020 23:48
@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@johanbrandhorst johanbrandhorst force-pushed the upgrade-protobuf branch 2 times, most recently from b384f3a to 60c365f Compare April 19, 2020 13:48
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@johanbrandhorst
Copy link
Collaborator Author

@achew22 @drigz can't understand why bazel is broken, the two recent errors seem in conflict:

INFO: Build completed successfully, 50 total actions
--- runtime/BUILD.bazel	1970-01-01 00:00:00.000000000 +0000
+++ runtime/BUILD.bazel	1970-01-01 00:00:00.000000000 +0000
@@ -66,11 +66,11 @@
     deps = [
         "//internal:go_default_library",
         "//utilities:go_default_library",
-        "//runtime/internal/examplepb:go_default_library",
         "@com_github_golang_protobuf//jsonpb:go_default_library_gen",
         "@com_github_golang_protobuf//proto:go_default_library",
         "@com_github_golang_protobuf//ptypes:go_default_library_gen",
         "@com_github_google_go_cmp//cmp:go_default_library",
+        "@com_github_grpc_ecosystem_grpc_gateway_v2//runtime/internal/examplepb:go_default_library",
         "@go_googleapis//google/api:httpbody_go_proto",
         "@go_googleapis//google/rpc:errdetails_go_proto",
         "@io_bazel_rules_go//proto/wkt:duration_go_proto",
ERROR: Bazel files out-of-date, please run `bazel run :gazelle

https://app.circleci.com/pipelines/github/grpc-ecosystem/grpc-gateway/168/workflows/f798c1c0-14e0-4053-b8cc-762cdeb315c7/jobs/6748

Analyzing: 61 targets (27 packages loaded, 0 targets configured)
ERROR: /src/grpc-gateway/runtime/BUILD.bazel:47:1: no such package '@com_github_grpc_ecosystem_grpc_gateway_v2//runtime/internal/examplepb': The repository '@com_github_grpc_ecosystem_grpc_gateway_v2' could not be resolved and referenced by '//runtime:go_default_test'
ERROR: Analysis of target '//runtime:go_default_test' failed; build aborted: no such package '@com_github_grpc_ecosystem_grpc_gateway_v2//runtime/internal/examplepb': The repository '@com_github_grpc_ecosystem_grpc_gateway_v2' could not be resolved
INFO: Elapsed time: 0.378s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (30 packages loaded, 10 targets configured)

https://app.circleci.com/pipelines/github/grpc-ecosystem/grpc-gateway/171/workflows/24f87f86-75cc-44bd-a0bb-58e4c7b28059/jobs/6770

Any ideas?

@achew22
Copy link
Collaborator

achew22 commented Apr 19, 2020

@johanbrandhorst I would bet you it is because Gazelle needs to be told the correct new prefix.

# gazelle:prefix github.com/grpc-ecosystem/grpc-gateway

@johanbrandhorst
Copy link
Collaborator Author

@johanbrandhorst I would bet you it is because Gazelle needs to be told the correct new prefix.

# gazelle:prefix github.com/grpc-ecosystem/grpc-gateway

I thought I updated this on v2 already 🤔

@johanbrandhorst
Copy link
Collaborator Author

Yeah that has already been fixed on v2: https://github.com/grpc-ecosystem/grpc-gateway/blob/v2/BUILD#L16

@johanbrandhorst johanbrandhorst mentioned this pull request Apr 20, 2020
13 tasks
@johanbrandhorst johanbrandhorst force-pushed the upgrade-protobuf branch 2 times, most recently from f60161f to de61178 Compare April 22, 2020 08:51
@johanbrandhorst
Copy link
Collaborator Author

@achew22 @drigz I'd still really appreciate help figuring out the bazel error. I don't understand what is wrong.

@drigz
Copy link
Contributor

drigz commented Apr 22, 2020

Looks like the Go importpath inside runtime/internal/examplepb is incorrect. drigz@f15ae7f

@johanbrandhorst
Copy link
Collaborator Author

Looks like the Go importpath inside runtime/internal/examplepb is incorrect. drigz@f15ae7f

Thank you so much Rodrigo! I'll fix it.

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

Successfully merging this pull request may close these issues.

None yet

4 participants