From d788b3e6cb8753ea3460bd2cb462aff27a966aef Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Tue, 28 Nov 2023 13:14:56 -0800 Subject: [PATCH] Merge aconfig files per-module Passing the list of all transitive aconfig files to Make causes extra Kati analysis runs when dependencies are changed in Android.bp files. Since Make is going to merge them anyways, merge them per-module and pass a single aconfig file to Make for each module. Fixes: 313698230 Test: m out/target/product/vsoc_x86_64/system/etc/aconfig_flags.pb Change-Id: Ifde4826bc93bc06e40338f72b4cb39eed26ca08d --- aconfig/aconfig_declarations.go | 51 ++++++++++++-------- aconfig/codegen/java_aconfig_library_test.go | 5 +- aconfig/init.go | 6 +++ apex/apex.go | 2 +- apex/apex_test.go | 4 +- bp2build/Android.bp | 2 +- bp2build/aconfig_conversion_test.go | 2 + cc/androidmk.go | 2 +- cc/cc.go | 8 +-- java/androidmk.go | 9 ++-- java/base.go | 12 ++--- 11 files changed, 56 insertions(+), 47 deletions(-) diff --git a/aconfig/aconfig_declarations.go b/aconfig/aconfig_declarations.go index e662f1da4..05a637129 100644 --- a/aconfig/aconfig_declarations.go +++ b/aconfig/aconfig_declarations.go @@ -126,7 +126,7 @@ var DeclarationsProviderKey = blueprint.NewProvider(DeclarationsProviderData{}) // This is used to collect the aconfig declarations info on the transitive closure, // the data is keyed on the container. type TransitiveDeclarationsInfo struct { - AconfigFiles map[string]*android.DepSet[android.Path] + AconfigFiles map[string]android.Paths } var TransitiveDeclarationsInfoProvider = blueprint.NewProvider(TransitiveDeclarationsInfo{}) @@ -177,40 +177,49 @@ func (module *DeclarationsModule) GenerateAndroidBuildActions(ctx android.Module }) } - -func CollectTransitiveAconfigFiles(ctx android.ModuleContext, transitiveAconfigFiles *map[string]*android.DepSet[android.Path]) { - if *transitiveAconfigFiles == nil { - *transitiveAconfigFiles = make(map[string]*android.DepSet[android.Path]) +func CollectDependencyAconfigFiles(ctx android.ModuleContext, mergedAconfigFiles *map[string]android.Paths) { + if *mergedAconfigFiles == nil { + *mergedAconfigFiles = make(map[string]android.Paths) } ctx.VisitDirectDeps(func(module android.Module) { if dep := ctx.OtherModuleProvider(module, DeclarationsProviderKey).(DeclarationsProviderData); dep.IntermediatePath != nil { - aconfigMap := make(map[string]*android.DepSet[android.Path]) - aconfigMap[dep.Container] = android.NewDepSet(android.POSTORDER, []android.Path{dep.IntermediatePath}, nil) - mergeTransitiveAconfigFiles(aconfigMap, *transitiveAconfigFiles) + (*mergedAconfigFiles)[dep.Container] = append((*mergedAconfigFiles)[dep.Container], dep.IntermediatePath) return } if dep := ctx.OtherModuleProvider(module, TransitiveDeclarationsInfoProvider).(TransitiveDeclarationsInfo); len(dep.AconfigFiles) > 0 { - mergeTransitiveAconfigFiles(dep.AconfigFiles, *transitiveAconfigFiles) + for container, v := range dep.AconfigFiles { + (*mergedAconfigFiles)[container] = append((*mergedAconfigFiles)[container], v...) + } } }) + for container, aconfigFiles := range *mergedAconfigFiles { + (*mergedAconfigFiles)[container] = mergeAconfigFiles(ctx, aconfigFiles) + } + ctx.SetProvider(TransitiveDeclarationsInfoProvider, TransitiveDeclarationsInfo{ - AconfigFiles: *transitiveAconfigFiles, + AconfigFiles: *mergedAconfigFiles, }) } -func mergeTransitiveAconfigFiles(from, to map[string]*android.DepSet[android.Path]) { - for fromKey, fromValue := range from { - if fromValue == nil { - continue - } - toValue, ok := to[fromKey] - if !ok { - to[fromKey] = fromValue - } else { - to[fromKey] = android.NewDepSet(android.POSTORDER, toValue.ToList(), []*android.DepSet[android.Path]{fromValue}) - } +func mergeAconfigFiles(ctx android.ModuleContext, inputs android.Paths) android.Paths { + if len(inputs) == 1 { + return android.Paths{inputs[0]} } + + output := android.PathForModuleOut(ctx, "aconfig_merged.pb") + + ctx.Build(pctx, android.BuildParams{ + Rule: mergeAconfigFilesRule, + Description: "merge aconfig files", + Inputs: inputs, + Output: output, + Args: map[string]string{ + "flags": android.JoinWithPrefix(inputs.Strings(), "--cache "), + }, + }) + + return android.Paths{output} } type bazelAconfigDeclarationsAttributes struct { diff --git a/aconfig/codegen/java_aconfig_library_test.go b/aconfig/codegen/java_aconfig_library_test.go index cbfdc2179..2523abcbc 100644 --- a/aconfig/codegen/java_aconfig_library_test.go +++ b/aconfig/codegen/java_aconfig_library_test.go @@ -60,9 +60,8 @@ func runJavaAndroidMkTest(t *testing.T, bp string) { entry := android.AndroidMkEntriesForTest(t, result.TestContext, module)[0] makeVar := entry.EntryMap["LOCAL_ACONFIG_FILES"] - android.AssertIntEquals(t, "len(LOCAL_ACONFIG_FILES)", 2, len(makeVar)) - android.EnsureListContainsSuffix(t, makeVar, "my_aconfig_declarations_foo/intermediate.pb") - android.EnsureListContainsSuffix(t, makeVar, "my_aconfig_declarations_bar/intermediate.pb") + android.AssertIntEquals(t, "len(LOCAL_ACONFIG_FILES)", 1, len(makeVar)) + android.EnsureListContainsSuffix(t, makeVar, "android_common/aconfig_merged.pb") } func TestAndroidMkJavaLibrary(t *testing.T) { diff --git a/aconfig/init.go b/aconfig/init.go index 79bf0027b..2e24db94d 100644 --- a/aconfig/init.go +++ b/aconfig/init.go @@ -48,6 +48,12 @@ var ( "${aconfig}", }, }, "cache_files") + + mergeAconfigFilesRule = pctx.AndroidStaticRule("mergeAconfigFilesRule", + blueprint.RuleParams{ + Command: `${aconfig} dump --dedup --format protobuf --out $out $flags`, + CommandDeps: []string{"${aconfig}"}, + }, "flags") ) func init() { diff --git a/apex/apex.go b/apex/apex.go index 5d67c7aa5..7ddb3e6c5 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -2394,7 +2394,7 @@ func (a *apexBundle) depVisitor(vctx *visitorContext, ctx android.ModuleContext, func addAconfigFiles(vctx *visitorContext, ctx android.ModuleContext, module blueprint.Module) { dep := ctx.OtherModuleProvider(module, aconfig.TransitiveDeclarationsInfoProvider).(aconfig.TransitiveDeclarationsInfo) if len(dep.AconfigFiles) > 0 && dep.AconfigFiles[ctx.ModuleName()] != nil { - vctx.aconfigFiles = append(vctx.aconfigFiles, dep.AconfigFiles[ctx.ModuleName()].ToList()...) + vctx.aconfigFiles = append(vctx.aconfigFiles, dep.AconfigFiles[ctx.ModuleName()]...) } } diff --git a/apex/apex_test.go b/apex/apex_test.go index 2f6acff58..2ed053ed8 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -11103,7 +11103,7 @@ func TestAconfigFilesJavaAndCcDeps(t *testing.T) { t.Fatalf("Expected 3 commands, got %d in:\n%s", len(aconfigArgs), s) } android.EnsureListContainsSuffix(t, aconfigArgs, "my_aconfig_declarations_foo/intermediate.pb") - android.EnsureListContainsSuffix(t, aconfigArgs, "my_aconfig_declarations_bar/intermediate.pb") + android.EnsureListContainsSuffix(t, aconfigArgs, "my_cc_library_bar/android_arm64_armv8-a_shared_apex10000/aconfig_merged.pb") android.EnsureListContainsSuffix(t, aconfigArgs, "my_aconfig_declarations_baz/intermediate.pb") buildParams := combineAconfigRule.BuildParams @@ -11111,7 +11111,7 @@ func TestAconfigFilesJavaAndCcDeps(t *testing.T) { t.Fatalf("Expected 3 input, got %d", len(buildParams.Inputs)) } android.EnsureListContainsSuffix(t, buildParams.Inputs.Strings(), "my_aconfig_declarations_foo/intermediate.pb") - android.EnsureListContainsSuffix(t, buildParams.Inputs.Strings(), "my_aconfig_declarations_bar/intermediate.pb") + android.EnsureListContainsSuffix(t, buildParams.Inputs.Strings(), "my_cc_library_bar/android_arm64_armv8-a_shared_apex10000/aconfig_merged.pb") android.EnsureListContainsSuffix(t, buildParams.Inputs.Strings(), "my_aconfig_declarations_baz/intermediate.pb") ensureContains(t, buildParams.Output.String(), "android_common_myapex/aconfig_flags.pb") } diff --git a/bp2build/Android.bp b/bp2build/Android.bp index 64ee01f88..c998074fe 100644 --- a/bp2build/Android.bp +++ b/bp2build/Android.bp @@ -6,7 +6,6 @@ bootstrap_go_package { name: "soong-bp2build", pkgPath: "android/soong/bp2build", srcs: [ - "aconfig_conversion_test.go", "androidbp_to_build_templates.go", "bp2build.go", "bp2build_product_config.go", @@ -43,6 +42,7 @@ bootstrap_go_package { testSrcs: [ "go_conversion_test.go", "aar_conversion_test.go", + "aconfig_conversion_test.go", "aidl_library_conversion_test.go", "android_app_certificate_conversion_test.go", "android_app_conversion_test.go", diff --git a/bp2build/aconfig_conversion_test.go b/bp2build/aconfig_conversion_test.go index d6e20df12..ac830c69f 100644 --- a/bp2build/aconfig_conversion_test.go +++ b/bp2build/aconfig_conversion_test.go @@ -15,6 +15,7 @@ package bp2build import ( + "android/soong/aconfig/codegen" "testing" "android/soong/aconfig" @@ -25,6 +26,7 @@ import ( func registerAconfigModuleTypes(ctx android.RegistrationContext) { aconfig.RegisterBuildComponents(ctx) + codegen.RegisterBuildComponents(ctx) ctx.RegisterModuleType("cc_library", cc.LibraryFactory) ctx.RegisterModuleType("java_library", java.LibraryFactory) } diff --git a/cc/androidmk.go b/cc/androidmk.go index 224ea9623..8cae634a8 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -134,7 +134,7 @@ func (c *Module) AndroidMkEntries() []android.AndroidMkEntries { "$(SOONG_SDK_VARIANT_MODULES) $(patsubst %.sdk,%,$(LOCAL_MODULE))") } // TODO(b/311155208): The container here should be system. - entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", c.getTransitiveAconfigFiles("")) + entries.SetPaths("LOCAL_ACONFIG_FILES", c.mergedAconfigFiles[""]) }, }, ExtraFooters: []android.AndroidMkExtraFootersFunc{ diff --git a/cc/cc.go b/cc/cc.go index 5d4ceb81d..f800dc0a0 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -928,7 +928,7 @@ type Module struct { hideApexVariantFromMake bool // Aconfig files for all transitive deps. Also exposed via TransitiveDeclarationsInfo - transitiveAconfigFiles map[string]*android.DepSet[android.Path] + mergedAconfigFiles map[string]android.Paths } func (c *Module) AddJSONData(d *map[string]interface{}) { @@ -2324,7 +2324,7 @@ func (c *Module) GenerateAndroidBuildActions(actx android.ModuleContext) { } ctx.SetProvider(blueprint.SrcsFileProviderKey, blueprint.SrcsFileProviderData{SrcPaths: deps.GeneratedSources.Strings()}) - aconfig.CollectTransitiveAconfigFiles(ctx, &c.transitiveAconfigFiles) + aconfig.CollectDependencyAconfigFiles(ctx, &c.mergedAconfigFiles) c.maybeInstall(ctx, apexInfo) } @@ -2345,10 +2345,6 @@ func (c *Module) maybeUnhideFromMake() { } } -func (c *Module) getTransitiveAconfigFiles(container string) []android.Path { - return c.transitiveAconfigFiles[container].ToList() -} - // maybeInstall is called at the end of both GenerateAndroidBuildActions and // ProcessBazelQueryResponse to run the install hooks for installable modules, // like binaries and tests. diff --git a/java/androidmk.go b/java/androidmk.go index 4da40d246..a3f94cd8c 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -129,7 +129,8 @@ func (library *Library) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_SOONG_DEXPREOPT_CONFIG", library.dexpreopter.configPath) } // TODO(b/311155208): The container here should be system. - entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", library.getTransitiveAconfigFiles("")) + + entries.SetPaths("LOCAL_ACONFIG_FILES", library.mergedAconfigFiles[""]) }, }, }) @@ -307,7 +308,7 @@ func (binary *Binary) AndroidMkEntries() []android.AndroidMkEntries { entries.SetString("LOCAL_SOONG_BUILT_INSTALLED", binary.dexpreopter.builtInstalled) } // TODO(b/311155208): The container here should be system. - entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", binary.getTransitiveAconfigFiles("")) + entries.SetPaths("LOCAL_ACONFIG_FILES", binary.mergedAconfigFiles[""]) }, }, ExtraFooters: []android.AndroidMkExtraFootersFunc{ @@ -461,7 +462,7 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries { if app.Name() != "framework-res" { // TODO(b/311155208): The container here should be system. - entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", app.getTransitiveAconfigFiles("")) + entries.SetPaths("LOCAL_ACONFIG_FILES", app.mergedAconfigFiles[""]) } }, }, @@ -540,7 +541,7 @@ func (a *AndroidLibrary) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_SOONG_EXPORT_PROGUARD_FLAGS", a.combinedExportedProguardFlagsFile) entries.SetBoolIfTrue("LOCAL_UNINSTALLABLE_MODULE", true) // TODO(b/311155208): The container here should be system. - entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", a.getTransitiveAconfigFiles("")) + entries.SetPaths("LOCAL_ACONFIG_FILES", a.mergedAconfigFiles[""]) }) return entriesList diff --git a/java/base.go b/java/base.go index 7e1381b34..c4b402620 100644 --- a/java/base.go +++ b/java/base.go @@ -22,9 +22,9 @@ import ( "android/soong/ui/metrics/bp2build_metrics_proto" + "github.com/google/blueprint" "github.com/google/blueprint/pathtools" "github.com/google/blueprint/proptools" - "github.com/google/blueprint" "android/soong/aconfig" "android/soong/android" @@ -514,8 +514,8 @@ type Module struct { // or the module should override Stem(). stem string - // Aconfig files for all transitive deps. Also exposed via TransitiveDeclarationsInfo - transitiveAconfigFiles map[string]*android.DepSet[android.Path] + // Single aconfig "cache file" merged from this module and all dependencies. + mergedAconfigFiles map[string]android.Paths } func (j *Module) CheckStableSdkVersion(ctx android.BaseModuleContext) error { @@ -1721,7 +1721,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath ctx.CheckbuildFile(outputFile) - aconfig.CollectTransitiveAconfigFiles(ctx, &j.transitiveAconfigFiles) + aconfig.CollectDependencyAconfigFiles(ctx, &j.mergedAconfigFiles) ctx.SetProvider(JavaInfoProvider, JavaInfo{ HeaderJars: android.PathsIfNonNil(j.headerJarFile), @@ -2078,10 +2078,6 @@ func (j *Module) IsInstallable() bool { return Bool(j.properties.Installable) } -func (j *Module) getTransitiveAconfigFiles(container string) []android.Path { - return j.transitiveAconfigFiles[container].ToList() -} - type sdkLinkType int const (