-
Notifications
You must be signed in to change notification settings - Fork 0
proto: Port *.proto to proto3 #2
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
Conversation
Define syntax to fix [1]:
protoc --go_out=./go config.proto
[libprotobuf WARNING google/protobuf/compiler/parser.cc:492] No
syntax specified for the proto file. Please use 'syntax =
"proto2";' or 'syntax = "proto3";' to specify a syntax
version. (Defaulted to proto2 syntax.)
Drop 'optional' (with 'sed -i 's/\toptional /\t/' *.proto') to fix:
protoc --go_out=./go config.proto
config.proto:9:18: Explicit 'optional' labels are disallowed in the
Proto3 syntax. To define 'optional' fields in Proto3, simply
remove the 'optional' label, as fields are 'optional' by default.
Replace the User extensions with 'Any' [2,3] to fix:
protoc --go_out=./go config.proto
config.proto: Extensions in proto3 are only allowed for defining
options.
Drop required (with 'sed -i 's/\trequired /\t/' *.proto') to fix:
protoc --go_out=./go runtime_config.proto
runtime_config.proto: Required fields are not allowed in proto3.
Drop DefaultState to fix:
protoc --go_out=./go runtime_config.proto
runtime_config.proto: Explicit default values are not allowed in
proto3.
There's still some trouble with the resulting Go:
go run ./example.go
go/config.pb.go:26:8: cannot find package "google/protobuf" in any of:
/usr/lib/go/src/google/protobuf (from $GOROOT)
/home/wking/.local/lib/go/src/google/protobuf (from $GOPATH)
Makefile:31: recipe for target 'example' failed
But I haven't been able to figure that out yet.
[1]: https://developers.google.com/protocol-buffers/docs/proto3#simple
[2]: https://developers.google.com/protocol-buffers/docs/proto3#any
[3]: protocolbuffers/protobuf#828
Signed-off-by: W. Trevor King <[email protected]>
In a separate branch where I tried to handle Any in Go, I got [1]:
go run ./example.go
go/config.pb.go:26:8: cannot find package "google/protobuf" in any of:
/usr/lib/go/src/google/protobuf (from $GOROOT)
/home/wking/.local/lib/go/src/google/protobuf (from $GOPATH)
Makefile:31: recipe for target 'example' failed
This commit switches the main example to C++, because it's protobuf's
implementation language, so new features like Any support tend to land
there first.
The example tries to round-trip source JSON into a protobuf message
and back. It does that by reading from config.json (I've skipped
runtime.json for now) into a message, and then writing JSON serialized
from that message to stdout. Attributes in the input JSON file that
aren't represented in the protobuf message structure seem to be
silently dropped (which makes forward compatibility somewhat easier,
but explicit validation somewhat harder ;).
The docs for this sort of thing seem sparse (although there is a stale
C++ Any example here [2]). The source skeleton [3] and Makefile rule
[4] are based on the protobuf examples. kTypeUrlPrefix [5],
GetTypeUrl [6], the resolver handling [7,8], and the basic to/from
JSON conversion [9].
The commented-out LinuxUser section was how I figured out what to put
in the process.user block of config.json. The bits of that that
weren't in [10], I figured out just by reading proto/cpp/config.pb.h.
The Makefile rule should likely be using:
$(filter %.cc, $(CPP_FILES))
but my more restrictive filter removes runtime_config.pb.cc to avoid:
$ make example_cpp
c++ example.cc ./cpp/config.pb.cc ./cpp/runtime_config.pb.cc -o example_cpp $(pkg-config --cflags --libs protobuf) -I ./cpp
./cpp/runtime_config.pb.h:647:30: error: expected unqualified-id before numeric constant
const ::oci::LinuxRuntime& linux() const;
^
...
[1]: vbatts#2
[2]: https://developers.google.com/protocol-buffers/docs/proto3#any
Although it recommends:
status.add_details()->PackFrom(details);
And the actual usage would be:
status.mutable_details()->PackFrom(details);
See [10].
[3]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/examples/add_person.cc
[4]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/examples/Makefile#L24-L26
[5]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/src/google/protobuf/util/json_util_test.cc#L52
[6]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/src/google/protobuf/util/json_util_test.cc#L54-L56
[7]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/src/google/protobuf/util/json_util_test.cc#L66-L67
[8]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/src/google/protobuf/util/json_util_test.cc#L85
[9]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/src/google/protobuf/util/json_util_test.cc#L70-L83
[10]: https://github.com/google/protobuf/blob/v3.0.0-beta-1/src/google/protobuf/any_test.cc#L42
Signed-off-by: W. Trevor King <[email protected]>
Using jsonpb [1,2].
Drop the proto.String bits to avoid errors like:
./example.go:17: cannot use proto.String("linux") (type *string) as
type string in field value
And use a reference to s to avoid:
./example.go:29: cannot use s (type oci.LinuxSpec) as type
proto.Message in argument to marshaler.Marshal: oci.LinuxSpec
does not implement proto.Message (ProtoMessage method has pointer
receiver)
[1]: golang/protobuf#44
[2]: http://godoc.org/github.com/golang/protobuf/jsonpb
Signed-off-by: W. Trevor King <[email protected]>
To avoid:
go run ./example.go
go/config.pb.go:26:8: cannot find package "google/protobuf" in any of:
/usr/lib/go/src/google/protobuf (from $GOROOT)
/home/wking/.local/lib/go/src/google/protobuf (from $GOPATH)
Makefile:31: recipe for target 'example' failed
The Dockerfile changes will compile Google's any.proto to Go and
install it in the GOPATH. Unfortunately, the jsonpb rendering isn't
quite right, since it's spitting out:
"user": {
"type_url": "oci.LinuxUser",
"value": "qAYBsAYBuAYFuAYG"
},
When it should be spitting out [1]:
"user": {
"@type": "type.googleapis.com/oci.LinuxUser",
"uid": 1,
"gid": 1,
"additionalGids": [
5,
6
]
},
[1]: https://developers.google.com/protocol-buffers/docs/proto3#json
Signed-off-by: W. Trevor King <[email protected]>
|
I've figured out the ‘cannot find package "google/protobuf"’ error, and pushed da7a33f, which gives output like: It looks like golang/protobuf/jsonpb needs some more work to give us the expected: we see with the C++ example in #3. |
|
As an alternative to my Dockerfile changes in da7a33f, we could also use go.pedge.io/google-protobuf (see golang/protobuf#50). I don't care either way. |
|
@wking Note I'm trying to push to get go.pedge.io/google-protobuf just merged into golang/protobuf, I also have go.pedge.io/googleapis now, it's no fun to maintain, but I promise neither will go anywhere (and the go.pedge.io server is dead simple). Of note, I also have https://github.com/peter-edge/go-tools/tree/master/protoc-all (go get go.pedge.io/tools/protoc-all), which I'm happy to walk anyone through, I'm going to rewrite it soon as it's become a bit of a mess but it takes care of all the import paths (and uses go.pedge.io/google-protobuf). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if google is moving away from this, but the general protobuf standard has been to not use plural names for repeated fields, so this would be mount or mount_point.
|
Just made a few comments with some thoughts/suggestions, no worries but hoping they may be helpful :) |
It became the default with golang/protobuf@9ebc6c4e (2015-10-19) and was removed completely with golang/protobuf@8a5d8e8b (jsonpb: Remove Marshaler.EnumsAsString, 2015-10-21). Signed-off-by: W. Trevor King <[email protected]>
|
On Sat, Oct 24, 2015 at 10:44:59AM -0700, Peter Edge wrote:
They sound reasonable to me, but I'm trying to keep this branch as |
|
This is obsolete now. 😅 |
Define syntax to fix:
Drop
optional(withsed -i 's/\toptional /\t/' *.proto) to fix:Replace the
Userextensions withAny(see also,protocolbuffers/protobuf#828) to fix:
Drop
required(withsed -i 's/\trequired /\t/' *.proto) to fix:Drop
DefaultStateto fix:There's still some trouble with the resulting Go:
But I haven't been able to figure that out yet.