Move proto compilation to RuleBuilder

Using blueprint.Rule for protoc commands was causing code duplication
because there was no good way to run the same protoc for cc, java and
python but then run custom source packaging steps for java and python.
Move most of the code into a common function that returns a
RuleBuilder, and then let java and python add their own commands at
the end of the rule.

Bug: 70706119
Test: All Soong tests
Test: m checkbuild
Change-Id: Ic692136775d273bcc4f4de99620ab4878667c83a

6 files changed, 68 insertions, 72 deletions

diff --git a/cc/builder.go b/cc/builder.go
index 6dd7c056..6b139f48 100644
--- a/cc/builder.go
+++ b/cc/builder.go
@@ -261,12 +261,9 @@ type builderFlags struct {
 	stripUseGnuStrip       bool
 
 	protoDeps        android.Paths
-	protoFlags       string
-	protoOutTypeFlag string
-	protoOutParams   string
+	proto            android.ProtoFlags
 	protoC           bool
 	protoOptionsFile bool
-	protoRoot        bool
 }
 
 type Objects struct {
diff --git a/cc/cc.go b/cc/cc.go
index f5a15678..8e55553a 100644
--- a/cc/cc.go
+++ b/cc/cc.go
@@ -162,13 +162,10 @@ type Flags struct {
 
 	GroupStaticLibs bool
 
+	proto            android.ProtoFlags
 	protoDeps        android.Paths
-	protoFlags       []string // Flags that apply to proto source files
-	protoOutTypeFlag string   // The output type, --cpp_out for example
-	protoOutParams   []string // Flags that modify the output of proto generated files
-	protoC           bool     // Whether to use C instead of C++
-	protoOptionsFile bool     // Whether to look for a .options file next to the .proto
-	ProtoRoot        bool
+	protoC           bool // Whether to use C instead of C++
+	protoOptionsFile bool // Whether to look for a .options file next to the .proto
 }
 
 type ObjectLinkerProperties struct {
diff --git a/cc/library.go b/cc/library.go
index cab75acb..5556fd4f 100644
--- a/cc/library.go
+++ b/cc/library.go
@@ -848,10 +848,10 @@ func (library *libraryDecorator) link(ctx ModuleContext,
 	if Bool(library.Properties.Proto.Export_proto_headers) {
 		if library.baseCompiler.hasSrcExt(".proto") {
 			includes := []string{}
-			if flags.ProtoRoot {
-				includes = append(includes, "-I"+android.ProtoSubDir(ctx).String())
+			if flags.proto.CanonicalPathFromRoot {
+				includes = append(includes, "-I"+flags.proto.SubDir.String())
 			}
-			includes = append(includes, "-I"+android.ProtoDir(ctx).String())
+			includes = append(includes, "-I"+flags.proto.Dir.String())
 			library.reexportFlags(includes)
 			library.reuseExportedFlags = append(library.reuseExportedFlags, includes...)
 			library.reexportDeps(library.baseCompiler.pathDeps) // TODO: restrict to proto deps
diff --git a/cc/proto.go b/cc/proto.go
index ce8a30ed..5a96ae78 100644
--- a/cc/proto.go
+++ b/cc/proto.go
@@ -15,47 +15,26 @@
 package cc
 
 import (
-	"strings"
-
-	"github.com/google/blueprint"
 	"github.com/google/blueprint/pathtools"
 
 	"android/soong/android"
 )
 
-func init() {
-	pctx.HostBinToolVariable("protocCmd", "aprotoc")
-	pctx.HostBinToolVariable("depFixCmd", "dep_fixer")
-}
-
-var (
-	proto = pctx.AndroidStaticRule("protoc",
-		blueprint.RuleParams{
-			Command: "$protocCmd $protoOut=$protoOutParams:$outDir --dependency_out=$out.d -I $protoBase $protoFlags $in && " +
-				`$depFixCmd $out.d`,
-			CommandDeps: []string{"$protocCmd", "$depFixCmd"},
-			Depfile:     "${out}.d",
-			Deps:        blueprint.DepsGCC,
-		}, "protoFlags", "protoOut", "protoOutParams", "protoBase", "outDir")
-)
-
 // genProto creates a rule to convert a .proto file to generated .pb.cc and .pb.h files and returns
 // the paths to the generated files.
-func genProto(ctx android.ModuleContext, protoFile android.Path, flags builderFlags) (ccFile, headerFile android.WritablePath) {
+func genProto(ctx android.ModuleContext, protoFile android.Path, flags builderFlags) (cc, header android.WritablePath) {
+	var ccFile, headerFile android.ModuleGenPath
 
 	srcSuffix := ".cc"
 	if flags.protoC {
 		srcSuffix = ".c"
 	}
 
-	var protoBase string
-	if flags.protoRoot {
-		protoBase = "."
+	if flags.proto.CanonicalPathFromRoot {
 		ccFile = android.GenPathWithExt(ctx, "proto", protoFile, "pb"+srcSuffix)
 		headerFile = android.GenPathWithExt(ctx, "proto", protoFile, "pb.h")
 	} else {
 		rel := protoFile.Rel()
-		protoBase = strings.TrimSuffix(protoFile.String(), rel)
 		ccFile = android.PathForModuleGen(ctx, "proto", pathtools.ReplaceExtension(rel, "pb"+srcSuffix))
 		headerFile = android.PathForModuleGen(ctx, "proto", pathtools.ReplaceExtension(rel, "pb.h"))
 	}
@@ -63,27 +42,19 @@ func genProto(ctx android.ModuleContext, protoFile android.Path, flags builderFl
 	protoDeps := flags.protoDeps
 	if flags.protoOptionsFile {
 		optionsFile := pathtools.ReplaceExtension(protoFile.String(), "options")
-		optionsPath := android.ExistentPathForSource(ctx, optionsFile)
-		if optionsPath.Valid() {
-			protoDeps = append(android.Paths{optionsPath.Path()}, protoDeps...)
-		}
+		optionsPath := android.PathForSource(ctx, optionsFile)
+		protoDeps = append(android.Paths{optionsPath}, protoDeps...)
 	}
 
-	ctx.Build(pctx, android.BuildParams{
-		Rule:           proto,
-		Description:    "protoc " + protoFile.Rel(),
-		Output:         ccFile,
-		ImplicitOutput: headerFile,
-		Input:          protoFile,
-		Implicits:      protoDeps,
-		Args: map[string]string{
-			"outDir":         android.ProtoDir(ctx).String(),
-			"protoFlags":     flags.protoFlags,
-			"protoOut":       flags.protoOutTypeFlag,
-			"protoOutParams": flags.protoOutParams,
-			"protoBase":      protoBase,
-		},
-	})
+	outDir := flags.proto.Dir
+	depFile := ccFile.ReplaceExtension(ctx, "d")
+	outputs := android.WritablePaths{ccFile, headerFile}
+
+	rule := android.NewRuleBuilder()
+
+	android.ProtoRule(ctx, rule, protoFile, flags.proto, protoDeps, outDir, depFile, outputs)
+
+	rule.Build(pctx, ctx, "protoc_"+protoFile.Rel(), "protoc "+protoFile.Rel())
 
 	return ccFile, headerFile
 }
@@ -143,13 +114,11 @@ func protoDeps(ctx BaseModuleContext, deps Deps, p *android.ProtoProperties, sta
 func protoFlags(ctx ModuleContext, flags Flags, p *android.ProtoProperties) Flags {
 	flags.CFlags = append(flags.CFlags, "-DGOOGLE_PROTOBUF_NO_RTTI")
 
-	flags.ProtoRoot = android.ProtoCanonicalPathFromRoot(ctx, p)
-	if flags.ProtoRoot {
-		flags.GlobalFlags = append(flags.GlobalFlags, "-I"+android.ProtoSubDir(ctx).String())
+	flags.proto = android.GetProtoFlags(ctx, p)
+	if flags.proto.CanonicalPathFromRoot {
+		flags.GlobalFlags = append(flags.GlobalFlags, "-I"+flags.proto.SubDir.String())
 	}
-	flags.GlobalFlags = append(flags.GlobalFlags, "-I"+android.ProtoDir(ctx).String())
-
-	flags.protoFlags = android.ProtoFlags(ctx, p)
+	flags.GlobalFlags = append(flags.GlobalFlags, "-I"+flags.proto.Dir.String())
 
 	var plugin string
 
@@ -157,18 +126,18 @@ func protoFlags(ctx ModuleContext, flags Flags, p *android.ProtoProperties) Flag
 	case "nanopb-c", "nanopb-c-enable_malloc", "nanopb-c-16bit", "nanopb-c-enable_malloc-16bit", "nanopb-c-32bit", "nanopb-c-enable_malloc-32bit":
 		flags.protoC = true
 		flags.protoOptionsFile = true
-		flags.protoOutTypeFlag = "--nanopb_out"
+		flags.proto.OutTypeFlag = "--nanopb_out"
 		plugin = "protoc-gen-nanopb"
 	case "full":
-		flags.protoOutTypeFlag = "--cpp_out"
+		flags.proto.OutTypeFlag = "--cpp_out"
 	case "lite":
-		flags.protoOutTypeFlag = "--cpp_out"
-		flags.protoOutParams = append(flags.protoOutParams, "lite")
+		flags.proto.OutTypeFlag = "--cpp_out"
+		flags.proto.OutParams = append(flags.proto.OutParams, "lite")
 	case "":
 		// TODO(b/119714316): this should be equivalent to "lite" in
 		// order to match protoDeps, but some modules are depending on
 		// this behavior
-		flags.protoOutTypeFlag = "--cpp_out"
+		flags.proto.OutTypeFlag = "--cpp_out"
 	default:
 		ctx.PropertyErrorf("proto.type", "unknown proto type %q",
 			String(p.Proto.Type))
@@ -177,7 +146,7 @@ func protoFlags(ctx ModuleContext, flags Flags, p *android.ProtoProperties) Flag
 	if plugin != "" {
 		path := ctx.Config().HostToolPath(ctx, plugin)
 		flags.protoDeps = append(flags.protoDeps, path)
-		flags.protoFlags = append(flags.protoFlags, "--plugin="+path.String())
+		flags.proto.Flags = append(flags.proto.Flags, "--plugin="+path.String())
 	}
 
 	return flags
diff --git a/cc/proto_test.go b/cc/proto_test.go
new file mode 100644
index 00000000..6fee9241
--- /dev/null
+++ b/cc/proto_test.go
@@ -0,0 +1,36 @@
+// Copyright 2016 Google Inc. All rights reserved.
+//
+// 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 cc
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestProto(t *testing.T) {
+	t.Run("simple", func(t *testing.T) {
+		ctx := testCc(t, `
+		cc_library_shared {
+			name: "libfoo",
+			srcs: ["a.proto"],
+		}`)
+
+		proto := ctx.ModuleForTests("libfoo", "android_arm_armv7-a-neon_core_shared").Output("proto/a.pb.cc")
+
+		if cmd := proto.RuleParams.Command; !strings.Contains(cmd, "--cpp_out=") {
+			t.Errorf("expected '--cpp_out' in %q", cmd)
+		}
+	})
+}
diff --git a/cc/util.go b/cc/util.go
index 782bf614..30a77a45 100644
--- a/cc/util.go
+++ b/cc/util.go
@@ -85,12 +85,9 @@ func flagsToBuilderFlags(in Flags) builderFlags {
 		groupStaticLibs: in.GroupStaticLibs,
 
 		protoDeps:        in.protoDeps,
-		protoFlags:       strings.Join(in.protoFlags, " "),
-		protoOutTypeFlag: in.protoOutTypeFlag,
-		protoOutParams:   strings.Join(in.protoOutParams, ","),
+		proto:            in.proto,
 		protoC:           in.protoC,
 		protoOptionsFile: in.protoOptionsFile,
-		protoRoot:        in.ProtoRoot,
 	}
 }