Move proto compilation to RuleBuilder am: 19878da6a0

am: 61918685aa

Change-Id: I0cbb9ae4a408e8dd8316c41c7f2f3816da951364

Android.bp

@@ -193,6 +193,7 @@ bootstrap_go_package {
         "cc/gen_test.go",
         "cc/genrule_test.go",
         "cc/library_test.go",
+        "cc/proto_test.go",
         "cc/test_data_test.go",
         "cc/util_test.go",
     ],

android/proto.go

@@ -14,6 +14,12 @@
 
 package android
 
+import (
+	"strings"
+
+	"github.com/google/blueprint/proptools"
+)
+
 // TODO(ccross): protos are often used to communicate between multiple modules.  If the only
 // way to convert a proto to source is to reference it as a source file, and external modules cannot
 // reference source files in other modules, then every module that owns a proto file will need to
@@ -22,9 +28,17 @@ package android
 // and then external modules could depend on the proto module but use their own settings to
 // generate the source.
 
-func ProtoFlags(ctx ModuleContext, p *ProtoProperties) []string {
+type ProtoFlags struct {
-	protoFlags := []string{}
+	Flags                 []string
+	CanonicalPathFromRoot bool
+	Dir                   ModuleGenPath
+	SubDir                ModuleGenPath
+	OutTypeFlag           string
+	OutParams             []string
+}
 
+func GetProtoFlags(ctx ModuleContext, p *ProtoProperties) ProtoFlags {
+	var protoFlags []string
 	if len(p.Proto.Local_include_dirs) > 0 {
 		localProtoIncludeDirs := PathsForModuleSrc(ctx, p.Proto.Local_include_dirs)
 		protoFlags = append(protoFlags, JoinWithPrefix(localProtoIncludeDirs.Strings(), "-I"))
@@ -34,24 +48,12 @@ func ProtoFlags(ctx ModuleContext, p *ProtoProperties) []string {
 		protoFlags = append(protoFlags, JoinWithPrefix(rootProtoIncludeDirs.Strings(), "-I"))
 	}
 
-	return protoFlags
+	return ProtoFlags{
-}
+		Flags:                 protoFlags,
-
+		CanonicalPathFromRoot: proptools.BoolDefault(p.Proto.Canonical_path_from_root, true),
-func ProtoCanonicalPathFromRoot(ctx ModuleContext, p *ProtoProperties) bool {
+		Dir:                   PathForModuleGen(ctx, "proto"),
-	if p.Proto.Canonical_path_from_root == nil {
+		SubDir:                PathForModuleGen(ctx, "proto", ctx.ModuleDir()),
-		return true
 	}
-	return *p.Proto.Canonical_path_from_root
-}
-
-// ProtoDir returns the module's "gen/proto" directory
-func ProtoDir(ctx ModuleContext) ModuleGenPath {
-	return PathForModuleGen(ctx, "proto")
-}
-
-// ProtoSubDir returns the module's "gen/proto/path/to/module" directory
-func ProtoSubDir(ctx ModuleContext) ModuleGenPath {
-	return PathForModuleGen(ctx, "proto", ctx.ModuleDir())
 }
 
 type ProtoProperties struct {
@@ -76,3 +78,28 @@ type ProtoProperties struct {
 		Canonical_path_from_root *bool
 	} `android:"arch_variant"`
 }
+
+func ProtoRule(ctx ModuleContext, rule *RuleBuilder, protoFile Path, flags ProtoFlags, deps Paths,
+	outDir WritablePath, depFile WritablePath, outputs WritablePaths) {
+
+	var protoBase string
+	if flags.CanonicalPathFromRoot {
+		protoBase = "."
+	} else {
+		rel := protoFile.Rel()
+		protoBase = strings.TrimSuffix(protoFile.String(), rel)
+	}
+
+	rule.Command().
+		Tool(ctx.Config().HostToolPath(ctx, "aprotoc")).
+		FlagWithArg(flags.OutTypeFlag+"=", strings.Join(flags.OutParams, ",")+":"+outDir.String()).
+		FlagWithDepFile("--dependency_out=", depFile).
+		FlagWithArg("-I ", protoBase).
+		Flags(flags.Flags).
+		Input(protoFile).
+		Implicits(deps).
+		ImplicitOutputs(outputs)
+
+	rule.Command().
+		Tool(ctx.Config().HostToolPath(ctx, "dep_fixer")).Flag(depFile.String())
+}

cc/builder.go

@@ -261,12 +261,9 @@ type builderFlags struct {
 	stripUseGnuStrip       bool
 
 	protoDeps        android.Paths
-	protoFlags       string
+	proto            android.ProtoFlags
-	protoOutTypeFlag string
-	protoOutParams   string
 	protoC           bool
 	protoOptionsFile bool
-	protoRoot        bool
 }
 
 type Objects struct {

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
+	protoC           bool // Whether to use C instead of C++
-	protoOutTypeFlag string   // The output type, --cpp_out for example
+	protoOptionsFile bool // Whether to look for a .options file next to the .proto
-	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
 }
 
 type ObjectLinkerProperties struct {

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 {
+			if flags.proto.CanonicalPathFromRoot {
-				includes = append(includes, "-I"+android.ProtoSubDir(ctx).String())
+				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

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.proto.CanonicalPathFromRoot {
-	if flags.protoRoot {
-		protoBase = "."
 		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)
+		optionsPath := android.PathForSource(ctx, optionsFile)
-		if optionsPath.Valid() {
+		protoDeps = append(android.Paths{optionsPath}, protoDeps...)
-			protoDeps = append(android.Paths{optionsPath.Path()}, protoDeps...)
-		}
 	}
 
-	ctx.Build(pctx, android.BuildParams{
+	outDir := flags.proto.Dir
-		Rule:           proto,
+	depFile := ccFile.ReplaceExtension(ctx, "d")
-		Description:    "protoc " + protoFile.Rel(),
+	outputs := android.WritablePaths{ccFile, headerFile}
-		Output:         ccFile,
+
-		ImplicitOutput: headerFile,
+	rule := android.NewRuleBuilder()
-		Input:          protoFile,
+
-		Implicits:      protoDeps,
+	android.ProtoRule(ctx, rule, protoFile, flags.proto, protoDeps, outDir, depFile, outputs)
-		Args: map[string]string{
+
-			"outDir":         android.ProtoDir(ctx).String(),
+	rule.Build(pctx, ctx, "protoc_"+protoFile.Rel(), "protoc "+protoFile.Rel())
-			"protoFlags":     flags.protoFlags,
-			"protoOut":       flags.protoOutTypeFlag,
-			"protoOutParams": flags.protoOutParams,
-			"protoBase":      protoBase,
-		},
-	})
 
 	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)
+	flags.proto = android.GetProtoFlags(ctx, p)
-	if flags.ProtoRoot {
+	if flags.proto.CanonicalPathFromRoot {
-		flags.GlobalFlags = append(flags.GlobalFlags, "-I"+android.ProtoSubDir(ctx).String())
+		flags.GlobalFlags = append(flags.GlobalFlags, "-I"+flags.proto.SubDir.String())
 	}
-	flags.GlobalFlags = append(flags.GlobalFlags, "-I"+android.ProtoDir(ctx).String())
+	flags.GlobalFlags = append(flags.GlobalFlags, "-I"+flags.proto.Dir.String())
-
-	flags.protoFlags = android.ProtoFlags(ctx, p)
 
 	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.proto.OutTypeFlag = "--cpp_out"
-		flags.protoOutParams = append(flags.protoOutParams, "lite")
+		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

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)
+		}
+	})
+}

cc/util.go

@@ -85,12 +85,9 @@ func flagsToBuilderFlags(in Flags) builderFlags {
 		groupStaticLibs: in.GroupStaticLibs,
 
 		protoDeps:        in.protoDeps,
-		protoFlags:       strings.Join(in.protoFlags, " "),
+		proto:            in.proto,
-		protoOutTypeFlag: in.protoOutTypeFlag,
-		protoOutParams:   strings.Join(in.protoOutParams, ","),
 		protoC:           in.protoC,
 		protoOptionsFile: in.protoOptionsFile,
-		protoRoot:        in.ProtoRoot,
 	}
 }
 

