From 6eff900b67a0ed7e639ae304b1d453b620e9b79d Mon Sep 17 00:00:00 2001 From: Ivan Lozano Date: Fri, 11 Dec 2020 15:25:59 -0500 Subject: [PATCH] Merge rust_protobuf and rust_grpcio module types. To allow grpc protobufs to include non-grpc protos in a singular library, we need to allow them to be defined as part of the same module. This CL merges the two previously distinct module types into rust_protobuf, and adds a new property for declaring which protos contain grpc definitions. Bug: 172952239 Test: rust_grpcio modules converted to rust_protobuf modules and build. Test: Example rust_protobuf module with both proto types builds. Change-Id: I0e627fd97bc6f74de89d309e3344694a1e76586d --- rust/protobuf.go | 169 +++++++++++++++++++++--------------------- rust/protobuf_test.go | 52 ++++++------- rust/rust_test.go | 18 +++-- rust/testing.go | 2 - 4 files changed, 121 insertions(+), 120 deletions(-) diff --git a/rust/protobuf.go b/rust/protobuf.go index 4fba34f92..b91fea86e 100644 --- a/rust/protobuf.go +++ b/rust/protobuf.go @@ -31,24 +31,22 @@ const ( type PluginType int -const ( - Protobuf PluginType = iota - Grpc -) - func init() { android.RegisterModuleType("rust_protobuf", RustProtobufFactory) android.RegisterModuleType("rust_protobuf_host", RustProtobufHostFactory) - android.RegisterModuleType("rust_grpcio", RustGrpcioFactory) - android.RegisterModuleType("rust_grpcio_host", RustGrpcioHostFactory) } var _ SourceProvider = (*protobufDecorator)(nil) type ProtobufProperties struct { - // List of relative paths to proto files that will be used to generate the source + // List of relative paths to proto files that will be used to generate the source. + // Either this or grpc_protos must be defined. Protos []string `android:"path,arch_variant"` + // List of relative paths to GRPC-containing proto files that will be used to generate the source. + // Either this or protos must be defined. + Grpc_protos []string `android:"path,arch_variant"` + // List of additional flags to pass to aprotoc Proto_flags []string `android:"arch_variant"` @@ -60,33 +58,54 @@ type protobufDecorator struct { *BaseSourceProvider Properties ProtobufProperties - plugin PluginType + protoNames []string + grpcNames []string + + grpcProtoFlags android.ProtoFlags + protoFlags android.ProtoFlags } func (proto *protobufDecorator) GenerateSource(ctx ModuleContext, deps PathDeps) android.Path { var protoFlags android.ProtoFlags - var pluginPaths android.Paths - var protoNames []string + var grpcProtoFlags android.ProtoFlags + var commonProtoFlags []string - protoFlags.OutTypeFlag = "--rust_out" outDir := android.PathForModuleOut(ctx) - - pluginPaths, protoFlags = proto.setupPlugin(ctx, protoFlags, outDir) - - protoFlags.Flags = append(protoFlags.Flags, defaultProtobufFlags...) - protoFlags.Flags = append(protoFlags.Flags, proto.Properties.Proto_flags...) - - protoFlags.Deps = append(protoFlags.Deps, pluginPaths...) - protoFiles := android.PathsForModuleSrc(ctx, proto.Properties.Protos) + grpcFiles := android.PathsForModuleSrc(ctx, proto.Properties.Grpc_protos) + protoPluginPath := ctx.Config().HostToolPath(ctx, "protoc-gen-rust") - if len(protoFiles) == 0 { - ctx.PropertyErrorf("protos", "at least one protobuf must be defined.") + commonProtoFlags = append(commonProtoFlags, defaultProtobufFlags...) + commonProtoFlags = append(commonProtoFlags, proto.Properties.Proto_flags...) + commonProtoFlags = append(commonProtoFlags, "--plugin=protoc-gen-rust="+protoPluginPath.String()) + + if len(protoFiles) > 0 { + protoFlags.OutTypeFlag = "--rust_out" + protoFlags.Flags = append(protoFlags.Flags, commonProtoFlags...) + + protoFlags.Deps = append(protoFlags.Deps, protoPluginPath) + } + + if len(grpcFiles) > 0 { + grpcPath := ctx.Config().HostToolPath(ctx, "grpc_rust_plugin") + + grpcProtoFlags.OutTypeFlag = "--rust_out" + grpcProtoFlags.Flags = append(grpcProtoFlags.Flags, "--grpc_out="+outDir.String()) + grpcProtoFlags.Flags = append(grpcProtoFlags.Flags, "--plugin=protoc-gen-grpc="+grpcPath.String()) + grpcProtoFlags.Flags = append(grpcProtoFlags.Flags, commonProtoFlags...) + + grpcProtoFlags.Deps = append(grpcProtoFlags.Deps, grpcPath, protoPluginPath) + } + + if len(protoFiles) == 0 && len(grpcFiles) == 0 { + ctx.PropertyErrorf("protos", + "at least one protobuf must be defined in either protos or grpc_protos.") } // Add exported dependency include paths for _, include := range deps.depIncludePaths { protoFlags.Flags = append(protoFlags.Flags, "-I"+include.String()) + grpcProtoFlags.Flags = append(grpcProtoFlags.Flags, "-I"+include.String()) } stem := proto.BaseSourceProvider.getStem(ctx) @@ -98,25 +117,50 @@ func (proto *protobufDecorator) GenerateSource(ctx ModuleContext, deps PathDeps) var outputs android.WritablePaths rule := android.NewRuleBuilder(pctx, ctx) + for _, protoFile := range protoFiles { - protoName := strings.TrimSuffix(protoFile.Base(), ".proto") - protoNames = append(protoNames, protoName) - - protoOut := android.PathForModuleOut(ctx, protoName+".rs") - ruleOutputs := android.WritablePaths{android.WritablePath(protoOut)} - - if proto.plugin == Grpc { - grpcOut := android.PathForModuleOut(ctx, protoName+grpcSuffix+".rs") - ruleOutputs = append(ruleOutputs, android.WritablePath(grpcOut)) + // Since we're iterating over the protoFiles already, make sure they're not redeclared in grpcFiles + if android.InList(protoFile.String(), grpcFiles.Strings()) { + ctx.PropertyErrorf("protos", + "A proto can only be added once to either grpc_protos or protos. %q is declared in both properties", + protoFile.String()) } + protoName := strings.TrimSuffix(protoFile.Base(), ".proto") + proto.protoNames = append(proto.protoNames, protoName) + + protoOut := android.PathForModuleOut(ctx, protoName+".rs") depFile := android.PathForModuleOut(ctx, protoName+".d") + ruleOutputs := android.WritablePaths{protoOut, depFile} + android.ProtoRule(rule, protoFile, protoFlags, protoFlags.Deps, outDir, depFile, ruleOutputs) outputs = append(outputs, ruleOutputs...) } - android.WriteFileRule(ctx, stemFile, proto.genModFileContents(ctx, protoNames)) + for _, grpcFile := range grpcFiles { + grpcName := strings.TrimSuffix(grpcFile.Base(), ".proto") + proto.grpcNames = append(proto.grpcNames, grpcName) + + // GRPC protos produce two files, a proto.rs and a proto_grpc.rs + protoOut := android.WritablePath(android.PathForModuleOut(ctx, grpcName+".rs")) + grpcOut := android.WritablePath(android.PathForModuleOut(ctx, grpcName+grpcSuffix+".rs")) + depFile := android.PathForModuleOut(ctx, grpcName+".d") + + ruleOutputs := android.WritablePaths{protoOut, grpcOut, depFile} + + android.ProtoRule(rule, grpcFile, grpcProtoFlags, grpcProtoFlags.Deps, outDir, depFile, ruleOutputs) + outputs = append(outputs, ruleOutputs...) + } + + // Check that all proto base filenames are unique as outputs are written to the same directory. + baseFilenames := append(proto.protoNames, proto.grpcNames...) + if len(baseFilenames) != len(android.FirstUniqueStrings(baseFilenames)) { + ctx.PropertyErrorf("protos", "proto filenames must be unique across 'protos' and 'grpc_protos' "+ + "to be used in the same rust_protobuf module. For example, foo.proto and src/foo.proto will conflict.") + } + + android.WriteFileRule(ctx, stemFile, proto.genModFileContents()) rule.Build("protoc_"+ctx.ModuleName(), "protoc "+ctx.ModuleName()) @@ -127,19 +171,19 @@ func (proto *protobufDecorator) GenerateSource(ctx ModuleContext, deps PathDeps) return stemFile } -func (proto *protobufDecorator) genModFileContents(ctx ModuleContext, protoNames []string) string { +func (proto *protobufDecorator) genModFileContents() string { lines := []string{ "// @Soong generated Source", } - for _, protoName := range protoNames { + for _, protoName := range proto.protoNames { lines = append(lines, fmt.Sprintf("pub mod %s;", protoName)) - - if proto.plugin == Grpc { - lines = append(lines, fmt.Sprintf("pub mod %s%s;", protoName, grpcSuffix)) - } } - if proto.plugin == Grpc { + for _, grpcName := range proto.grpcNames { + lines = append(lines, fmt.Sprintf("pub mod %s;", grpcName)) + lines = append(lines, fmt.Sprintf("pub mod %s%s;", grpcName, grpcSuffix)) + } + if len(proto.grpcNames) > 0 { lines = append( lines, "pub mod empty {", @@ -150,27 +194,6 @@ func (proto *protobufDecorator) genModFileContents(ctx ModuleContext, protoNames return strings.Join(lines, "\n") } -func (proto *protobufDecorator) setupPlugin(ctx ModuleContext, protoFlags android.ProtoFlags, outDir android.ModuleOutPath) (android.Paths, android.ProtoFlags) { - pluginPaths := []android.Path{} - - if proto.plugin == Protobuf { - pluginPath := ctx.Config().HostToolPath(ctx, "protoc-gen-rust") - pluginPaths = append(pluginPaths, pluginPath) - protoFlags.Flags = append(protoFlags.Flags, "--plugin="+pluginPath.String()) - } else if proto.plugin == Grpc { - grpcPath := ctx.Config().HostToolPath(ctx, "grpc_rust_plugin") - protobufPath := ctx.Config().HostToolPath(ctx, "protoc-gen-rust") - pluginPaths = append(pluginPaths, grpcPath, protobufPath) - protoFlags.Flags = append(protoFlags.Flags, "--grpc_out="+outDir.String()) - protoFlags.Flags = append(protoFlags.Flags, "--plugin=protoc-gen-grpc="+grpcPath.String()) - protoFlags.Flags = append(protoFlags.Flags, "--plugin=protoc-gen-rust="+protobufPath.String()) - } else { - ctx.ModuleErrorf("Unknown protobuf plugin type requested") - } - - return pluginPaths, protoFlags -} - func (proto *protobufDecorator) SourceProviderProps() []interface{} { return append(proto.BaseSourceProvider.SourceProviderProps(), &proto.Properties) } @@ -180,7 +203,7 @@ func (proto *protobufDecorator) SourceProviderDeps(ctx DepsContext, deps Deps) D deps.Rustlibs = append(deps.Rustlibs, "libprotobuf") deps.HeaderLibs = append(deps.SharedLibs, proto.Properties.Header_libs...) - if proto.plugin == Grpc { + if len(proto.Properties.Grpc_protos) > 0 { deps.Rustlibs = append(deps.Rustlibs, "libgrpcio", "libfutures") deps.HeaderLibs = append(deps.HeaderLibs, "libprotobuf-cpp-full") } @@ -203,34 +226,10 @@ func RustProtobufHostFactory() android.Module { return module.Init() } -func RustGrpcioFactory() android.Module { - module, _ := NewRustGrpcio(android.HostAndDeviceSupported) - return module.Init() -} - -// A host-only variant of rust_protobuf. Refer to rust_protobuf for more details. -func RustGrpcioHostFactory() android.Module { - module, _ := NewRustGrpcio(android.HostSupported) - return module.Init() -} - func NewRustProtobuf(hod android.HostOrDeviceSupported) (*Module, *protobufDecorator) { protobuf := &protobufDecorator{ BaseSourceProvider: NewSourceProvider(), Properties: ProtobufProperties{}, - plugin: Protobuf, - } - - module := NewSourceProviderModule(hod, protobuf, false) - - return module, protobuf -} - -func NewRustGrpcio(hod android.HostOrDeviceSupported) (*Module, *protobufDecorator) { - protobuf := &protobufDecorator{ - BaseSourceProvider: NewSourceProvider(), - Properties: ProtobufProperties{}, - plugin: Grpc, } module := NewSourceProviderModule(hod, protobuf, false) diff --git a/rust/protobuf_test.go b/rust/protobuf_test.go index 608a4e819..1ac66f388 100644 --- a/rust/protobuf_test.go +++ b/rust/protobuf_test.go @@ -69,31 +69,19 @@ func TestRustProtobuf(t *testing.T) { } } -func TestRustGrpcio(t *testing.T) { +func TestRustGrpc(t *testing.T) { ctx := testRust(t, ` - rust_grpcio { + rust_protobuf { name: "librust_grpcio", - protos: ["buf.proto", "proto.proto"], + protos: ["buf.proto"], + grpc_protos: ["foo.proto", "proto.proto"], crate_name: "rust_grpcio", source_stem: "buf", - shared_libs: ["libfoo_shared"], - static_libs: ["libfoo_static"], - } - cc_library_shared { - name: "libfoo_shared", - export_include_dirs: ["shared_include"], - } - cc_library_static { - name: "libfoo_static", - export_include_dirs: ["static_include"], } `) // Check that libprotobuf is added as a dependency. librust_grpcio_module := ctx.ModuleForTests("librust_grpcio", "android_arm64_armv8-a_dylib").Module().(*Module) - if !android.InList("libprotobuf", librust_grpcio_module.Properties.AndroidMkDylibs) { - t.Errorf("libprotobuf dependency missing for rust_grpcio (dependency missing from AndroidMkDylibs)") - } // Check that libgrpcio is added as a dependency. if !android.InList("libgrpcio", librust_grpcio_module.Properties.AndroidMkDylibs) { @@ -106,20 +94,12 @@ func TestRustGrpcio(t *testing.T) { } // Make sure the correct plugin is being used. - librust_grpcio_out := ctx.ModuleForTests("librust_grpcio", "android_arm64_armv8-a_source").Output("buf_grpc.rs") + librust_grpcio_out := ctx.ModuleForTests("librust_grpcio", "android_arm64_armv8-a_source").Output("foo_grpc.rs") cmd := librust_grpcio_out.RuleParams.Command if w := "protoc-gen-grpc"; !strings.Contains(cmd, w) { t.Errorf("expected %q in %q", w, cmd) } - // Check exported include directories - if w := "-Ishared_include"; !strings.Contains(cmd, w) { - t.Errorf("expected %q in %q", w, cmd) - } - if w := "-Istatic_include"; !strings.Contains(cmd, w) { - t.Errorf("expected %q in %q", w, cmd) - } - // Check that we're including the exported directory from libprotobuf-cpp-full if w := "-Ilibprotobuf-cpp-full-includes"; !strings.Contains(cmd, w) { t.Errorf("expected %q in %q", w, cmd) @@ -132,3 +112,25 @@ func TestRustGrpcio(t *testing.T) { librust_grpcio_outputs) } } + +func TestRustProtoErrors(t *testing.T) { + testRustError(t, "A proto can only be added once to either grpc_protos or protos.*", ` + rust_protobuf { + name: "librust_grpcio", + protos: ["buf.proto"], + grpc_protos: ["buf.proto"], + crate_name: "rust_grpcio", + source_stem: "buf", + } + `) + + testRustError(t, "proto filenames must be unique across 'protos' and 'grpc_protos'.*", ` + rust_protobuf { + name: "librust_grpcio", + protos: ["buf.proto"], + grpc_protos: ["proto/buf.proto"], + crate_name: "rust_grpcio", + source_stem: "buf", + } + `) +} diff --git a/rust/rust_test.go b/rust/rust_test.go index 4edc6cd85..48c8d74f6 100644 --- a/rust/rust_test.go +++ b/rust/rust_test.go @@ -106,14 +106,16 @@ func newTestRustCtx(t *testing.T, bp string) *testRustCtx { // useMockedFs setup a default mocked filesystem for the test environment. func (tctx *testRustCtx) useMockedFs() { tctx.fs = map[string][]byte{ - "foo.rs": nil, - "foo.c": nil, - "src/bar.rs": nil, - "src/any.h": nil, - "proto.proto": nil, - "buf.proto": nil, - "liby.so": nil, - "libz.so": nil, + "foo.rs": nil, + "foo.c": nil, + "src/bar.rs": nil, + "src/any.h": nil, + "proto.proto": nil, + "proto/buf.proto": nil, + "buf.proto": nil, + "foo.proto": nil, + "liby.so": nil, + "libz.so": nil, } } diff --git a/rust/testing.go b/rust/testing.go index 963a4eaf7..07f557ab8 100644 --- a/rust/testing.go +++ b/rust/testing.go @@ -153,8 +153,6 @@ func RegisterRequiredBuildComponentsForTest(ctx android.RegistrationContext) { ctx.RegisterModuleType("rust_ffi_host", RustFFIHostFactory) ctx.RegisterModuleType("rust_ffi_host_shared", RustFFISharedHostFactory) ctx.RegisterModuleType("rust_ffi_host_static", RustFFIStaticHostFactory) - ctx.RegisterModuleType("rust_grpcio", RustGrpcioFactory) - ctx.RegisterModuleType("rust_grpcio_host", RustGrpcioHostFactory) ctx.RegisterModuleType("rust_proc_macro", ProcMacroFactory) ctx.RegisterModuleType("rust_protobuf", RustProtobufFactory) ctx.RegisterModuleType("rust_protobuf_host", RustProtobufHostFactory)