proto: Add example.cpp to get working JSON serialization #3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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]>
Since the Go example is broken in this branch. See the previous commit for details. Signed-off-by: W. Trevor King <[email protected]>
With LinuxSpec, a parser will have to know ahead of time what sort of
config it's reading, any you'll end up with config JSON like:
{
"spec": { ...platform agnostic stuff ... },
"linux_config": { ...Linux-specific stuff... }
}
By dropping it and adding a LinuxConfig entry to Spec itself, we get
back to our Markdown-specified:
{
...platform agnostic stuff... ,
"linux": { ...Linux-specific stuff... }
}
As a minor (I hope) wrinkle, I've changed the field name from "linux"
to "linuxx" to avoid:
$ make
protoc --cpp_out=./cpp/ config.proto
pkg-config --cflags protobuf # fails if protobuf is not installed
-pthread
c++ example.cc ./cpp/config.pb.cc -o example_cpp $(pkg-config --cflags --libs protobuf) -I ./cpp
./cpp/config.pb.h:159:25: error: expected unqualified-id before numeric constant
const ::oci::Process& linux() const;
^
...
that seems tied to attributes named "linux". Hopefully someone will
figure out how to avoid that, in which case we can drop the silly
"linuxx" name.
Signed-off-by: W. Trevor King <[email protected]>
This mirror's the previous commit's change for config.proto, so see that commit message for an explaination. The "linuxx" hack works around the "expected unqualified-id before numeric constant" breakage, so we can restore cpp/runtime_config.pb.h in the Makefile rule. Signed-off-by: W. Trevor King <[email protected]>
So it matches (modulo key order in the process object and the @type addition required by Any) our examples from config.md. This is all successfully passed through example_cpp: $ diff -u config.json <(./example_cpp | jq .) $ echo $? 0 Signed-off-by: W. Trevor King <[email protected]>
So it matches (modulo key order) our examples from the
runtime-config.md series. To get as much matching up as possible,
I've converted externally-visible runtime_config.proto entries from
under_scores to camelCase. We're still not matching some cases well,
and I've interspersed my notes on why in the diff:
$ diff -u <(cat config.json runtime.json) <(./example_cpp | jq .)
--- /dev/fd/63 2015-09-27 00:04:18.464247761 -0700
+++ /dev/fd/62 2015-09-27 00:04:18.463247761 -0700
@@ -34,43 +34,9 @@
}
}
{
- "mounts": {
- "proc": {
- "type": "proc",
- "source": "proc",
- "options": []
- },
- ...
- },
+ "mounts": [
+ {}
+ ],
Protobuf doesn't like objects with arbitrary keys (or at least we
haven't found a way to set that up). So we probably need a
pre-processer to convert mount entries into:
"mounts": [
{
"key": "proc",
"value": {
"type": "proc",
"source": "proc",
"options": []
},
}
]
You can do that with jq [1], and that makes the mount difference much
smaller (sysctl need the same change):
$ mv runtime.json runtime.json-orig
$ jq '.mounts |= to_entries | .linuxx.sysctl |= to_entries' <runtime.json-orig >runtime.json
$ diff -u <(cat config.json runtime.json) <(./example_cpp | jq .)
--- /dev/fd/63 2015-09-27 00:15:56.656282533 -0700
+++ /dev/fd/62 2015-09-27 00:15:56.656282533 -0700
@@ -78,8 +78,7 @@
"key": "proc",
"value": {
"type": "proc",
- "source": "proc",
- "options": []
+ "source": "proc"
}
}
],
...
The difference here is that protobuf isn't serializing default values,
so it drops the empty array [2] (which I'd suggested we do anyway [3] ;)
...
"rlimits": [
{
"type": "RLIMIT_NPROC",
- "soft": 1024,
- "hard": 102400
+ "hard": "102400",
+ "soft": "1024"
}
],
...
Protobuf uses strings when writing 64-bit-wide numbers [2], but it
reads both numbers and strings, so this isn't a big deal.
@@ -198,63 +171,44 @@
"devices": [
{
"path": "/dev/random",
- "type": "c",
- "major": 1,
- "minor": 8,
- "permissions": "rwm",
- "fileMode": 666,
- "uid": 0,
- "gid": 0
+ "major": "1",
+ "minor": "8",
+ "permissions": "rwm",
+ "fileMode": 666
},
...
This has some of the default-dropping and number-string issues
mentioned earlier, as well as the effect of runtime_config.proto's:
// TODO(vbatts) ensure int32 is fine here, instead of golang's rune
optional int32 type = 2;
[1]: https://stedolan.github.io/jq/
[2]: https://developers.google.com/protocol-buffers/docs/proto3#json
[3]: opencontainers#123
Signed-off-by: W. Trevor King <[email protected]>
This was referenced Sep 27, 2015
| "process": { | ||
| "terminal": true, | ||
| "user": { | ||
| "@type": "type.googleapis.com/oci.LinuxUser", |
There was a problem hiding this comment.
Is type added by the json generation? Or is this something we have in the spec now? (I don't see it)
There was a problem hiding this comment.
On Wed, Sep 30, 2015 at 10:29:33AM -0700, Timothy Chen wrote:
"@type": "type.googleapis.com/oci.LinuxUser",Is type added by the json generation?
It's added by protobuf so it can figure out which message type to use
for deserializing Any fields. See 1.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Lots more details in the commit messages, but basically this gets us
working JSON (de)serialization for testing until we get protobuf-JSON
bindings working in Go (see #2).
Regardless of which language the eventual example lives in, I think
the Linux(Runtime)Spec drops from 9371318 and 83aff07 make sense
(motivation in the 9371318 commit message), although I'd love to have
a clean way to avoid my “linuxx” hack.
I don't care who shifts to close the under_score vs. camelCase gap (I
shifted runtime_config.proto in 5f422ee to avoid changing the Markdown
spec, but if we want to transition the Markdown spec to under_scores
I'm fine with that (we're still pre-1.0 ;).