From 33056e8a9afa4a99ba685fce7168bdf481230b21 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Fri, 19 Feb 2021 13:49:08 +0000 Subject: [PATCH] Export aidl, proto and sysprop generated headers separately Previously, a cc library that included .aidl, .proto and/or .sysprop files and exported headers generated from at least one of those types would actually export generated headers from all of them. While headers generated from .sysprop files are always exported those generated from .aidl or .proto should only be exported when explicitly requested. This change treats them separately as expected. It has the potential to break the build as it could reduce the set of headers exported and so a dependent module that needed those would break. The fix in that case is to simply add one (or both) of the following to the module that previously exported those headers: aidl: { export_aidl_headers: true, } proto: { export_proto_headers: true, } Bug: 180712399 Test: m droid Change-Id: I488182e27dd423d261443612f98d5c112dd3ef8f --- cc/cc_test.go | 23 +++-------------------- cc/compiler.go | 5 ++++- cc/gen.go | 51 +++++++++++++++++++++++++++++++++++++++++++++----- cc/library.go | 14 ++++++-------- 4 files changed, 59 insertions(+), 34 deletions(-) diff --git a/cc/cc_test.go b/cc/cc_test.go index 942e397ef..37cbff115 100644 --- a/cc/cc_test.go +++ b/cc/cc_test.go @@ -4092,7 +4092,6 @@ func TestIncludeDirsExporting(t *testing.T) { ) }) - // TODO: fix this test as it exports all generated headers. t.Run("ensure only aidl headers are exported", func(t *testing.T) { ctx := testCc(t, genRuleModules+` cc_library_shared { @@ -4117,18 +4116,15 @@ func TestIncludeDirsExporting(t *testing.T) { .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/b.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bnb.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bpb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/proto/a.pb.h `), expectedOrderOnlyDeps(` .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/b.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bnb.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bpb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/proto/a.pb.h `), ) }) - // TODO: fix this test as it exports all generated headers. t.Run("ensure only proto headers are exported", func(t *testing.T) { ctx := testCc(t, genRuleModules+` cc_library_shared { @@ -4150,22 +4146,17 @@ func TestIncludeDirsExporting(t *testing.T) { `), expectedSystemIncludeDirs(``), expectedGeneratedHeaders(` - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/b.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bnb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bpb.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/proto/a.pb.h `), expectedOrderOnlyDeps(` - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/b.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bnb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bpb.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/proto/a.pb.h `), ) }) - // TODO: fix this test as it exports all generated headers including public and non-public. - t.Run("ensure only non-public sysprop headers are exported", func(t *testing.T) { + // TODO fix this test as it exports the sysprop public and non-public headers irrespective of + // which include directory is exported. + t.Run("ensure only sysprop headers are exported", func(t *testing.T) { ctx := testCc(t, genRuleModules+` cc_library_shared { name: "libfoo", @@ -4186,18 +4177,10 @@ func TestIncludeDirsExporting(t *testing.T) { expectedGeneratedHeaders(` .intermediates/libfoo/android_arm64_armv8-a_shared/gen/sysprop/include/a.sysprop.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/sysprop/public/include/a.sysprop.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/b.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bnb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bpb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/proto/a.pb.h `), expectedOrderOnlyDeps(` .intermediates/libfoo/android_arm64_armv8-a_shared/gen/sysprop/include/a.sysprop.h .intermediates/libfoo/android_arm64_armv8-a_shared/gen/sysprop/public/include/a.sysprop.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/b.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bnb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/aidl/Bpb.h - .intermediates/libfoo/android_arm64_armv8-a_shared/gen/proto/a.pb.h `), ) }) diff --git a/cc/compiler.go b/cc/compiler.go index 5f30d3d6e..e96295c2a 100644 --- a/cc/compiler.go +++ b/cc/compiler.go @@ -230,6 +230,8 @@ type baseCompiler struct { // other modules and filegroups. May include source files that have not yet been translated to // C/C++ (.aidl, .proto, etc.) srcsBeforeGen android.Paths + + generatedSourceInfo } var _ compiler = (*baseCompiler)(nil) @@ -634,10 +636,11 @@ func (compiler *baseCompiler) compile(ctx ModuleContext, flags Flags, deps PathD srcs := append(android.Paths(nil), compiler.srcsBeforeGen...) - srcs, genDeps := genSources(ctx, srcs, buildFlags) + srcs, genDeps, info := genSources(ctx, srcs, buildFlags) pathDeps = append(pathDeps, genDeps...) compiler.pathDeps = pathDeps + compiler.generatedSourceInfo = info compiler.cFlagsDeps = flags.CFlagsDeps // Save src, buildFlags and context diff --git a/cc/gen.go b/cc/gen.go index 75b9e49e5..83c019c33 100644 --- a/cc/gen.go +++ b/cc/gen.go @@ -220,8 +220,35 @@ func genWinMsg(ctx android.ModuleContext, srcFile android.Path, flags builderFla return rcFile, headerFile } +// Used to communicate information from the genSources method back to the library code that uses +// it. +type generatedSourceInfo struct { + // The headers created from .proto files + protoHeaders android.Paths + + // The files that can be used as order only dependencies in order to ensure that the proto header + // files are up to date. + protoOrderOnlyDeps android.Paths + + // The headers created from .aidl files + aidlHeaders android.Paths + + // The files that can be used as order only dependencies in order to ensure that the aidl header + // files are up to date. + aidlOrderOnlyDeps android.Paths + + // The headers created from .sysprop files + syspropHeaders android.Paths + + // The files that can be used as order only dependencies in order to ensure that the sysprop + // header files are up to date. + syspropOrderOnlyDeps android.Paths +} + func genSources(ctx android.ModuleContext, srcFiles android.Paths, - buildFlags builderFlags) (android.Paths, android.Paths) { + buildFlags builderFlags) (android.Paths, android.Paths, generatedSourceInfo) { + + var info generatedSourceInfo var deps android.Paths var rsFiles android.Paths @@ -258,7 +285,9 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, case ".proto": ccFile, headerFile := genProto(ctx, srcFile, buildFlags) srcFiles[i] = ccFile - deps = append(deps, headerFile) + info.protoHeaders = append(info.protoHeaders, headerFile) + // Use the generated header as an order only dep to ensure that it is up to date when needed. + info.protoOrderOnlyDeps = append(info.protoOrderOnlyDeps, headerFile) case ".aidl": if aidlRule == nil { aidlRule = android.NewRuleBuilder(pctx, ctx).Sbox(android.PathForModuleGen(ctx, "aidl"), @@ -267,7 +296,12 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, cppFile := android.GenPathWithExt(ctx, "aidl", srcFile, "cpp") depFile := android.GenPathWithExt(ctx, "aidl", srcFile, "cpp.d") srcFiles[i] = cppFile - deps = append(deps, genAidl(ctx, aidlRule, srcFile, cppFile, depFile, buildFlags.aidlFlags)...) + aidlHeaders := genAidl(ctx, aidlRule, srcFile, cppFile, depFile, buildFlags.aidlFlags) + info.aidlHeaders = append(info.aidlHeaders, aidlHeaders...) + // Use the generated headers as order only deps to ensure that they are up to date when + // needed. + // TODO: Reduce the size of the ninja file by using one order only dep for the whole rule + info.aidlOrderOnlyDeps = append(info.aidlOrderOnlyDeps, aidlHeaders...) case ".rscript", ".fs": cppFile := rsGeneratedCppFile(ctx, srcFile) rsFiles = append(rsFiles, srcFiles[i]) @@ -279,7 +313,10 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, case ".sysprop": cppFile, headerFiles := genSysprop(ctx, srcFile) srcFiles[i] = cppFile - deps = append(deps, headerFiles...) + info.syspropHeaders = append(info.syspropHeaders, headerFiles...) + // Use the generated headers as order only deps to ensure that they are up to date when + // needed. + info.syspropOrderOnlyDeps = append(info.syspropOrderOnlyDeps, headerFiles...) } } @@ -291,9 +328,13 @@ func genSources(ctx android.ModuleContext, srcFiles android.Paths, yaccRule_.Build("yacc", "gen yacc") } + deps = append(deps, info.protoOrderOnlyDeps...) + deps = append(deps, info.aidlOrderOnlyDeps...) + deps = append(deps, info.syspropOrderOnlyDeps...) + if len(rsFiles) > 0 { deps = append(deps, rsGenerateCpp(ctx, rsFiles, buildFlags.rsFlags)...) } - return srcFiles, deps + return srcFiles, deps, info } diff --git a/cc/library.go b/cc/library.go index 65533bc2b..f1363c9c1 100644 --- a/cc/library.go +++ b/cc/library.go @@ -1313,9 +1313,8 @@ func (library *libraryDecorator) link(ctx ModuleContext, dir := android.PathForModuleGen(ctx, "aidl") library.reexportDirs(dir) - // TODO: restrict to aidl deps - library.reexportDeps(library.baseCompiler.pathDeps...) - library.addExportedGeneratedHeaders(library.baseCompiler.pathDeps...) + library.reexportDeps(library.baseCompiler.aidlOrderOnlyDeps...) + library.addExportedGeneratedHeaders(library.baseCompiler.aidlHeaders...) } } @@ -1329,9 +1328,8 @@ func (library *libraryDecorator) link(ctx ModuleContext, includes = append(includes, flags.proto.Dir) library.reexportDirs(includes...) - // TODO: restrict to proto deps - library.reexportDeps(library.baseCompiler.pathDeps...) - library.addExportedGeneratedHeaders(library.baseCompiler.pathDeps...) + library.reexportDeps(library.baseCompiler.protoOrderOnlyDeps...) + library.addExportedGeneratedHeaders(library.baseCompiler.protoHeaders...) } } @@ -1352,8 +1350,8 @@ func (library *libraryDecorator) link(ctx ModuleContext, // Add sysprop-related directories to the exported directories of this library. library.reexportDirs(dir) - library.reexportDeps(library.baseCompiler.pathDeps...) - library.addExportedGeneratedHeaders(library.baseCompiler.pathDeps...) + library.reexportDeps(library.baseCompiler.syspropOrderOnlyDeps...) + library.addExportedGeneratedHeaders(library.baseCompiler.syspropHeaders...) } // Add stub-related flags if this library is a stub library.