From 96b64a34d1eac5063a8708dfe813614057bed09b Mon Sep 17 00:00:00 2001 From: Kazuyoshi Kato Date: Thu, 23 Sep 2021 09:28:31 -0700 Subject: [PATCH] Support multiple generators The latest version of protoc-gen-go doesn't support gRPC. Supporting multiple generators is the way to support gRPC going forward. Signed-off-by: Kazuyoshi Kato --- config.go | 57 +++++++++++++++++++++++++++++++++++++++++------ config_test.go | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++ main.go | 13 ++++++----- protoc.go | 16 +++++++++----- protoc_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 19 deletions(-) create mode 100644 config_test.go create mode 100644 protoc_test.go diff --git a/config.go b/config.go index 4f978e8..28d4e49 100644 --- a/config.go +++ b/config.go @@ -27,10 +27,19 @@ import ( const configVersion = "unstable" type config struct { - Version string + Version string + Generators []string + + // Generator is a code generator which is used from protoc. + // Deprecated: Use Generators instead. Generator string - Plugins []string - Includes struct { + + // Plugins will be deprecated. It has to be per-Generator setting, + // but neither protoc-gen-go nor protoc-gen-go-grpc support plugins. + // So the refactoring is not worth to do. + Plugins []string + + Includes struct { Before []string Vendored []string Packages []string @@ -40,9 +49,12 @@ type config struct { Packages map[string]string Overrides []struct { - Prefixes []string - Generator string - Plugins *[]string + Prefixes []string + // Generator is a code generator which is used from protoc. + // Deprecated: Use Generators instead. + Generator string + Generators []string + Plugins *[]string // TODO(stevvooe): We could probably support overriding of includes and // package maps, but they don't seem to be as useful. Likely, @@ -59,7 +71,6 @@ type config struct { func newDefaultConfig() config { return config{ - Generator: "go", Includes: struct { Before []string Vendored []string @@ -77,6 +88,10 @@ func readConfig(path string) (config, error) { if err != nil { log.Fatalln(err) } + return readConfigFrom(p) +} + +func readConfigFrom(p []byte) (config, error) { c := newDefaultConfig() if err := toml.Unmarshal(p, &c); err != nil { log.Fatalln(err) @@ -86,5 +101,33 @@ func readConfig(path string) (config, error) { return config{}, fmt.Errorf("unknown file version %v; please upgrade to %v", c.Version, configVersion) } + if c.Generator != "" { + if len(c.Generators) > 0 { + return config{}, fmt.Errorf( + `specify either "generators = %v" or "generator = %v", not both`, + c.Generators, c.Generator, + ) + } + c.Generators = []string{c.Generator} + c.Generator = "" + } + + for i, o := range c.Overrides { + if o.Generator != "" { + if len(o.Generators) > 0 { + return config{}, fmt.Errorf( + `specify either "overrides[%d].generators" or "overrides[%d].generator", not both`, + i, i, + ) + } + c.Overrides[i].Generators = []string{o.Generator} + c.Overrides[i].Generator = "" + } + } + + if len(c.Generators) == 0 { + c.Generators = []string{"go"} + } + return c, nil } diff --git a/config_test.go b/config_test.go new file mode 100644 index 0000000..b29e1a2 --- /dev/null +++ b/config_test.go @@ -0,0 +1,60 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package main + +import "testing" + +func TestReadConfigFrom(t *testing.T) { + testcases := []struct { + name string + toml string + }{ + { + name: "empty", + toml: `version="unstable"`, + }, + { + name: "generator", + toml: ` +version="unstable" +generator="go" +`, + }, + { + name: "generators", + toml: ` +version="unstable" +generators=["go"] +`, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + c, err := readConfigFrom([]byte(tc.toml)) + if err != nil { + t.Fatalf("err must be nil, but got %v", err) + } + if c.Generator == "go" { + t.Fatalf("Generator must be cleared, but got %v", c.Generator) + } + if len(c.Generators) != 1 || c.Generators[0] != "go" { + t.Fatalf("Generators must be [go], but got %v", c.Generators) + } + }) + } +} diff --git a/main.go b/main.go index a0c3a54..91fc918 100644 --- a/main.go +++ b/main.go @@ -77,9 +77,10 @@ func main() { // Index overrides by target import path overrides := map[string]struct { - Prefixes []string - Generator string - Plugins *[]string + Prefixes []string + Generator string + Generators []string + Plugins *[]string }{} for _, override := range c.Overrides { for _, prefix := range override.Prefixes { @@ -166,7 +167,7 @@ func main() { includes = append(includes, c.Includes.After...) protoc := protocCmd{ - Name: c.Generator, + Names: c.Generators, ImportPath: pkg.GoImportPath, PackageMap: c.Packages, Plugins: c.Plugins, @@ -182,8 +183,8 @@ func main() { if override, ok := overrides[importDirPath]; ok { // selectively apply the overrides to the protoc structure. - if override.Generator != "" { - protoc.Name = override.Generator + if len(override.Generators) > 0 { + protoc.Names = override.Generators } if override.Plugins != nil { diff --git a/protoc.go b/protoc.go index c8fa0de..842b0e2 100644 --- a/protoc.go +++ b/protoc.go @@ -30,12 +30,16 @@ var ( {{if $index}}` + string(filepath.ListSeparator) + `{{end -}} {{.}} {{- end -}} - {{- if .Descriptors}} --include_imports --descriptor_set_out={{.Descriptors}}{{- end }} -- - {{- .Name -}}_out={{if .Plugins}}plugins={{- range $index, $plugin := .Plugins -}} - {{- if $index}}+{{end}} - {{- $plugin}} + {{- if .Descriptors}} --include_imports --descriptor_set_out={{.Descriptors}}{{- end -}} + + {{- range $index, $name := .Names }} --{{- $name -}}_out= + {{- if $.Plugins}}plugins={{- range $index, $plugin := $.Plugins -}} + {{- if $index}}+{{end}} + {{- $plugin}} + {{- end -}},{{- end -}} + import_path={{$.ImportPath}} {{- end -}} - ,{{- end -}}import_path={{.ImportPath}} + {{- range $proto, $gopkg := .PackageMap -}},M {{- $proto}}={{$gopkg -}} {{- end -}} @@ -46,7 +50,7 @@ var ( // protocParams defines inputs to a protoc command string. type protocCmd struct { - Name string // backend name + Names []string Includes []string Plugins []string Descriptors string diff --git a/protoc_test.go b/protoc_test.go new file mode 100644 index 0000000..c8e86eb --- /dev/null +++ b/protoc_test.go @@ -0,0 +1,57 @@ +/* + Copyright The containerd Authors. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package main + +import "testing" + +func TestMkcmd(t *testing.T) { + testcases := []struct { + name string + cmd protocCmd + expected string + }{ + { + name: "basic", + cmd: protocCmd{Names: []string{"go"}}, + expected: "protoc -I --go_out=import_path=:", + }, + { + name: "plugin", + cmd: protocCmd{Names: []string{"go"}, Plugins: []string{"grpc"}}, + expected: "protoc -I --go_out=plugins=grpc,import_path=:", + }, + { + name: "use protoc-gen-go-grpc instead of plugins", + cmd: protocCmd{Names: []string{"go", "go-grpc"}}, + expected: "protoc -I --go_out=import_path= --go-grpc_out=import_path=:", + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + cmd := &tc.cmd + + s, err := cmd.mkcmd() + if err != nil { + t.Fatalf("err must be nil but %+v", err) + } + + if s != tc.expected { + t.Fatalf(`s must be %q, but %q`, tc.expected, s) + } + }) + } +}