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

@@ -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)

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",
+		}
+	`)
+}

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,
 	}
 }
 

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)