uber / prototool Goto Github PK
View Code? Open in Web Editor NEWYour Swiss Army Knife for Protocol Buffers
License: MIT License
Your Swiss Army Knife for Protocol Buffers
License: MIT License
My plugin emits a warning to stderr, but exits with code 0 and no Error
in the MessageResponse
- however, prototool
treats it as unrecoverable error.
prototool all "endpoint/"
2018-06-24T22:11:33.431+0300 WARN protoc returned a line we do not understand, please file this as an issue at https://github.com/uber/prototool/issues/new {"protocLine": "I0624 22:11:33.429160 27601 handler.go:81] service/itemapi/itemapipb/client.proto: no target service defined in the file"}
<input>:1:1:I0624 22:11:33.429160 27601 handler.go:81] service/itemapi/itemapipb/client.proto: no target service defined in the file
make: *** [Makefile:54: gen] Error 255
We probably want to outlaw enum aliases as part of linting by default, but this is a discussion item https://developers.google.com/protocol-buffers/docs/proto3#enum
The internal/text
package is primarily used in Prototool's proto compilation and lint processes. Seeing as this package is mostly concerned with instantiating text.Failure
, we should standardize how they are represented.
#87 proposes several changes that should be considered at a later time. Specifically,
text
to failure
(motivated by https://blog.golang.org/package-names)failure.Type
to differentiate failures found in protoc vs. linttext.Failure.String
-- if not required)failure.LintIdentifier
enum so that lint IDs are not free-form strings.These items can be largely ignored for the time being, but should be iterated on as we continue to develop out the protoc and lint processes.
There's a lot of strings.HasPrefix
and fmt.Sprintf("%s/%s", some, path)
lying around Prototool that needs to be cleaned up into filepath.Join
etc calls. It's not that difficult, but just needs to be done.
I would like to override the following package related configuration included in uber.proto
and used by default by the prototool
// The go package is always $(basename PACKAGE)pb.
// Do not use the "long-form" package name with a directory path.
option go_package = "uberpb";
// The java package is always com.PACKAGE.pb.
option java_package = "com.style.uber.pb";
Documentation mentions that
You can add or exclude lint rules in your prototool.yaml file.
but prototool.yaml
has no configuration regarding linting of package names.
How can I do this?
To get this off the ground, I did what I was used to and used Glide as my dependency management tool. I wanted this to be go-gettable, so all this does is specify all necessary dependencies without versions, which are then checked into vendor
. This has been fine for development, as Prototool does not expose any external API, and this makes installing Prototool as easy as go get github.com/uber/prototool/cmd/prototool
.
However, we eventually will want to expose parts of Prototool as a Golang library, and this isn't a very efficient way to do this. We probably don't want to check in vendor
in this case, and we want to get up to date by using Dep. Installing Prototool will instead move to curl'ing binaries released to GitHub Releases.
Update: No longer checking in vendor, so probably all that is left here is to move from Glide to Dep.
Some of the lint options imply some sort of technical limitation, for example MESSAGE_FIELDS_NOT_FLOATS
. I think the package should define a reference for the lint options and a rationale for each option so the user can make an informed decision about which ones to include.
When running prototool format
on the following proto:
syntax = "proto3";
import "protoc-gen-swagger/options/annotations.proto";
option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = {
info: {
version: "1.0"
}
external_docs: {}
schemes: HTTPS
};
It produces the following output:
syntax = "proto3";
import "protoc-gen-swagger/options/annotations.proto";
option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = {
info: {
version: "1.0"
}
external_docs:
schemes: HTTPS
};
This does not compile with protoc
, instead giving the error:
$
test.proto: Error while parsing option value for "openapiv2_swagger": Expected "{", found "schemes".
The correct thing here would be to leave the {}
.
I'm not entirely sure why this is failing. I have installed prototool in /usr/local/bin
and the command prototool -h
works. However attempting to build the examples does not.
go install \
-ldflags "-X 'github.com/uber/prototool/internal/x/vars.GitCommit=db0d67f111a6bf4731d904e86b94fb39bd72cf3c' -X 'github.com/uber/prototool/internal/x/vars.BuiltTimestamp=Wed 11 Apr 2018 20:01:07 UTC'" \
github.com/uber/prototool/cmd/prototool
can't load package: package github.com/uber/prototool/cmd/prototool: cannot find package "github.com/uber/prototool/cmd/prototool" in any of:
/usr/local/Cellar/go/1.9/libexec/src/github.com/uber/prototool/cmd/prototool (from $GOROOT)
/Users/jameshar/go/src/github.com/uber/prototool/cmd/prototool (from $GOPATH)
make: *** [install] Error 1
brew update
and brew upgrade
reports that everything is upto date. I have protobuf 3.5.1 installed using brew.
There's a long list of commands that prototool
provides, some of which we probably should not have for v1.0. The full list:
$ prototool help
Usage:
prototool [command]
Available Commands:
all Compile, then format and overwrite, then re-compile and generate, then lint, stopping if any step fails.
binary-to-json Convert the data from json to binary for the message path and data.
clean Delete the cache.
compile Compile with protoc to check for failures.
descriptor-proto Get the descriptor proto for the message path.
download Downlond the protobuf artifacts to a cache.
field-descriptor-proto Get the field descriptor proto for the field path.
files Print all files that match the input arguments.
format Format a proto file and compile with protoc to check for failures.
gen Generate with protoc.
grpc Call a gRPC endpoint.
help Help about any command
init Generate an initial config file in the current or given directory.
json-to-binary Convert the data from json to binary for the message path and data.
lint Lint proto files and compile with protoc to check for failures.
list-all-lint-groups List all the available lint groups.
list-all-linters List all available linters.
list-lint-group List the linters in the given lint group.
list-linters List the configurerd linters.
protoc-commands Print the commands that would be run on compile or gen.
service-descriptor-proto Get the service descriptor proto for the service path.
version Print the version.
Things that I would nominate for deletion:
Things I'm not sure of the name of:
Right now, most commands take something along the lines of dirOrProtoFiles...
as their first arguments. These commands also have a flag --dir-mode
. The possibilities:
protoc
calls etc.prototool.yaml
file (or uses the current directory if none is found), and then walks starting at this location for all .proto
files, and these are used, except for files in the excludes
lists in prototool.yaml
files.--dir-mode
. This has the effect as if you specified the directory of this file (using the logic above), but errors are only printed for that file. This is useful for e.g. Vim integration.The idea with "directory builds" is that you often need more than just one file to do a protoc
call, for example if you have types in other files in the same package that are not referenced by their fully-qualified name, and/or if you need to know what directories to specify with -I
to protoc
(by default, the directory of the prototool.yaml
file is used).
In general practice, directory builds are what you always want to do. File builds were just added for convenience.
There's a proposal to do the following instead:
--dir-mode
above.There are some side effects, for example you need to special-case the format
command probably, but we probably want to do this. It's not as trivial as I first realized, and wanted to open this up for discussion before I do it.
The Protobuf team at Google had a request for a tool that brings up a mock gRPC server, given a Protobuf definition. It would spit back dummy data for any endpoint. This isn't overly difficult, but needs spec'ing out as to what this means (options to return errors? sizes of dummy data? etc).
The Golang Protobuf parsing library github.com/emicklei/proto has done wonders for Prototool, and this project would not exist without it - when I started Prototool, it was originally conceived as just a linter for Uber to keep us on a common standard, and the prototype solely used this library. I can't thank Ernest Micklei enough.
However, Prototool has grown into something much larger, and emicklei/proto does not fully implement the Protobuf spec - we explicitly need to check against protoc to verify a file compiles before linting or formatting, and cleaning up edge cases in emicklei/proto has been a case of exception-driven development. Along the way, the internal/x/protoc
package picked up the ability to compile Protobuf files on the fly and provide a full FileDescriptorSet to Prototool packages, which a lot of the functionality within Prototool relies on. If SourceCodeInfo was also outputted (easily done by adding --include_imports --include_source_info
to protoc
calls) then we could effectively replicate the functionality of Prototool.
There's a few options here:
Message/Service/Enum/etc
structs/interfaces based on FileDescriptorSets/SourceCodeInfo
.FileDescriptorSets/SourceCodeInfo
to use the existing setup in https://godoc.org/github.com/jhump/protoreflect/desc. There's a few things missing in the descriptors there, like source representation for options and the ability to walk descriptors in order they appear in the file, but if we're going to factor out emicklei/proto this is probably the best option as we use jhump/protoreflect elsewhere and that probably isn't going away.internal/x/lint
and internal/x/format
, so we might just keep it for now.The following protofile produces a pretty hard-to-debug error when linting (and probably for other stuff as well):
syntax = "proto3";
import "google/api/annotations.proto";
service Test {
rpc Test(Void) returns (Void) {
option (google.api.http) = {
post: "/api/v1/test";
body: "*";
};
}
}
message Void {}
The error:
$ prototool lint test.proto
test.proto:12:1: found "}" but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]
Specifically, it's the semicolons inside the options
object that the parser doesn't like.
This is, of course, a perfectly valid protofile definition (as protoc
accepts it), though I admit I'm having trouble finding a definition for multi-value options. See https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#option, https://developers.google.com/protocol-buffers/docs/proto#customoptions for more information. The latter lists a single example of a multi-value option where all options are on the same line, without semicolon;
// usage:
message Bar {
optional int32 a = 1 [(foo_options).opt1 = 123, (foo_options).opt2 = "baz"];
// alternative aggregate syntax (uses TextFormat):
optional int32 b = 2 [(foo_options) = { opt1: 123 opt2: "baz" }];
}
So perhaps this could be a linter option? In any case the parser should allow it.
Right now, config files are named prototool.yaml
and searched for starting at the Protobuf file path, and going up a directory until hitting root. All paths in the config files can be relative or absolute, but if relative, the actual path used will start at the config file directory. If using directory mode, this means multiple prototool.yaml
files corresponding to multiple found directories with Protobuf files may be used. After a config file is found, Prototool walks starting at that directory searching for Protobuf files, or if no config file is found, Prototool searches starting at the current directory.
A few things:
prototool.yaml
the right name?prototool.yaml
file in your repo, but do in your home directory, prototool
will go all the way down to your home directory, and then walk everything in your home directory, which can be annoying.prototool.yaml
file to use prototool
? See the above. It's good practice to always have a prototool.yaml
file to at least lock down the version of protoc
you use, but this can be annoying for OSS users who are just starting out.prototool.yaml
files to be discovered? If multiple prototool.yaml
files are discovered, this means that multiple calls to protoc
must be done as they denote different builds, and more than one FileDescriptorSet
is created. This gets bad when you do e.g. gRPC calls as foo.bar.BazService
could be in more than one FileDescriptorSet
, so you basically have to stop when you find the first one. All packages in Prototool have to handle multiple FileDescriptorSets
, which leads to some other bad outcomes. Most people shouldn't have multiple prototool.yaml
files, but it could be useful for compiling vendored prototool.yaml
files.It's desirable to know all protoc errors and handle them separately, so we can output errors in a consistent manner with prototool
ie filename:line:column:message
. This was done by handing errors as they showed up during development:
var (
// special cased
pluginFailedRegexp = regexp.MustCompile("^--.*_out: protoc-gen-(.*): Plugin failed with status code (.*).$")
otherPluginFailureRegexp = regexp.MustCompile("^--(.*)_out: (.*)$")
extraImportRegexp = regexp.MustCompile("^(.*): warning: Import (.*) but not used.$")
fileNotFoundRegexp = regexp.MustCompile("^(.*): File not found.$")
// protoc outputs both this line and fileNotFound, so we end up ignoring this one
// TODO figure out what the error is for errors in the import
importNotFoundRegexp = regexp.MustCompile("^(.*): Import (.*) was not found or had errors.$")
noSyntaxSpecifiedRegexp = regexp.MustCompile("No syntax specified for the proto file: (.*)\\. Please use")
jsonCamelCaseRegexp = regexp.MustCompile("^(.*): (The JSON camel-case name of field.*)$")
isNotDefinedRegexp = regexp.MustCompile("^(.*): (.*) is not defined.$")
seemsToBeDefinedRegexp = regexp.MustCompile(`^(.*): (".*" seems to be defined in ".*", which is not imported by ".*". To use it here, please add the necessary import.)$`)
explicitDefaultValuesProto3Regexp = regexp.MustCompile("^(.*): Explicit default values are not allowed in proto3.$")
optionValueRegexp = regexp.MustCompile("^(.*): Error while parsing option value for (.*)$")
programNotFoundRegexp = regexp.MustCompile("protoc-gen-(.*): program not found or is not executable$")
firstEnumValueZeroRegexp = regexp.MustCompile("^(.*): The first enum value must be zero in proto3.$")
)
This isn't great. We need to either figure out what all the errors or, or allow the catch-all to do something other than return a "system" error.
We have dynamic values in our prototool.yaml
file. E.g. the path can change for grpc-ruby
between each developer environment.
It would be nice if we were able to override the default values set in prototool.yaml
from the command line and/or and overrides yaml file. E.g.
prototool gen --set 'gen.plugin_overrides.grpc-ruby:/usr/bin/grpc_tools_ruby_protoc_plugin'
Or
prototool gen --overrides overrides.yaml
When the formatter processes the following file:
syntax = "proto3";
import "google/api/annotations.proto";
service Test {
rpc Test(Void) returns (Void) {
option (google.api.http) = {
get: "/api/v1/test"
additional_bindings {
post: "/api/v1/test"
body: "*"
}
};
}
}
message Void {}
It produces the following:
syntax = "proto3";
import "google/api/annotations.proto";
service Test {
rpc Test(Void) returns (Void) {
option (google.api.http) = {
get: "/api/v1/test"
additional_bindings.post: "/api/v1/test"
additional_bindings.body: "*"
};
}
}
message Void {}
This output is not parsable by emicklei/proto
at the moment (see emicklei/proto#80).
The options as I see it:
emicklei/proto
with something else (see #1).Building on #2, there was a request from the Protobuf team at Google to derive the location of generated files for known plugins. For example, if you have foo.proto
, used protoc-gen-go
, and had an output directory of ../../gen/proto/go
, the list of generated files would be ["../../gen/proto/go/foo.pb.go"]
. This would help with integration into build systems.
Instead of writing your own Protobuf formatter, why not use the popular clang-format tool which already supports Protobuf files?
Hello,
When I try to install protool, I've got this error:
curl -sSL https://github.com/uber/prototool/releases/download/v0.1.0/prototool-$(uname -s)-$(uname -m) \ -o /usr/local/bin/prototool && \ chmod +x /usr/local/bin/prototool && \ prototool -h
return
curl: (23) Failed writing body (0 != 16360)
The library github.com/fullstorydev/grpcurl, built on top of github.com/jhump/protoreflect, enabled Prototool to get off-the-ground with gRPC support easily. However, Prototool does not implement all the features that grpcurl has. Right now:
FileDescriptorSets
on your own if the gRPC reflection service is not available.FileDescriptorSets
for you.It'd be nice to merge this Venn diagram in some manner.
input:
syntax = "proto3";
package group.v1;
command: prototool lint test.proto
output:
test.proto:3:14: found "." but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]
expected: No output
I found emicklei/proto#37 and this looks like the exact same issue. Does github.com/emicklei/proto
need to updated?
Hoping to use this tool in my org's CI.
We used github.com/uber-go/zap for logging, which is fine as long as this is just a CLI tool, but as we learned the hard way in github.com/grpc/grpc-go, it's not great to assume a given logging library unless you need to. The logging in Prototool is very basic, and almost always at the debug level, so creating a minimal package similar to https://godoc.org/google.golang.org/grpc/grpclog (but even simpler) might be nice.
In Protoeasy, I took the tact that Protoeasy should know the entire world, and it will only handle known plugins. This worked well for basic usage, but proved too limiting, as I eventually had to add the --extra-plugin
flag, and keeping up to date with all handled plugins was not scalable in any sense of the word. In prototyping Prototool, I took the compete opposite tact, that I like to think of as the "dependency injection approach", where Prototool is not aware of any plugins, instead leaving you to specify what to compile and where to output it. Through discussions with the Protobuf team at Google, it's become apparent that a middle-ground approach is desired, where we do handle "well-known plugins" in a consistent manner, but leave room to "break glass" if necessary.
The current setup requires something like this:
gen:
go_options:
import_path: github.com/uber/prototool/example/idl/uber
extra_modifiers:
google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
plugins:
- name: gogoslick
type: gogo
flags: plugins=grpc
output: ../../gen/proto/go
- name: yarpc-go
type: gogo
output: ../../gen/proto/go
- name: java
output: ../../gen/proto/java
- name: grpc-gateway
type: go
output: ../../gen/proto/go
This will output stubs for github.com/gogo/protobuf including grpc-go stubs, github.com/yarpc/yarpc-go, java (the built-in "plugin" for java), and github.com/grpc-ecosystem/grpc-gateway. We denote the type
for a Golang plugin to say whether to do Well-Known Type imports from github.com/golang/protobuf or github.com/gogo/protobuf. The import_path
will probably be needed in some form regardless, unless we get too cute. extra_modifiers
are needed for grpc-gateway, and all the plugins need to know where to output their result.
One of the goals of Prototool is for users to not have to think about these things, and for output to be consistent - a user can start using Prototool, and all output will be in the same location. Roughly, I have a few potential ideas in my head of how to accomplish this.
Add a base_output
option to gen
. In the example above, base_output
would be ../../gen/proto
. This says the directory to put all output to. The default value might be gen/proto
.
Add the concept of Well-Known Plugins. This would be the set of the most commonly used plugins , and we would define default values for the type, flags, output
values. We might also overload the concept of name
to have some Well-Known Plugins do gRPC generation. As an instructive example, the following would be the equivalent (except for the java override):
gen:
base_output: ../../gen/proto
plugins:
- name: gogoslick-grpc
- name: yarpc-go
- name: java
output: java/example
- name: grpc-gateway
Where we say output
is go
for gogoslick-grpc, yarpc-go, grpc-gateway
, and the output
is java/example
for java
(the default would have been java
).
The Well-Known Plugins off the top of my head would be all the builtins (cpp, csharp, java, etc), all the golang/protobuf and gogo/protobuf plugins, grpc variants of all of these that we can, along with grpc-gateway and yarpc-go (yarpc-go is needed for Uber).
"uber"
for example might be gogoslick-grpc, yarpc-go, java, cpp, python
along with some gRPC variants. The Plugin Group named "default"
or "google"
might be much more extensive. Think the following:gen:
plugin_group:
name: uber
exclude:
- cpp
- python
Going along with #3, we have a lot of options in the config file for linters. I'm not sure where we landed on this, but worth further discussion.
We've committed our draft Style Guide into etc/style/uber/uber.proto
. This is the product of a lot of hashing out for what we want Protobuf files to look like within Uber. The rules inside this are inherently strict - we want there to be One Way To Do Things™ within Uber, and we want easy-to-follow rules that prevent users from making common mistakes. As Jisi on the Protobuf team at Google pointed out though, this style guide would require migration for most existing users.
What my hope is is that we have the new "recommended/strict Style Guide" that accomplishes our goals within Uber, which I think apply to the greater community - if we have a consistent way to do enums, a consistent naming convention for go_package, java_package
, etc, everyone wins, and makes things like consistent output of stubs easier. We also have the "default Style Guide" that does the bare minimum enforcement.
For the record, here are the rules I have to ignore to get googleapis to pass lint:
lint:
exclude_ids:
- REQUEST_RESPONSE_TYPES_UNIQUE
- REQUEST_RESPONSE_TYPES_IN_SAME_FILE
- ENUM_FIELD_PREFIXES
- ENUM_ZERO_VALUES_INVALID
- FILE_OPTIONS_EQUAL_GO_PACKAGE_PB_SUFFIX
- FILE_OPTIONS_EQUAL_JAVA_PACKAGE_COM_PB
- FILE_OPTIONS_REQUIRE_JAVA_PACKAGE
Action items:
Copying Jisi's feedback from an email (hope this is ok Jisi!):
There are two parts in the style guide,
- Fromatting rules., e.g. spaces, empty lines, import orders. Those can be safely applied to the whole depot without causing any issues.
- Naming: e.g. packages, enums, etc. Those could affect the APIs.
For a global style guide, we should probably only enforce the format ones. For the naming, we can list several best-practices (we do have such a doc in google), but as protobuf is already widely used, each organization probably has its own naming style. For example, you can find Google API's style guide here. Another idea is we can have different profile for naming.
Some of the detailed feedback about the style guide:
- package: each organization probably has its own style of naming packages. I can image the migration cost for google to a converged package style would be impractical given the # of protos we have.
- enum default value: internally we recommend _UNSPECIFIED or _UNSET as 0. It's the proto3 design goal to not distinguish between unset and set-to-default. In current proto3 implementation, you won't be able to distinguish between the two -- has-bit is gone and setting a value to 0 is a no-op (values default to 0). The serialization will simply skip the field even if you set it to 0 value explicitly. This is also a API change and we should probably just give suggestions rather than enforce it.
https://developers.google.com/protocol-buffers/docs/proto3#packages says that packages are optional, which even having used Protobuf for a while, is news to me. We should enforce as part of linting that packages are required - we use them for gRPC paths, for fully-qualified references, and relate them to the go_package
and java_package
options. For consistency, we should always have them.
In https://github.com/uber/prototool/blob/dev/internal/x/lint/check_package_lower_snake_case.go#L64, we actually enforce this, but this should be split out into a separate linting rule, and we should add the Filename to the scanner.Position
there.
We should consider the prototool.yaml
hierarchy and determine whether or not we should stick to the current tree/override behavior. A single prototool.yaml
at the project's root might be sufficient for now. This would also simplify some of the implementation details found in the settings
package.
If there isn't an obvious need for the hierarchy, it might be worth deferring until after a 1.0 release.
If you have a field:
int woot = 5 [(bar.field_option) = true];
Print that out instead of the current:
int woot = 5 [
(bar.field_option) = true
];
Any chance to produce a windows binary as well ?
In #2 we describe the wanted functionality of handing known plugins. We install protoc
for you within Prototool. It would be nice for Prototool to handle installation of things like protoc-gen-go
as well. A problem here is that you want your library code to match your generated code, so it might be better for you to do:
.PHONY: gen
gen:
go install ./vendor/github.com/golang/protobuf/protoc-gen-go
prototool gen
So this might end up causing more problems than it solves. This is worth discussion though.
prototool grpc
leaves behind temporary protobuf descriptors on every call.
To reproduce, make any call with grpc
and inspect your OS's typical temp file location. For example:
$ prototool grpc foo.Bar/Baz ''
$ ls /tmp/prototool*
/tmp/prototool661509915 /tmp/prototool892888492
This came up when fixing #27. Will fix in a follow-up PR.
https://github.com/nilslice/protolock just came out recently, and this is a nice tool, but it would be nice if similar functionality was built into Prototool. Additionally, Protolock generates a proto.lock
file that stores information on the Protobuf files in a repository, which I'd like to get rid of. This is a new format and inherently can have bugs, but more to the point is duplicated information - the proto.lock
file is generated via the checked-in Protobuf files, which in turn contain all the information you need. If you were to generate a FileDescriptorSet
for the current Protobuf files, and a FileDescriptorSet
for the old Protobuf files, and then compare the two, you could get the same thing. A proto.lock
file could have just been the result of --descriptor_set_out
as a further example, and then used https://github.com/jhump/protoreflect, but again instead of storing another file, we can just generate this on the fly. I'd also like errors to be printed in the Prototool standard filename:line:column:message
format.
Example of what I'd imagine:
prototool check-compatibility --from dev # dev is the name of the base branch
a/b/c.proto:3:4:Cannot remove field "foo".
a/b/d.proto:1:1:Cannot change syntax.
...
We need to agree on how to expose a Golang external API for library usage. We need this for yab for example. This will probably be something similar to internal/x/exec.Runner
, but with actual inputs and return values, as opposed to printing to stdout.
Hi!
I'm raising this for discussion as I assume the formatting is already based on some formatting agreed within Uber. Anyway, the Google API Design guidelines explicitly list the following macro ordering:
The file structure must put higher level and more important definitions before lower level and less important items. In each proto file, the applicable sections should be in the following order:
Now, I think specifically point 2 is violated by prototool format
as it puts import statements above both options and package declarations.
I'm trying to get the linter to pass on my swagger annotations however it seems like protoc is invoked differently for prototool lint
command than for prototool gen
. The modifications below to the example break lint but work for gen. Is this an issue with my configuration or are the swagger annotations not supported yet?
# prototool lint
[...]
2018-05-08T12:07:26.706-0400 DEBUG running protoc {"command": "/Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/bin/protoc -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I /Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/include -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber -o /dev/null /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber/foo/foo.proto"}
foo/foo.proto:18:1: found "}" but expected [.proto element {comment|option|import|syntax|enum|service|package|message}]
# prototool gen
[...]
2018-05-08T12:07:05.189-0400 DEBUG running protoc {"command": "/Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/bin/protoc -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I /Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/include -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber --gogoslick_out=plugins=grpc,Mgoogle/protobuf/compiler/plugin.proto=github.com/gogo/protobuf/protoc-gen-gogo/plugin,Mgoogle/protobuf/descriptor.proto=github.com/gogo/protobuf/protoc-gen-gogo/descriptor,Mgoogle/protobuf/duration.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/source_context.proto=google.golang.org/genproto/protobuf/source_context,Mgoogle/protobuf/type.proto=google.golang.org/genproto/protobuf/ptype,Mgoogle/protobuf/any.proto=github.com/gogo/protobuf/types,Mgoogle/protobuf/api.proto=google.golang.org/genproto/protobuf/api,Mgoogle/protobuf/empty.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,Mgoogle/api/http.proto=google.golang.org/genproto/googleapis/api/annotations,Mprotoc-gen-swagger/options/annotations.proto=github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options/annotations.proto,Mgoogle/api/annotations.proto=google.golang.org/genproto/googleapis/api/annotations:/Users/lucasvo/Code/go/src/github.com/uber/prototool/example/gen/proto/go /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber/foo/foo.proto"}
2018-05-08T12:07:05.189-0400 DEBUG running protoc {"command": "/Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/bin/protoc -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I /Users/lucasvo/Library/Caches/prototool/Darwin/x86_64/protobuf/3.5.1/include -I /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber --java_out=/Users/lucasvo/Code/go/src/github.com/uber/prototool/example/gen/proto/java /Users/lucasvo/Code/go/src/github.com/uber/prototool/example/idl/uber/foo/foo.proto"}
The diff for the example:
diff --git a/example/idl/uber/foo/foo.proto b/example/idl/uber/foo/foo.proto
index 7e94743..0d8e47a 100644
--- a/example/idl/uber/foo/foo.proto
+++ b/example/idl/uber/foo/foo.proto
@@ -5,6 +5,7 @@ import "google/protobuf/timestamp.proto";
import "foo/dep.proto";
import "google/api/annotations.proto";
+import "protoc-gen-swagger/options/annotations.proto";
import "sub/sub.proto";
package foo;
@@ -12,6 +13,9 @@ package foo;
option go_package = "foopb";
option java_multiple_files = true;
option java_package = "com.foo.pb";
+option (grpc.gateway.protoc_gen_swagger.options.openapiv2_swagger) = {
+ host: "example.com";
+};
// Hello is a hello.
enum Hello {
diff --git a/example/idl/uber/prototool.yaml b/example/idl/uber/prototool.yaml
index 592b48e..5177db7 100644
--- a/example/idl/uber/prototool.yaml
+++ b/example/idl/uber/prototool.yaml
@@ -2,6 +2,7 @@
protoc_includes:
# note vendor is .gitignored
- ../../../vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis
+ - ../../../vendor/github.com/grpc-ecosystem/grpc-gateway
protoc_include_wkt: true
lint:
# run "prototool list-linters" to see the currently configured linters
@@ -17,6 +18,8 @@ gen:
extra_modifiers:
google/api/annotations.proto: google.golang.org/genproto/googleapis/api/annotations
google/api/http.proto: google.golang.org/genproto/googleapis/api/annotations
+ protoc-gen-swagger/options/annotations.proto: github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options/annotations.proto
+
plugins:
- name: gogoslick
type: gogo
On a fresh clone of the repo:
~/go/src/github.com/uber/prototool (dev) 13:51:16
$ make
git status --short > /var/folders/nk/lrxg8bsn08l9c4w9ld_00v440000gn/T/checknodiffgenerated_pre.XXXXX.IE2l7UzP
/Applications/Xcode.app/Contents/Developer/usr/bin/make generate
go install ./vendor/go.uber.org/tools/update-license
can't load package: package github.com/uber/prototool/vendor/go.uber.org/tools/update-license: cannot find package "github.com/uber/prototool/vendor/go.uber.org/tools/update-license" in any of:
/usr/local/Cellar/go/1.10.1/libexec/src/github.com/uber/prototool/vendor/go.uber.org/tools/update-license (from $GOROOT)
/Users/blampe/go/src/github.com/uber/prototool/vendor/go.uber.org/tools/update-license (from $GOPATH)
make[1]: *** [license] Error 1
make: *** [checknodiffgenerated] Error 2
After running glide i
I still get a failure:
$ make
git status --short > /var/folders/nk/lrxg8bsn08l9c4w9ld_00v440000gn/T/checknodiffgenerated_pre.XXXXX.o7dDQPDS
/Applications/Xcode.app/Contents/Developer/usr/bin/make generate
go install ./vendor/go.uber.org/tools/update-license
update-license ./cmd/prototool/main.go ./internal/cmd/cmd.go ./internal/cmd/cmd_test.go ./internal/desc/desc.go ./internal/diff/diff.go ./internal/exec/exec.go ./internal/exec/runner.go ./internal/extract/extract.go ./internal/extract/getter.go ./internal/file/file.go ./internal/file/proto_set_provider.go ./internal/file/proto_set_provider_test.go ./internal/format/base_visitor.go ./internal/format/first_pass_visitor.go ./internal/format/format.go ./internal/format/log_visitor.go ./internal/format/middle_visitor.go ./internal/format/printer.go ./internal/format/transformer.go ./internal/gen/gen-prototool-bash-completion/main.go ./internal/gen/gen-prototool-manpages/main.go ./internal/gen/gen-prototool-zsh-completion/main.go ./internal/grpc/grpc.go ./internal/grpc/handler.go ./internal/grpc/invocation_event_handler.go ./internal/lint/base_checker.go ./internal/lint/base_visitor.go ./internal/lint/check_comments_no_c_style.go ./internal/lint/check_enum_field_names_upper_snake_case.go ./internal/lint/check_enum_field_names_uppercase.go ./internal/lint/check_enum_field_prefixes.go ./internal/lint/check_enum_names_camel_case.go ./internal/lint/check_enum_names_capitalized.go ./internal/lint/check_enum_zero_values_invalid.go ./internal/lint/check_enums_have_comments.go ./internal/lint/check_file_options_equal.go ./internal/lint/check_file_options_required.go ./internal/lint/check_message_field_names_lower_snake_case.go ./internal/lint/check_message_field_names_lowercase.go ./internal/lint/check_message_fields_not_floats.go ./internal/lint/check_message_names_camel_case.go ./internal/lint/check_message_names_capitalized.go ./internal/lint/check_messages_have_comments.go ./internal/lint/check_messages_have_comments_except_request_response_types.go ./internal/lint/check_oneof_names_lower_snake_case.go ./internal/lint/check_package_lower_snake_case.go ./internal/lint/check_request_response_names_match_rpc.go ./internal/lint/check_request_response_types_in_same_file.go ./internal/lint/check_request_response_types_unique.go ./internal/lint/check_rpc_names_camel_case.go ./internal/lint/check_rpc_names_capitalized.go ./internal/lint/check_rpcs_have_comments.go ./internal/lint/check_service_names_camel_case.go ./internal/lint/check_service_names_capitalized.go ./internal/lint/check_services_have_comments.go ./internal/lint/check_syntax_proto3.go ./internal/lint/check_wkt_directly_imported.go ./internal/lint/lint.go ./internal/lint/runner.go ./internal/protoc/compiler.go ./internal/protoc/downloader.go ./internal/protoc/downloader_test.go ./internal/protoc/protoc.go ./internal/reflect/handler.go ./internal/reflect/reflect.go ./internal/settings/config_provider.go ./internal/settings/config_provider_test.go ./internal/settings/settings.go ./internal/strs/strs.go ./internal/strs/strs_test.go ./internal/text/text.go ./internal/text/text_test.go ./internal/wkt/wkt.go ./prototool.go
git log --format='%aN <%aE>' | sort -fu > CONTRIBUTORS
go install \
-ldflags "-X 'github.com/uber/prototool.GitCommit=2f29e7013073f1209afe97db4682dfd931e760e3' -X 'github.com/uber/prototool.BuiltTimestamp=Mon Apr 16 20:57:53 UTC 2018'" \
github.com/uber/prototool/cmd/prototool
for file in internal/cmd/testdata/format/bar/bar.proto.golden internal/cmd/testdata/format/bar/bar_proto2.proto.golden internal/cmd/testdata/format/foo/foo.proto.golden internal/cmd/testdata/format/foo/foo_proto2.proto.golden; do \
rm -f ${file}; \
done
for file in internal/cmd/testdata/format/bar/bar.proto internal/cmd/testdata/format/bar/bar_proto2.proto internal/cmd/testdata/format/foo/foo.proto internal/cmd/testdata/format/foo/foo_proto2.proto; do \
prototool format ${file} > ${file}.golden || true; \
done
go install ./vendor/github.com/gogo/protobuf/protoc-gen-gogoslick
go install ./vendor/github.com/golang/protobuf/protoc-gen-go
go install ./vendor/github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway
go install ./vendor/go.uber.org/yarpc/encoding/protobuf/protoc-gen-yarpc-go
rm -rf example/gen
prototool all example/idl/uber
go build ./example/gen/proto/go/foo
go build ./example/gen/proto/go/sub
go build ./example/cmd/excited/main.go
prototool lint etc/style
prototool gen internal/cmd/testdata/grpc
rm -rf etc/release
mkdir -p etc/release/etc/bash_completion.d
mkdir -p etc/release/etc/zsh_completion.d
mkdir -p etc/release/share/man/man1
go run internal/gen/gen-prototool-bash-completion/main.go > etc/release/etc/bash_completion.d/prototool
go run internal/gen/gen-prototool-zsh-completion/main.go > etc/release/etc/zsh_completion.d/prototool
go run internal/gen/gen-prototool-manpages/main.go etc/release/share/man/man1
git status --short > /var/folders/nk/lrxg8bsn08l9c4w9ld_00v440000gn/T/checknodiffgenerated_post.XXXXX.kzuns5G8
make generate produced a diff, make sure to check these in:
0a1,21
> M etc/release/share/man/man1/prototool-all.1
> M etc/release/share/man/man1/prototool-binary-to-json.1
> M etc/release/share/man/man1/prototool-clean.1
> M etc/release/share/man/man1/prototool-compile.1
> M etc/release/share/man/man1/prototool-descriptor-proto.1
> M etc/release/share/man/man1/prototool-download.1
> M etc/release/share/man/man1/prototool-field-descriptor-proto.1
> M etc/release/share/man/man1/prototool-files.1
> M etc/release/share/man/man1/prototool-format.1
> M etc/release/share/man/man1/prototool-gen.1
> M etc/release/share/man/man1/prototool-grpc.1
> M etc/release/share/man/man1/prototool-json-to-binary.1
> M etc/release/share/man/man1/prototool-lint.1
> M etc/release/share/man/man1/prototool-list-all-lint-groups.1
> M etc/release/share/man/man1/prototool-list-all-linters.1
> M etc/release/share/man/man1/prototool-list-lint-group.1
> M etc/release/share/man/man1/prototool-list-linters.1
> M etc/release/share/man/man1/prototool-protoc-commands.1
> M etc/release/share/man/man1/prototool-service-descriptor-proto.1
> M etc/release/share/man/man1/prototool-version.1
> M etc/release/share/man/man1/prototool.1
make: *** [checknodiffgenerated] Error 1
Prototool shells out to protoc to do a lot of the heavy lifting. Right now, we are not handling OS signals if Prototool is sent a SIGTERM, SIGINT, SIGQUIT, SIGKILL
, etc. This isn't difficult to add, just something we need to do before v1.0.
The latest version of prototool
on dev
adds java_option
s unbidden when running prototool format -w
.
This probably won't happen, but just for discussion.
In Protoeasy, I made it possible to run Protoeasy as a server with a given docker container (which also had versions of all known plugins) so there was no need to install anything locally. This was useful when it was not as trivial to install protoc, and was also useful for installing things like golang/protobuf grpc-gateway in a consistent manner. This worked using https://github.com/peter-edge/protoeasy-go/blob/master/protoeasy.proto by tarring up all .proto
files in your context, sending the tarball to the server, running the generation logic, then tarring up the entire result, and untarring in your local context.
Not sure if we want something similar here. Prototool handles installing protoc for you, which is easier now with GitHub releases. Depending on what we do with #2, we might want this to make it easy for known plugins to be handled as well.
vim/prototool
contains the Vim integration. Along with dense-analysis/ale#1382, we should either get all the code into ALE, or have a separate prototool.vim
repository.
Prototool does not currently handle import public
https://developers.google.com/protocol-buffers/docs/proto3#importing-definitions. We need to handle this before v1.0.
We also should handle import weak
as it is part of the spec, even if not really used. https://developers.google.com/protocol-buffers/docs/reference/proto3-spec#import_statement
Similar to #11, we need a full review of the prototool.yaml
config file. It exposes way too much functionality for v1.0 in my opinion. See etc/config/example/prototool.yaml
for all options.
Things I nominate for potential deletion:
Things I'm unsure of:
I am using protocol buffers version 2.6.1 and linting fails with the following error on debug
2018-05-06T23:59:52.701+0200 DEBUG downloaded protobuf zip file {"url": "https://github.com/google/protobuf/releases/download/v2.6.1/protoc-2.6.1-osx-x86_64.zip"}
zip: not a valid zip file
The URL indeed does not work and as you can see from the file prototool
tries to download we are talking about an OSX system here.
Is prototool
compatible with version 2 protos?
Example: cat input.json | prototool grpc example 0.0.0.0:8080 foo.ExcitedService/Exclamation
The trailing -
is missing to denote stdin reading, but this is technically a legal invocation where it assumes you mean the current directory, with address example
, method name 0.0.0.0:8080
, and data foo.ExcitedService/Exclamation
. We need to fix this in some manner, perhaps flags for the input args.
jq json parser is needed.
make cover
./etc/bin/cover.sh: line 72: jq: command not found
Maybe is it possible to add a check for jq on make init ?
https://cloud.google.com/apis/design/file_structure
Specifically:
java_package
will be com.PACKAGE
instead of com.PACKAGE.pb
java_multiple_files
will be set to truejava_outer_classname
will be FILENAMEProto
go_package
will not change as there's no guidance on that, and we won't do anything with csharp_namespace
, objc_class_prefix
, or php_namespace
as of yet, as we don't use those and we don't need to overcomplicate this for now (we might want to do objc_class_prefix
in the future though).
Changes that need to be made:
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.