From 9aef7778263e617eb57b70efc417f228245db622 Mon Sep 17 00:00:00 2001 From: Jihoon Kang Date: Fri, 14 Jun 2024 23:45:06 +0000 Subject: [PATCH] Propagate flags_packages to static reverse dependencies flags_packages property was added to supports flagging the manifest files. The listed names of `aconfig_declarations` modules are passed to aapt2. However, this is currently scoped to the module level, and is not propagated to the reverse dependencies. In other words, if the manifest is flagged with `featureFlag` property, all of the reverse dependency of the android_app/android_library should specify `flags_packages` property in the bp module definition, leading to huge toil for the users. In order to resolve such inconvenience, this change modifies the build rules of android_app,android_library and runtime_resource_overlay such that flags_packages of the static dependencies are collected. Test: Patch ag/27816261 && m Settings --no-skip-soong-tests Bug: 347289274 Change-Id: I4c3855541dd09cb72293515eb2626eaf4ae8c0df --- java/aar.go | 7 ++++- java/app.go | 43 ++++++++++++++++++++------- java/app_test.go | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ java/rro.go | 7 ++++- 4 files changed, 121 insertions(+), 12 deletions(-) diff --git a/java/aar.go b/java/aar.go index 07392f6e5..3168d9bf4 100644 --- a/java/aar.go +++ b/java/aar.go @@ -831,12 +831,13 @@ func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) if a.usesLibrary.shouldDisableDexpreopt { a.dexpreopter.disableDexpreopt() } + aconfigTextFilePaths := getAconfigFilePaths(ctx) a.aapt.buildActions(ctx, aaptBuildActionOptions{ sdkContext: android.SdkContext(a), classLoaderContexts: a.classLoaderContexts, enforceDefaultTargetSdkVersion: false, - aconfigTextFiles: getAconfigFilePaths(ctx), + aconfigTextFiles: aconfigTextFilePaths, usesLibrary: &a.usesLibrary, }, ) @@ -906,6 +907,10 @@ func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) JniPackages: prebuiltJniPackages, }) } + + android.SetProvider(ctx, FlagsPackagesProvider, FlagsPackages{ + AconfigTextFiles: aconfigTextFilePaths, + }) } func (a *AndroidLibrary) IDEInfo(dpInfo *android.IdeInfo) { diff --git a/java/app.go b/java/app.go index a24099c28..739ef1a50 100644 --- a/java/app.go +++ b/java/app.go @@ -47,6 +47,13 @@ var ( }, "packageName") ) +type FlagsPackages struct { + // Paths to the aconfig dump output text files that are consumed by aapt2 + AconfigTextFiles android.Paths +} + +var FlagsPackagesProvider = blueprint.NewProvider[FlagsPackages]() + func RegisterAppBuildComponents(ctx android.RegistrationContext) { ctx.RegisterModuleType("android_app", AndroidAppFactory) ctx.RegisterModuleType("android_test", AndroidTestFactory) @@ -478,18 +485,27 @@ func (a *AndroidApp) renameResourcesPackage() bool { } func getAconfigFilePaths(ctx android.ModuleContext) (aconfigTextFilePaths android.Paths) { - ctx.VisitDirectDepsWithTag(aconfigDeclarationTag, func(dep android.Module) { - if provider, ok := android.OtherModuleProvider(ctx, dep, android.AconfigDeclarationsProviderKey); 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(), - ) + ctx.VisitDirectDeps(func(dep android.Module) { + tag := ctx.OtherModuleDependencyTag(dep) + switch tag { + case staticLibTag: + if flagPackages, ok := android.OtherModuleProvider(ctx, dep, FlagsPackagesProvider); ok { + aconfigTextFilePaths = append(aconfigTextFilePaths, flagPackages.AconfigTextFiles...) + } + + case aconfigDeclarationTag: + if provider, ok := android.OtherModuleProvider(ctx, dep, android.AconfigDeclarationsProviderKey); 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(), + ) + } } }) - return aconfigTextFilePaths + return android.FirstUniquePaths(aconfigTextFilePaths) } func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) { @@ -544,6 +560,9 @@ func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) { // Use non final ids if we are doing optimized shrinking and are using R8. nonFinalIds := a.dexProperties.optimizedResourceShrinkingEnabled(ctx) && a.dexer.effectiveOptimizeEnabled() + + aconfigTextFilePaths := getAconfigFilePaths(ctx) + a.aapt.buildActions(ctx, aaptBuildActionOptions{ sdkContext: android.SdkContext(a), @@ -552,13 +571,17 @@ func (a *AndroidApp) aaptBuildActions(ctx android.ModuleContext) { enforceDefaultTargetSdkVersion: a.enforceDefaultTargetSdkVersion(), forceNonFinalResourceIDs: nonFinalIds, extraLinkFlags: aaptLinkFlags, - aconfigTextFiles: getAconfigFilePaths(ctx), + aconfigTextFiles: aconfigTextFilePaths, usesLibrary: &a.usesLibrary, }, ) // apps manifests are handled by aapt, don't let Module see them a.properties.Manifest = nil + + android.SetProvider(ctx, FlagsPackagesProvider, FlagsPackages{ + AconfigTextFiles: aconfigTextFilePaths, + }) } func (a *AndroidApp) proguardBuildActions(ctx android.ModuleContext) { diff --git a/java/app_test.go b/java/app_test.go index 804949435..1a862fa54 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -4369,6 +4369,82 @@ func TestAppFlagsPackages(t *testing.T) { ) } +func TestAppFlagsPackagesPropagation(t *testing.T) { + ctx := testApp(t, ` + aconfig_declarations { + name: "foo", + package: "com.example.package.foo", + container: "com.android.foo", + srcs: [ + "foo.aconfig", + ], + } + aconfig_declarations { + name: "bar", + package: "com.example.package.bar", + container: "com.android.bar", + srcs: [ + "bar.aconfig", + ], + } + aconfig_declarations { + name: "baz", + package: "com.example.package.baz", + container: "com.android.baz", + srcs: [ + "baz.aconfig", + ], + } + android_library { + name: "foo_lib", + srcs: ["a.java"], + sdk_version: "current", + flags_packages: [ + "foo", + ], + } + android_library { + name: "bar_lib", + srcs: ["a.java"], + sdk_version: "current", + flags_packages: [ + "bar", + ], + } + android_app { + name: "baz_app", + srcs: ["a.java"], + sdk_version: "current", + flags_packages: [ + "baz", + ], + static_libs: [ + "bar_lib", + ], + libs: [ + "foo_lib", + ], + } + `) + + bazApp := ctx.ModuleForTests("baz_app", "android_common") + + // android_app module depends on aconfig_declarations listed in flags_packages + // and that of static libs, but not libs + aapt2LinkRule := bazApp.Rule("android/soong/java.aapt2Link") + linkInFlags := aapt2LinkRule.Args["inFlags"] + android.AssertStringDoesContain(t, + "aapt2 link command expected to pass feature flags arguments of flags_packages and that of its static libs", + linkInFlags, + "--feature-flags @out/soong/.intermediates/bar/intermediate.txt --feature-flags @out/soong/.intermediates/baz/intermediate.txt", + ) + android.AssertStringDoesNotContain(t, + "aapt2 link command expected to not pass feature flags arguments of flags_packages of its libs", + linkInFlags, + "--feature-flags @out/soong/.intermediates/foo/intermediate.txt", + ) +} + // Test that dexpreopt is disabled if an optional_uses_libs exists, but does not provide an implementation. func TestNoDexpreoptOptionalUsesLibDoesNotHaveImpl(t *testing.T) { bp := ` diff --git a/java/rro.go b/java/rro.go index 72170fc6d..0fc6e1c6f 100644 --- a/java/rro.go +++ b/java/rro.go @@ -150,12 +150,13 @@ func (r *RuntimeResourceOverlay) GenerateAndroidBuildActions(ctx android.ModuleC aaptLinkFlags = append(aaptLinkFlags, "--rename-overlay-category "+*r.overridableProperties.Category) } + aconfigTextFilePaths := getAconfigFilePaths(ctx) r.aapt.buildActions(ctx, aaptBuildActionOptions{ sdkContext: r, enforceDefaultTargetSdkVersion: false, extraLinkFlags: aaptLinkFlags, - aconfigTextFiles: getAconfigFilePaths(ctx), + aconfigTextFiles: aconfigTextFilePaths, }, ) @@ -176,6 +177,10 @@ func (r *RuntimeResourceOverlay) GenerateAndroidBuildActions(ctx android.ModuleC partition := rroPartition(ctx) r.installDir = android.PathForModuleInPartitionInstall(ctx, partition, "overlay", String(r.properties.Theme)) ctx.InstallFile(r.installDir, r.outputFile.Base(), r.outputFile) + + android.SetProvider(ctx, FlagsPackagesProvider, FlagsPackages{ + AconfigTextFiles: aconfigTextFilePaths, + }) } func (r *RuntimeResourceOverlay) SdkVersion(ctx android.EarlyModuleContext) android.SdkSpec {