java/builder.go

@@ -153,10 +153,7 @@ type javaBuilderFlags struct {
 	kotlincFlags     string
 	kotlincClasspath classpath
 
-	protoFlags       []string
+	proto android.ProtoFlags
-	protoOutTypeFlag string // The flag itself: --java_out
-	protoOutParams   string // Parameters to that flag: --java_out=$protoOutParams:$outDir
-	protoRoot        bool
 }
 
 func TransformJavaToClasses(ctx android.ModuleContext, outputFile android.WritablePath, shardIdx int,

java/proto.go

@@ -15,57 +15,34 @@
 package java
 
 import (
-	"strings"
-
-	"github.com/google/blueprint"
-
 	"android/soong/android"
 )
 
-func init() {
-	pctx.HostBinToolVariable("protocCmd", "aprotoc")
-	pctx.HostBinToolVariable("depFixCmd", "dep_fixer")
-}
-
-var (
-	proto = pctx.AndroidStaticRule("protoc",
-		blueprint.RuleParams{
-			Command: `rm -rf $out.tmp && mkdir -p $out.tmp && ` +
-				`$protocCmd $protoOut=$protoOutParams:$out.tmp --dependency_out=$out.d -I $protoBase $protoFlags $in && ` +
-				`$depFixCmd $out.d && ` +
-				`${config.SoongZipCmd} -jar -o $out -C $out.tmp -D $out.tmp && rm -rf $out.tmp`,
-			CommandDeps: []string{
-				"$protocCmd",
-				"$depFixCmd",
-				"${config.SoongZipCmd}",
-			},
-			Depfile: "${out}.d",
-			Deps:    blueprint.DepsGCC,
-		}, "protoBase", "protoFlags", "protoOut", "protoOutParams")
-)
-
 func genProto(ctx android.ModuleContext, protoFile android.Path, flags javaBuilderFlags) android.Path {
 	srcJarFile := android.GenPathWithExt(ctx, "proto", protoFile, "srcjar")
 
-	var protoBase string
+	outDir := srcJarFile.ReplaceExtension(ctx, "tmp")
-	if flags.protoRoot {
+	depFile := srcJarFile.ReplaceExtension(ctx, "srcjar.d")
-		protoBase = "."
-	} else {
-		protoBase = strings.TrimSuffix(protoFile.String(), protoFile.Rel())
-	}
 
-	ctx.Build(pctx, android.BuildParams{
+	rule := android.NewRuleBuilder()
-		Rule:        proto,
+
-		Description: "protoc " + protoFile.Rel(),
+	rule.Command().Text("rm -rf").Flag(outDir.String())
-		Output:      srcJarFile,
+	rule.Command().Text("mkdir -p").Flag(outDir.String())
-		Input:       protoFile,
+
-		Args: map[string]string{
+	android.ProtoRule(ctx, rule, protoFile, flags.proto, nil, outDir, depFile, nil)
-			"protoBase":      protoBase,
+
-			"protoOut":       flags.protoOutTypeFlag,
+	// Proto generated java files have an unknown package name in the path, so package the entire output directory
-			"protoOutParams": flags.protoOutParams,
+	// into a srcjar.
-			"protoFlags":     strings.Join(flags.protoFlags, " "),
+	rule.Command().
-		},
+		Tool(ctx.Config().HostToolPath(ctx, "soong_zip")).
-	})
+		Flag("-jar").
+		FlagWithOutput("-o ", srcJarFile).
+		FlagWithArg("-C ", outDir.String()).
+		FlagWithArg("-D ", outDir.String())
+
+	rule.Command().Text("rm -rf").Flag(outDir.String())
+
+	rule.Build(pctx, ctx, "protoc_"+protoFile.Rel(), "protoc "+protoFile.Rel())
 
 	return srcJarFile
 }
@@ -93,30 +70,24 @@ func protoDeps(ctx android.BottomUpMutatorContext, p *android.ProtoProperties) {
 func protoFlags(ctx android.ModuleContext, j *CompilerProperties, p *android.ProtoProperties,
 	flags javaBuilderFlags) javaBuilderFlags {
 
+	flags.proto = android.GetProtoFlags(ctx, p)
+
 	switch String(p.Proto.Type) {
 	case "micro":
-		flags.protoOutTypeFlag = "--javamicro_out"
+		flags.proto.OutTypeFlag = "--javamicro_out"
 	case "nano":
-		flags.protoOutTypeFlag = "--javanano_out"
+		flags.proto.OutTypeFlag = "--javanano_out"
 	case "lite":
-		flags.protoOutTypeFlag = "--java_out"
+		flags.proto.OutTypeFlag = "--java_out"
-		flags.protoOutParams = "lite"
+		flags.proto.OutParams = append(flags.proto.OutParams, "lite")
 	case "full", "":
-		flags.protoOutTypeFlag = "--java_out"
+		flags.proto.OutTypeFlag = "--java_out"
 	default:
 		ctx.PropertyErrorf("proto.type", "unknown proto type %q",
 			String(p.Proto.Type))
 	}
 
-	if len(j.Proto.Output_params) > 0 {
+	flags.proto.OutParams = append(flags.proto.OutParams, j.Proto.Output_params...)
-		if flags.protoOutParams != "" {
-			flags.protoOutParams += ","
-		}
-		flags.protoOutParams += strings.Join(j.Proto.Output_params, ",")
-	}
-
-	flags.protoFlags = android.ProtoFlags(ctx, p)
-	flags.protoRoot = android.ProtoCanonicalPathFromRoot(ctx, p)
 
 	return flags
 }

python/proto.go

@@ -16,58 +16,35 @@ package python
 
 import (
 	"android/soong/android"
-	"strings"
-
-	"github.com/google/blueprint"
 )
 
-func init() {
+func genProto(ctx android.ModuleContext, protoFile android.Path, flags android.ProtoFlags, pkgPath string) android.Path {
-	pctx.HostBinToolVariable("protocCmd", "aprotoc")
+	srcsZipFile := android.PathForModuleGen(ctx, protoFile.Base()+".srcszip")
-}
 
-var (
+	outDir := srcsZipFile.ReplaceExtension(ctx, "tmp")
-	proto = pctx.AndroidStaticRule("protoc",
+	depFile := srcsZipFile.ReplaceExtension(ctx, "srcszip.d")
-		blueprint.RuleParams{
-			Command: `rm -rf $out.tmp && mkdir -p $out.tmp && ` +
-				`$protocCmd --python_out=$out.tmp --dependency_out=$out.d -I $protoBase $protoFlags $in && ` +
-				`$parCmd -o $out $pkgPathArgs -C $out.tmp -D $out.tmp && rm -rf $out.tmp`,
-			CommandDeps: []string{
-				"$protocCmd",
-				"$parCmd",
-			},
-			Depfile: "${out}.d",
-			Deps:    blueprint.DepsGCC,
-		}, "protoBase", "protoFlags", "pkgPathArgs")
-)
 
-func genProto(ctx android.ModuleContext, p *android.ProtoProperties,
+	rule := android.NewRuleBuilder()
-	protoFile android.Path, protoFlags []string, pkgPath string) android.Path {
-	srcJarFile := android.PathForModuleGen(ctx, protoFile.Base()+".srcszip")
 
-	protoRoot := android.ProtoCanonicalPathFromRoot(ctx, p)
+	rule.Command().Text("rm -rf").Flag(outDir.String())
+	rule.Command().Text("mkdir -p").Flag(outDir.String())
 
-	var protoBase string
+	android.ProtoRule(ctx, rule, protoFile, flags, nil, outDir, depFile, nil)
-	if protoRoot {
-		protoBase = "."
-	} else {
-		protoBase = strings.TrimSuffix(protoFile.String(), protoFile.Rel())
-	}
 
-	var pkgPathArgs string
+	// Proto generated python files have an unknown package name in the path, so package the entire output directory
+	// into a srcszip.
+	zipCmd := rule.Command().
+		Tool(ctx.Config().HostToolPath(ctx, "soong_zip")).
+		FlagWithOutput("-o ", srcsZipFile).
+		FlagWithArg("-C ", outDir.String()).
+		FlagWithArg("-D ", outDir.String())
 	if pkgPath != "" {
-		pkgPathArgs = "-P " + pkgPath
+		zipCmd.FlagWithArg("-P ", pkgPath)
 	}
-	ctx.Build(pctx, android.BuildParams{
-		Rule:        proto,
-		Description: "protoc " + protoFile.Rel(),
-		Output:      srcJarFile,
-		Input:       protoFile,
-		Args: map[string]string{
-			"protoBase":   protoBase,
-			"protoFlags":  strings.Join(protoFlags, " "),
-			"pkgPathArgs": pkgPathArgs,
-		},
-	})
 
-	return srcJarFile
+	rule.Command().Text("rm -rf").Flag(outDir.String())
+
+	rule.Build(pctx, ctx, "protoc_"+protoFile.Rel(), "protoc "+protoFile.Rel())
+
+	return srcsZipFile
 }

python/python.go

@@ -516,9 +516,11 @@ func (p *Module) createSrcsZip(ctx android.ModuleContext, pkgPath string) androi
 	}
 	var zips android.Paths
 	if len(protoSrcs) > 0 {
+		protoFlags := android.GetProtoFlags(ctx, &p.protoProperties)
+		protoFlags.OutTypeFlag = "--python_out"
+
 		for _, srcFile := range protoSrcs {
-			zip := genProto(ctx, &p.protoProperties, srcFile,
+			zip := genProto(ctx, srcFile, protoFlags, pkgPath)
-				android.ProtoFlags(ctx, &p.protoProperties), pkgPath)
 			zips = append(zips, zip)
 		}
 	}