From ab9f4268c01390b9d049c746e399b53a0f2c6dce Mon Sep 17 00:00:00 2001 From: Dan Willemsen Date: Wed, 14 Feb 2018 13:58:34 -0800 Subject: [PATCH] Add proto.canonical_path_from_root Historically, we've always passed '-I .' as the first argument to protoc, essentially treating all proto file package names as their full path in the android source tree. This would make sense in a monorepo world, but it makes less sense when we're pulling in external projects with established package names. So keep the same default (for now), but allow individual builds to opt into using local paths as the default names with 'canonical_path_from_root: false'. A cleanup effort and/or large scale change in the future could change the default to false. As part of this, run protoc once per input proto file, since the flags may need to change per-file. We'll also need this in order to specify --dependency_out in the future. Bug: 70704330 Test: aosp/master build-aosp_arm.ninja is identical Test: aosp/master soong/build.ninja has expected changes Test: m Test: Build protobuf test Change-Id: I9d6de9fd630326bbcced1c62a4a7e9546429b0ce --- android/proto.go | 19 +++++++++++++++++-- cc/builder.go | 1 + cc/cc.go | 1 + cc/gen.go | 2 +- cc/library.go | 11 ++++++----- cc/proto.go | 33 ++++++++++++++++++++++++--------- cc/util.go | 1 + java/builder.go | 1 + java/gen.go | 12 ++---------- java/proto.go | 36 +++++++++++++++++++++++------------- 10 files changed, 77 insertions(+), 40 deletions(-) diff --git a/android/proto.go b/android/proto.go index 0cf7c26e1..801837e01 100644 --- a/android/proto.go +++ b/android/proto.go @@ -23,8 +23,7 @@ package android // generate the source. func ProtoFlags(ctx ModuleContext, p *ProtoProperties) []string { - // -I . must come first, it affects where protoc places the output files. - protoFlags := []string{"-I ."} + protoFlags := []string{} if len(p.Proto.Local_include_dirs) > 0 { localProtoIncludeDirs := PathsForModuleSrc(ctx, p.Proto.Local_include_dirs) @@ -38,6 +37,13 @@ func ProtoFlags(ctx ModuleContext, p *ProtoProperties) []string { return protoFlags } +func ProtoCanonicalPathFromRoot(ctx ModuleContext, p *ProtoProperties) bool { + if p.Proto.Canonical_path_from_root == nil { + 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") @@ -59,5 +65,14 @@ type ProtoProperties struct { // list of directories relative to the bp file that will // be added to the protoc include paths. Local_include_dirs []string + + // whether to identify the proto files from the root of the + // source tree (the original method in Android, useful for + // android-specific protos), or relative from where they were + // specified (useful for external/third party protos). + // + // This defaults to true today, but is expected to default to + // false in the future. + Canonical_path_from_root *bool } `android:"arch_variant"` } diff --git a/cc/builder.go b/cc/builder.go index 8f253caac..1d12b5f09 100644 --- a/cc/builder.go +++ b/cc/builder.go @@ -252,6 +252,7 @@ type builderFlags struct { tidy bool coverage bool sAbiDump bool + protoRoot bool systemIncludeFlags string diff --git a/cc/cc.go b/cc/cc.go index 05a1579b4..887f39435 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -134,6 +134,7 @@ type Flags struct { Tidy bool Coverage bool SAbiDump bool + ProtoRoot bool RequiredInstructionSet string DynamicLinker string diff --git a/cc/gen.go b/cc/gen.go index be5e4fc03..f22a7837c 100644 --- a/cc/gen.go +++ b/cc/gen.go @@ -154,7 +154,7 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, genLex(ctx, srcFile, cppFile) case ".proto": ccFile, headerFile := genProto(ctx, srcFile, buildFlags.protoFlags, - buildFlags.protoOutParams) + buildFlags.protoOutParams, buildFlags.protoRoot) srcFiles[i] = ccFile deps = append(deps, headerFile) case ".aidl": diff --git a/cc/library.go b/cc/library.go index f8e20e242..d3717dd9e 100644 --- a/cc/library.go +++ b/cc/library.go @@ -690,12 +690,13 @@ func (library *libraryDecorator) link(ctx ModuleContext, if Bool(library.Properties.Proto.Export_proto_headers) { if library.baseCompiler.hasSrcExt(".proto") { - flags := []string{ - "-I" + android.ProtoSubDir(ctx).String(), - "-I" + android.ProtoDir(ctx).String(), + includes := []string{} + if flags.ProtoRoot { + includes = append(includes, "-I"+android.ProtoSubDir(ctx).String()) } - library.reexportFlags(flags) - library.reuseExportedFlags = append(library.reuseExportedFlags, flags...) + includes = append(includes, "-I"+android.ProtoDir(ctx).String()) + library.reexportFlags(includes) + library.reuseExportedFlags = append(library.reuseExportedFlags, includes...) library.reexportDeps(library.baseCompiler.pathDeps) // TODO: restrict to proto deps library.reuseExportedDeps = append(library.reuseExportedDeps, library.baseCompiler.pathDeps...) } diff --git a/cc/proto.go b/cc/proto.go index 3b5fd3b63..c53dcf405 100644 --- a/cc/proto.go +++ b/cc/proto.go @@ -15,7 +15,10 @@ package cc import ( + "strings" + "github.com/google/blueprint" + "github.com/google/blueprint/pathtools" "github.com/google/blueprint/proptools" "android/soong/android" @@ -28,18 +31,27 @@ func init() { var ( proto = pctx.AndroidStaticRule("protoc", blueprint.RuleParams{ - Command: "$protocCmd --cpp_out=$protoOutParams:$outDir $protoFlags $in", + Command: "$protocCmd --cpp_out=$protoOutParams:$outDir -I $protoBase $protoFlags $in", CommandDeps: []string{"$protocCmd"}, - }, "protoFlags", "protoOutParams", "outDir") + }, "protoFlags", "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, - protoFlags string, protoOutParams string) (ccFile, headerFile android.WritablePath) { + protoFlags, protoOutParams string, root bool) (ccFile, headerFile android.WritablePath) { - ccFile = android.GenPathWithExt(ctx, "proto", protoFile, "pb.cc") - headerFile = android.GenPathWithExt(ctx, "proto", protoFile, "pb.h") + var protoBase string + if root { + protoBase = "." + ccFile = android.GenPathWithExt(ctx, "proto", protoFile, "pb.cc") + 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.cc")) + headerFile = android.PathForModuleGen(ctx, "proto", pathtools.ReplaceExtension(rel, "pb.h")) + } ctx.Build(pctx, android.BuildParams{ Rule: proto, @@ -50,6 +62,7 @@ func genProto(ctx android.ModuleContext, protoFile android.Path, "outDir": android.ProtoDir(ctx).String(), "protoFlags": protoFlags, "protoOutParams": protoOutParams, + "protoBase": protoBase, }, }) @@ -92,10 +105,12 @@ 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.GlobalFlags = append(flags.GlobalFlags, - "-I"+android.ProtoSubDir(ctx).String(), - "-I"+android.ProtoDir(ctx).String(), - ) + + flags.ProtoRoot = android.ProtoCanonicalPathFromRoot(ctx, p) + if flags.ProtoRoot { + flags.GlobalFlags = append(flags.GlobalFlags, "-I"+android.ProtoSubDir(ctx).String()) + } + flags.GlobalFlags = append(flags.GlobalFlags, "-I"+android.ProtoDir(ctx).String()) flags.protoFlags = android.ProtoFlags(ctx, p) diff --git a/cc/util.go b/cc/util.go index 92a32bc9a..aaf0f71c8 100644 --- a/cc/util.go +++ b/cc/util.go @@ -80,6 +80,7 @@ func flagsToBuilderFlags(in Flags) builderFlags { coverage: in.Coverage, tidy: in.Tidy, sAbiDump: in.SAbiDump, + protoRoot: in.ProtoRoot, systemIncludeFlags: strings.Join(in.SystemIncludeFlags, " "), diff --git a/java/builder.go b/java/builder.go index 5aea1cb1c..b8f8351fb 100644 --- a/java/builder.go +++ b/java/builder.go @@ -169,6 +169,7 @@ type javaBuilderFlags struct { protoFlags []string protoOutTypeFlag string // The flag itself: --java_out protoOutParams string // Parameters to that flag: --java_out=$protoOutParams:$outDir + protoRoot bool } func TransformKotlinToClasses(ctx android.ModuleContext, outputFile android.WritablePath, diff --git a/java/gen.go b/java/gen.go index 4893e889b..a99382974 100644 --- a/java/gen.go +++ b/java/gen.go @@ -85,7 +85,6 @@ func genLogtags(ctx android.ModuleContext, logtagsFile android.Path) android.Pat func (j *Module) genSources(ctx android.ModuleContext, srcFiles android.Paths, flags javaBuilderFlags) android.Paths { - var protoFiles android.Paths outSrcFiles := make(android.Paths, 0, len(srcFiles)) for _, srcFile := range srcFiles { @@ -98,20 +97,13 @@ func (j *Module) genSources(ctx android.ModuleContext, srcFiles android.Paths, javaFile := genLogtags(ctx, srcFile) outSrcFiles = append(outSrcFiles, javaFile) case ".proto": - protoFiles = append(protoFiles, srcFile) + srcJarFile := genProto(ctx, srcFile, flags) + outSrcFiles = append(outSrcFiles, srcJarFile) default: outSrcFiles = append(outSrcFiles, srcFile) } } - if len(protoFiles) > 0 { - protoSrcJar := android.PathForModuleGen(ctx, "proto.srcjar") - genProto(ctx, protoSrcJar, protoFiles, - flags.protoFlags, flags.protoOutTypeFlag, flags.protoOutParams) - - outSrcFiles = append(outSrcFiles, protoSrcJar) - } - return outSrcFiles } diff --git a/java/proto.go b/java/proto.go index 226fac086..2991ad982 100644 --- a/java/proto.go +++ b/java/proto.go @@ -30,31 +30,40 @@ func init() { var ( proto = pctx.AndroidStaticRule("protoc", blueprint.RuleParams{ - Command: `rm -rf $outDir && mkdir -p $outDir && ` + - `$protocCmd $protoOut=$protoOutParams:$outDir $protoFlags $in && ` + - `${config.SoongZipCmd} -jar -o $out -C $outDir -D $outDir`, + Command: `rm -rf $out.tmp && mkdir -p $out.tmp && ` + + `$protocCmd $protoOut=$protoOutParams:$out.tmp -I $protoBase $protoFlags $in && ` + + `${config.SoongZipCmd} -jar -o $out -C $out.tmp -D $out.tmp && rm -rf $out.tmp`, CommandDeps: []string{ "$protocCmd", "${config.SoongZipCmd}", }, - }, "protoFlags", "protoOut", "protoOutParams", "outDir") + }, "protoBase", "protoFlags", "protoOut", "protoOutParams") ) -func genProto(ctx android.ModuleContext, outputSrcJar android.WritablePath, - protoFiles android.Paths, protoFlags []string, protoOut, protoOutParams string) { +func genProto(ctx android.ModuleContext, protoFile android.Path, flags javaBuilderFlags) android.Path { + srcJarFile := android.GenPathWithExt(ctx, "proto", protoFile, "srcjar") + + var protoBase string + if flags.protoRoot { + protoBase = "." + } else { + protoBase = strings.TrimSuffix(protoFile.String(), protoFile.Rel()) + } ctx.Build(pctx, android.BuildParams{ Rule: proto, - Description: "protoc " + protoFiles[0].Rel(), - Output: outputSrcJar, - Inputs: protoFiles, + Description: "protoc " + protoFile.Rel(), + Output: srcJarFile, + Input: protoFile, Args: map[string]string{ - "outDir": android.ProtoDir(ctx).String(), - "protoOut": protoOut, - "protoOutParams": protoOutParams, - "protoFlags": strings.Join(protoFlags, " "), + "protoBase": protoBase, + "protoOut": flags.protoOutTypeFlag, + "protoOutParams": flags.protoOutParams, + "protoFlags": strings.Join(flags.protoFlags, " "), }, }) + + return srcJarFile } func protoDeps(ctx android.BottomUpMutatorContext, p *android.ProtoProperties) { @@ -103,6 +112,7 @@ func protoFlags(ctx android.ModuleContext, j *CompilerProperties, p *android.Pro } flags.protoFlags = android.ProtoFlags(ctx, p) + flags.protoRoot = android.ProtoCanonicalPathFromRoot(ctx, p) return flags }