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

Well Known Types not supported by gofast_out #325

Open
utrack opened this issue Aug 16, 2017 · 17 comments
Open

Well Known Types not supported by gofast_out #325

utrack opened this issue Aug 16, 2017 · 17 comments
Labels

Comments

@utrack
Copy link

@utrack utrack commented Aug 16, 2017

Hi! I've got a protofile like that:

syntax = "proto3";
package pb;

import "google/api/annotations.proto";
import "google/protobuf/timestamp.proto";
...

that is compiled using gofast:
protoc -I/usr/local/include -I. -I /home/u/go/src -I vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --proto_path . --proto_path vendor/ --gofast_out=plugins=grpc:. --goclay_out=:. endpoint/pb/*.proto

I'm getting these errors:

» go build ./endpoint                                                                                                                        2017-08-16 18:41:50
# listing_api/endpoint/pb
endpoint/pb/product.pb.go:2727: m.SpecialFromDate.Size undefined (type *timestamp.Timestamp has no field or method Size)
endpoint/pb/product.pb.go:2728: m.SpecialFromDate.MarshalTo undefined (type *timestamp.Timestamp has no field or method MarshalTo)
endpoint/pb/product.pb.go:2751: m.SpecialToDate.Size undefined (type *timestamp.Timestamp has no field or method Size)
endpoint/pb/product.pb.go:2752: m.SpecialToDate.MarshalTo undefined (type *timestamp.Timestamp has no field or method MarshalTo)
endpoint/pb/product.pb.go:2814: m.SpecialFromDate.Size undefined (type *timestamp.Timestamp has no field or method Size)
endpoint/pb/product.pb.go:2815: m.SpecialFromDate.MarshalTo undefined (type *timestamp.Timestamp has no field or method MarshalTo)
endpoint/pb/product.pb.go:2834: m.SpecialToDate.Size undefined (type *timestamp.Timestamp has no field or method Size)
endpoint/pb/product.pb.go:2835: m.SpecialToDate.MarshalTo undefined (type *timestamp.Timestamp has no field or method MarshalTo)
endpoint/pb/product.pb.go:2874: m.StartDate.Size undefined (type *timestamp.Timestamp has no field or method Size)
endpoint/pb/product.pb.go:2875: m.StartDate.MarshalTo undefined (type *timestamp.Timestamp has no field or method MarshalTo)
endpoint/pb/product.pb.go:2875: too many errors

in generated file there's import
import google_protobuf1 "github.com/golang/protobuf/ptypes/timestamp"
It'll compile if I'd change it to
import google_protobuf1 "github.com/gogo/protobuf/types".
I suspect there's a problem w/ my imports. What should I change?

@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Aug 16, 2017

Yes for timestamp you will be required to use gogofast.

If you would like to help by providing a pull request to support timestamp in gofast, that will be more than welcome.

But I guess that most users of gofast will move over to gogofaster and gogoslick and eventually even more custom usages very quickly.
Maybe I am wrong though and support for timestamp in gofast would be extremely valuable.

@utrack
Copy link
Author

@utrack utrack commented Aug 16, 2017

Ah, I see :) thank you!
We can't switch over to gogo* sadly since it breaks gRPC reflection and enums encoder in vanilla pbjson due to different registries.
I'll see what I can do in the spare time.

@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Aug 16, 2017

@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Aug 16, 2017

@utrack
Copy link
Author

@utrack utrack commented Aug 17, 2017

Thanks, I'll try that.
I've just tried the same command (same imports etc) with gogofast though and got same compilation errors...

@utrack
Copy link
Author

@utrack utrack commented Aug 17, 2017

When I do
protoc -I/usr/local/include -I. -I ./vendor -I vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --proto_path . --proto_path vendor/github.com/gogo/protobuf/protobuf --gogofast_out=goproto_registration=true,plugins=grpc:. --goclay_out=:. endpoint/pb/*.proto
(i.e. importing gogo proto/proto) it generates even worse import - import google_protobuf1 "google/protobuf"

@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Aug 17, 2017

I didn't know you could do --gogofast_out=goproto_registration=true in the command line? I thought you had to add it as an extension in the proto file. Are you sure this works?

I forgot that you also need to import the timestamp.proto from the gogoprotobuf repo. It is binary compatible with the protoc timestamp.proto
https://github.com/gogo/protobuf/tree/master/protobuf/google/protobuf

Hopefully this helps?

@utrack
Copy link
Author

@utrack utrack commented Aug 18, 2017

Did it like that:

PROTO_MAKER := protoc \
		-I/usr/local/include -I. \
		-I vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
	--proto_path vendor/github.com/gogo/protobuf/protobuf \
	--proto_path endpoint/pb \
	--gogofast_out=plugins=grpc:. \
	--goclay_out=:.

Import is google/protobuf/timestamp.proto. Generation is fine, but it doesn't compile:

endpoint/pb/product.pb.go:9:8: cannot find package "google/protobuf" in any of:
        /home/u/go/src/listing_api/vendor/google/protobuf (vendor tree)
        /usr/lib/go/src/google/protobuf (from $GOROOT)
        /home/u/go/src/google/protobuf (from $GOPATH)
@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Aug 18, 2017

Could you try adding M parameters like in this test
https://github.com/gogo/protobuf/blob/master/test/types/Makefile

@utrack
Copy link
Author

@utrack utrack commented Aug 18, 2017

Thank you! Works with that one.
I'm leaving the issue open since gofast still doesn't work - but I'll try to fix that if nobody else picks that up.

@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Aug 18, 2017

I agree, this issue should stay open.

I would welcome a pull request. I just warn you that its not a small amount of work, but its doable.

If you would like to maybe give your final command line, that might help future developers with the same problem. If you could also include and example proto file especially with the imports, I think that would also be of great help.

@GeertJohan
Copy link

@GeertJohan GeertJohan commented Oct 9, 2017

I found this thread useful, as I had the same problem.
For future readers, this is the protoc command in my Makefile. Note that the M… and plugin=grpc options cannot have indentation/whitespace.

	protoc -I. \
		-I$(GOPATH)/src/github.com/gogo/protobuf/protobuf \
		-I$(GOPATH)/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
		--gogofast_out=\
Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/struct.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/wrappers.proto=github.com/gogo/protobuf/types,\
plugins=grpc:. \
		--grpc-gateway_out=logtostderr=true:. \
		*.proto
@bcessa
Copy link

@bcessa bcessa commented Jan 16, 2018

I'm managing dependencies using dep; related to that I was having some issues with the code generated by --grpc-gateway_out

proto/broker.pb.gw.go:12:2: cannot find package "google/protobuf" in any of:
	.../go/src/github.com/fairbank-io/platform/vendor/google/protobuf (vendor tree)
	/usr/local/Cellar/go/1.9.2/libexec/src/google/protobuf (from $GOROOT)
	.../go/src/google/protobuf (from $GOPATH)
make: *** [build] Error 1

I leave here the Make configuration I'm using to solve it in case is useful.

proto: ## Compile protocol buffers and RPC services
	@protoc \
	-I=./vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis \
	-I=./vendor/github.com/gogo/protobuf/protobuf \
	-I=./vendor/github.com/gogo/protobuf \
	-I=./vendor \
	-I=. \
	--gogofast_out=\
Mgogoproto/gogo.proto=github.com/gogo/protobuf/gogoproto,\
Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/field_mask.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/struct.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/wrappers.proto=github.com/gogo/protobuf/types,\
plugins=grpc:. \
	--grpc-gateway_out=\
Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/empty.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/field_mask.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/struct.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types,\
Mgoogle/protobuf/wrappers.proto=github.com/gogo/protobuf/types,\
logtostderr=true:. \
	-oproto/service.desc \
	proto/*.proto
@awalterschulze
Copy link
Member

@awalterschulze awalterschulze commented Jan 17, 2018

Could you include the go file or the imports of the go file that is causing the compilation error?

@johanbrandhorst
Copy link
Member

@johanbrandhorst johanbrandhorst commented Feb 18, 2018

For reference, I don't think importing the gogo/protobuf/types fixes the reflection issues, as the types are not registered with golang/protobuf.

@huyntsgs
Copy link

@huyntsgs huyntsgs commented Mar 14, 2018

You should include Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types as --gogofaster_out=Mgoogle/protobuf/timestamp.proto=github.com/gogo/protobuf/types, ...

Also, add -I=path of directory contains timestamp.proto, on window is protoc-3.5.1-win32\include\google\protobuf

@awalterschulze awalterschulze changed the title can't compile go code using google.protobuf.Timestamp Well Known Types not supported by gofast_out Sep 20, 2018
@johanbrandhorst
Copy link
Member

@johanbrandhorst johanbrandhorst commented Apr 3, 2019

I wonder if there would be some value to transparently falling back to the standard marshalling strategy for marshalling types. There's a real interest in the community in getting the benefits of custom code marshalling for your own code while staying as compatible as possible with golang/protobuf. Would it be possible for the gofast code to fall back to the standard marshalling strategy for nested types that do not have these methods?

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

Successfully merging a pull request may close this issue.

None yet
6 participants