From cca3e0c4b56b7620dee8f2dcf76b7b6682d8ebe8 Mon Sep 17 00:00:00 2001 From: Jihoon Kang Date: Wed, 29 Nov 2023 19:35:29 +0000 Subject: [PATCH 1/2] Add aconfig flag value text file in aconfig_declarations provider Alongside with the generated proto file, the aconfig_declaration now also outputs a text file that lists aconfig flags and values of its corresponding proto file, in the format as shown below: ``` my.flag1=true my.flag2=false ... ``` To prevent confusion between the preexisting proto file and the newly introduced text file, the change also renames the variables of the proto file from `intermediatePath` to `intermediateCacheOutputPath` and likewise. The utilization of the generated text file will be done in the child changes. Test: m out/soong/.intermediates/build/make/tools/aconfig/aconfig.test.flags/intermediate.txt && inspect output Bug: 306024510 Change-Id: Iee16ad57bb87e992a477fc96502f79e971d01233 --- aconfig/aconfig_declarations.go | 31 +++++++++++++++++-------- aconfig/aconfig_declarations_test.go | 7 ++++-- aconfig/all_aconfig_declarations.go | 2 +- aconfig/codegen/cc_aconfig_library.go | 2 +- aconfig/codegen/java_aconfig_library.go | 3 ++- aconfig/codegen/rust_aconfig_library.go | 2 +- aconfig/init.go | 13 +++++++++++ 7 files changed, 44 insertions(+), 16 deletions(-) diff --git a/aconfig/aconfig_declarations.go b/aconfig/aconfig_declarations.go index 05a637129..9bf75649a 100644 --- a/aconfig/aconfig_declarations.go +++ b/aconfig/aconfig_declarations.go @@ -20,6 +20,7 @@ import ( "android/soong/android" "android/soong/bazel" + "github.com/google/blueprint" ) @@ -116,9 +117,10 @@ func optionalVariable(prefix string, value string) string { // Provider published by aconfig_value_set type DeclarationsProviderData struct { - Package string - Container string - IntermediatePath android.WritablePath + Package string + Container string + IntermediateCacheOutputPath android.WritablePath + IntermediateDumpOutputPath android.WritablePath } var DeclarationsProviderKey = blueprint.NewProvider(DeclarationsProviderData{}) @@ -151,14 +153,14 @@ func (module *DeclarationsModule) GenerateAndroidBuildActions(ctx android.Module // Intermediate format declarationFiles := android.PathsForModuleSrc(ctx, module.properties.Srcs) - intermediatePath := android.PathForModuleOut(ctx, "intermediate.pb") + intermediateCacheFilePath := android.PathForModuleOut(ctx, "intermediate.pb") defaultPermission := ctx.Config().ReleaseAconfigFlagDefaultPermission() inputFiles := make([]android.Path, len(declarationFiles)) copy(inputFiles, declarationFiles) inputFiles = append(inputFiles, valuesFiles...) ctx.Build(pctx, android.BuildParams{ Rule: aconfigRule, - Output: intermediatePath, + Output: intermediateCacheFilePath, Inputs: inputFiles, Description: "aconfig_declarations", Args: map[string]string{ @@ -170,10 +172,19 @@ func (module *DeclarationsModule) GenerateAndroidBuildActions(ctx android.Module }, }) + intermediateDumpFilePath := android.PathForModuleOut(ctx, "intermediate.txt") + ctx.Build(pctx, android.BuildParams{ + Rule: aconfigTextRule, + Output: intermediateDumpFilePath, + Inputs: android.Paths{intermediateCacheFilePath}, + Description: "aconfig_text", + }) + ctx.SetProvider(DeclarationsProviderKey, DeclarationsProviderData{ - Package: module.properties.Package, - Container: module.properties.Container, - IntermediatePath: intermediatePath, + Package: module.properties.Package, + Container: module.properties.Container, + IntermediateCacheOutputPath: intermediateCacheFilePath, + IntermediateDumpOutputPath: intermediateDumpFilePath, }) } @@ -182,8 +193,8 @@ func CollectDependencyAconfigFiles(ctx android.ModuleContext, mergedAconfigFiles *mergedAconfigFiles = make(map[string]android.Paths) } ctx.VisitDirectDeps(func(module android.Module) { - if dep := ctx.OtherModuleProvider(module, DeclarationsProviderKey).(DeclarationsProviderData); dep.IntermediatePath != nil { - (*mergedAconfigFiles)[dep.Container] = append((*mergedAconfigFiles)[dep.Container], dep.IntermediatePath) + if dep := ctx.OtherModuleProvider(module, DeclarationsProviderKey).(DeclarationsProviderData); dep.IntermediateCacheOutputPath != nil { + (*mergedAconfigFiles)[dep.Container] = append((*mergedAconfigFiles)[dep.Container], dep.IntermediateCacheOutputPath) return } if dep := ctx.OtherModuleProvider(module, TransitiveDeclarationsInfoProvider).(TransitiveDeclarationsInfo); len(dep.AconfigFiles) > 0 { diff --git a/aconfig/aconfig_declarations_test.go b/aconfig/aconfig_declarations_test.go index e6bd87cd5..9035c710d 100644 --- a/aconfig/aconfig_declarations_test.go +++ b/aconfig/aconfig_declarations_test.go @@ -41,7 +41,10 @@ func TestAconfigDeclarations(t *testing.T) { depData := result.ModuleProvider(module, DeclarationsProviderKey).(DeclarationsProviderData) android.AssertStringEquals(t, "package", depData.Package, "com.example.package") android.AssertStringEquals(t, "container", depData.Container, "com.android.foo") - if !strings.HasSuffix(depData.IntermediatePath.String(), "/intermediate.pb") { - t.Errorf("Missing intermediates path in provider: %s", depData.IntermediatePath.String()) + if !strings.HasSuffix(depData.IntermediateCacheOutputPath.String(), "/intermediate.pb") { + t.Errorf("Missing intermediates proto path in provider: %s", depData.IntermediateCacheOutputPath.String()) + } + if !strings.HasSuffix(depData.IntermediateDumpOutputPath.String(), "/intermediate.txt") { + t.Errorf("Missing intermediates text path in provider: %s", depData.IntermediateDumpOutputPath.String()) } } diff --git a/aconfig/all_aconfig_declarations.go b/aconfig/all_aconfig_declarations.go index 7ccb81a37..2686e760e 100644 --- a/aconfig/all_aconfig_declarations.go +++ b/aconfig/all_aconfig_declarations.go @@ -41,7 +41,7 @@ func (this *allAconfigDeclarationsSingleton) GenerateBuildActions(ctx android.Si return } decl := ctx.ModuleProvider(module, DeclarationsProviderKey).(DeclarationsProviderData) - cacheFiles = append(cacheFiles, decl.IntermediatePath) + cacheFiles = append(cacheFiles, decl.IntermediateCacheOutputPath) }) // Generate build action for aconfig diff --git a/aconfig/codegen/cc_aconfig_library.go b/aconfig/codegen/cc_aconfig_library.go index 7b68844e9..68551187b 100644 --- a/aconfig/codegen/cc_aconfig_library.go +++ b/aconfig/codegen/cc_aconfig_library.go @@ -132,7 +132,7 @@ func (this *CcAconfigLibraryCallbacks) GeneratorBuildActions(ctx cc.ModuleContex ctx.Build(pctx, android.BuildParams{ Rule: cppRule, - Input: declarations.IntermediatePath, + Input: declarations.IntermediateCacheOutputPath, Outputs: []android.WritablePath{ this.generatedCpp, this.generatedH, diff --git a/aconfig/codegen/java_aconfig_library.go b/aconfig/codegen/java_aconfig_library.go index e2fb15bc7..211bdce90 100644 --- a/aconfig/codegen/java_aconfig_library.go +++ b/aconfig/codegen/java_aconfig_library.go @@ -21,6 +21,7 @@ import ( "android/soong/android" "android/soong/bazel" "android/soong/java" + "github.com/google/blueprint" "github.com/google/blueprint/proptools" ) @@ -86,7 +87,7 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuild ctx.Build(pctx, android.BuildParams{ Rule: javaRule, - Input: declarations.IntermediatePath, + Input: declarations.IntermediateCacheOutputPath, Output: srcJarPath, Description: "aconfig.srcjar", Args: map[string]string{ diff --git a/aconfig/codegen/rust_aconfig_library.go b/aconfig/codegen/rust_aconfig_library.go index 3525de19a..e5870562c 100644 --- a/aconfig/codegen/rust_aconfig_library.go +++ b/aconfig/codegen/rust_aconfig_library.go @@ -74,7 +74,7 @@ func (a *aconfigDecorator) GenerateSource(ctx rust.ModuleContext, deps rust.Path ctx.Build(pctx, android.BuildParams{ Rule: rustRule, - Input: declarations.IntermediatePath, + Input: declarations.IntermediateCacheOutputPath, Outputs: []android.WritablePath{ generatedSource, }, diff --git a/aconfig/init.go b/aconfig/init.go index 2e24db94d..72f6bb620 100644 --- a/aconfig/init.go +++ b/aconfig/init.go @@ -40,6 +40,19 @@ var ( Restat: true, }, "release_version", "package", "declarations", "values", "default-permission") + // For create-device-config-sysprops: Generate aconfig flag value map text file + aconfigTextRule = pctx.AndroidStaticRule("aconfig_text", + blueprint.RuleParams{ + Command: `${aconfig} dump --format bool` + + ` --cache ${in}` + + ` --out ${out}.tmp` + + ` && ( if cmp -s ${out}.tmp ${out} ; then rm ${out}.tmp ; else mv ${out}.tmp ${out} ; fi )`, + CommandDeps: []string{ + "${aconfig}", + }, + Restat: true, + }) + // For all_aconfig_declarations: Combine all parsed_flags proto files AllDeclarationsRule = pctx.AndroidStaticRule("All_aconfig_declarations_dump", blueprint.RuleParams{ From 84b2589e6dc85d15af273b8a293ed7f064142af7 Mon Sep 17 00:00:00 2001 From: Jihoon Kang Date: Fri, 1 Dec 2023 22:01:06 +0000 Subject: [PATCH 2/2] Add aconfig flag support for android_app This change adds an overrideable property flags_packages to android_app, which is used to list the aconfig_declarations module names that the app depends on. The build action of android_app is modified to pass all flags text file provided by the aconfig_declarations to aapt2 link as --feature-flags arguments. Test: m nothing --no-skip-soong-tests Bug: 306024510 Change-Id: I4924f88b9954950cc1936a472cd7ac70f41add5d --- java/aapt2.go | 8 +++++++- java/aar.go | 6 ++++-- java/app.go | 33 ++++++++++++++++++++++++++++----- java/app_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++++ java/java.go | 1 + java/java_test.go | 3 +++ 6 files changed, 88 insertions(+), 8 deletions(-) diff --git a/java/aapt2.go b/java/aapt2.go index 17ee6ee42..445e91298 100644 --- a/java/aapt2.go +++ b/java/aapt2.go @@ -202,7 +202,8 @@ var mergeAssetsRule = pctx.AndroidStaticRule("mergeAssets", func aapt2Link(ctx android.ModuleContext, packageRes, genJar, proguardOptions, rTxt android.WritablePath, flags []string, deps android.Paths, - compiledRes, compiledOverlay, assetPackages android.Paths, splitPackages android.WritablePaths) { + compiledRes, compiledOverlay, assetPackages android.Paths, splitPackages android.WritablePaths, + featureFlagsPaths android.Paths) { var inFlags []string @@ -255,6 +256,11 @@ func aapt2Link(ctx android.ModuleContext, }) } + for _, featureFlagsPath := range featureFlagsPaths { + deps = append(deps, featureFlagsPath) + inFlags = append(inFlags, "--feature-flags", "@"+featureFlagsPath.String()) + } + // Note the absence of splitPackages. The caller is supposed to compose and provide --split flag // values via the flags parameter when it wants to split outputs. // TODO(b/174509108): Perhaps we can process it in this func while keeping the code reasonably diff --git a/java/aar.go b/java/aar.go index 57a05d47b..1ab452965 100644 --- a/java/aar.go +++ b/java/aar.go @@ -349,6 +349,7 @@ type aaptBuildActionOptions struct { excludedLibs []string enforceDefaultTargetSdkVersion bool extraLinkFlags []string + aconfigTextFiles android.Paths } func (a *aapt) buildActions(ctx android.ModuleContext, opts aaptBuildActionOptions) { @@ -529,7 +530,8 @@ func (a *aapt) buildActions(ctx android.ModuleContext, opts aaptBuildActionOptio transitiveAssets = android.ReverseSliceInPlace(staticDeps.assets()) } aapt2Link(ctx, packageRes, srcJar, proguardOptionsFile, rTxt, - linkFlags, linkDeps, compiledRes, compiledOverlay, transitiveAssets, splitPackages) + linkFlags, linkDeps, compiledRes, compiledOverlay, transitiveAssets, splitPackages, + opts.aconfigTextFiles) // Extract assets from the resource package output so that they can be used later in aapt2link // for modules that depend on this one. if android.PrefixInList(linkFlags, "-A ") { @@ -1193,7 +1195,7 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { transitiveAssets := android.ReverseSliceInPlace(staticDeps.assets()) aapt2Link(ctx, a.exportPackage, nil, proguardOptionsFile, a.rTxt, - linkFlags, linkDeps, nil, overlayRes, transitiveAssets, nil) + linkFlags, linkDeps, nil, overlayRes, transitiveAssets, nil, nil) a.rJar = android.PathForModuleOut(ctx, "busybox/R.jar") resourceProcessorBusyBoxGenerateBinaryR(ctx, a.rTxt, a.manifest, a.rJar, nil, true) diff --git a/java/app.go b/java/app.go index 0f46b4ec8..d8822aff3 100755 --- a/java/app.go +++ b/java/app.go @@ -22,7 +22,9 @@ import ( "path/filepath" "strings" + "android/soong/aconfig" "android/soong/testing" + "github.com/google/blueprint" "github.com/google/blueprint/proptools" @@ -170,6 +172,9 @@ type overridableAppProperties struct { // binaries would be installed by default (in PRODUCT_PACKAGES) the other binary will be removed // from PRODUCT_PACKAGES. Overrides []string + + // Names of aconfig_declarations modules that specify aconfig flags that the app depends on. + Flags_packages []string } type AndroidApp struct { @@ -316,6 +321,10 @@ func (a *AndroidApp) OverridablePropertiesDepsMutator(ctx android.BottomUpMutato `must be names of android_app_certificate modules in the form ":module"`) } } + + for _, aconfig_declaration := range a.overridableAppProperties.Flags_packages { + ctx.AddDependency(ctx.Module(), aconfigDeclarationTag, aconfig_declaration) + } } func (a *AndroidTestHelperApp) GenerateAndroidBuildActions(ctx android.ModuleContext) { @@ -500,13 +509,27 @@ func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) { if a.Updatable() { a.aapt.defaultManifestVersion = android.DefaultUpdatableModuleVersion } + + var aconfigTextFilePaths android.Paths + ctx.VisitDirectDepsWithTag(aconfigDeclarationTag, func(dep android.Module) { + if provider, ok := ctx.OtherModuleProvider(dep, aconfig.DeclarationsProviderKey).(aconfig.DeclarationsProviderData); ok { + aconfigTextFilePaths = append(aconfigTextFilePaths, provider.IntermediateDumpOutputPath) + } else { + ctx.ModuleErrorf("Only aconfig_declarations module type is allowed for "+ + "flags_packages property, but %s is not aconfig_declarations module type", + dep.Name(), + ) + } + }) + a.aapt.buildActions(ctx, aaptBuildActionOptions{ - android.SdkContext(a), - a.classLoaderContexts, - a.usesLibraryProperties.Exclude_uses_libs, - a.enforceDefaultTargetSdkVersion(), - aaptLinkFlags, + sdkContext: android.SdkContext(a), + classLoaderContexts: a.classLoaderContexts, + excludedLibs: a.usesLibraryProperties.Exclude_uses_libs, + enforceDefaultTargetSdkVersion: a.enforceDefaultTargetSdkVersion(), + extraLinkFlags: aaptLinkFlags, + aconfigTextFiles: aconfigTextFilePaths, }, ) diff --git a/java/app_test.go b/java/app_test.go index 5cb4a2301..0936b281a 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -4333,3 +4333,48 @@ func TestApexGlobalMinSdkVersionOverride(t *testing.T) { ) } + +func TestAppFlagsPackages(t *testing.T) { + ctx := testApp(t, ` + android_app { + name: "foo", + srcs: ["a.java"], + sdk_version: "current", + flags_packages: [ + "bar", + "baz", + ], + } + aconfig_declarations { + name: "bar", + package: "com.example.package", + srcs: [ + "bar.aconfig", + ], + } + aconfig_declarations { + name: "baz", + package: "com.example.package", + srcs: [ + "baz.aconfig", + ], + } + `) + + foo := ctx.ModuleForTests("foo", "android_common") + + // android_app module depends on aconfig_declarations listed in flags_packages + android.AssertBoolEquals(t, "foo expected to depend on bar", true, + CheckModuleHasDependency(t, ctx, "foo", "android_common", "bar")) + + android.AssertBoolEquals(t, "foo expected to depend on baz", true, + CheckModuleHasDependency(t, ctx, "foo", "android_common", "baz")) + + aapt2LinkRule := foo.Rule("android/soong/java.aapt2Link") + linkInFlags := aapt2LinkRule.Args["inFlags"] + android.AssertStringDoesContain(t, + "aapt2 link command expected to pass feature flags arguments", + linkInFlags, + "--feature-flags @out/soong/.intermediates/bar/intermediate.txt --feature-flags @out/soong/.intermediates/baz/intermediate.txt", + ) +} diff --git a/java/java.go b/java/java.go index 2236d05b2..d5d22b2aa 100644 --- a/java/java.go +++ b/java/java.go @@ -409,6 +409,7 @@ var ( syspropPublicStubDepTag = dependencyTag{name: "sysprop public stub"} javaApiContributionTag = dependencyTag{name: "java-api-contribution"} depApiSrcsTag = dependencyTag{name: "dep-api-srcs"} + aconfigDeclarationTag = dependencyTag{name: "aconfig-declaration"} jniInstallTag = installDependencyTag{name: "jni install"} binaryInstallTag = installDependencyTag{name: "binary install"} usesLibReqTag = makeUsesLibraryDependencyTag(dexpreopt.AnySdkVersion, false) diff --git a/java/java_test.go b/java/java_test.go index 0c750ebe1..d4b056bc9 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -26,6 +26,7 @@ import ( "github.com/google/blueprint/proptools" + "android/soong/aconfig" "android/soong/android" "android/soong/cc" "android/soong/dexpreopt" @@ -47,6 +48,8 @@ var prepareForJavaTest = android.GroupFixturePreparers( cc.PrepareForTestWithCcBuildComponents, // Include all the default java modules. PrepareForTestWithDexpreopt, + // Include aconfig modules. + aconfig.PrepareForTestWithAconfigBuildComponents, ) func TestMain(m *testing.M) {