From 6fe59eb7f0985ee787e23e3edbca270a1a33e817 Mon Sep 17 00:00:00 2001 From: Joe Onorato Date: Sun, 16 Jul 2023 13:20:33 -0700 Subject: [PATCH] Track transitive usage of aconfig flags and add LOCAL_ACONFIG_FILES to Android-.mk Bug: 283910439 Test: m nothing (runs soong tests) Change-Id: I59f9bef7b7c502565d531a5685c002a177e0a77c --- aconfig/Android.bp | 1 + aconfig/java_aconfig_library.go | 7 +- aconfig/java_aconfig_library_test.go | 154 +++++++++++++++++++++++++++ java/aar.go | 1 + java/androidmk.go | 14 +++ java/base.go | 39 +++++++ java/device_host_converter.go | 2 + java/generated_java_library.go | 4 +- java/generated_java_library_test.go | 3 +- java/java.go | 15 ++- 10 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 aconfig/java_aconfig_library_test.go diff --git a/aconfig/Android.bp b/aconfig/Android.bp index ae8276f39..5e0620e85 100644 --- a/aconfig/Android.bp +++ b/aconfig/Android.bp @@ -28,6 +28,7 @@ bootstrap_go_package { "aconfig_declarations_test.go", "aconfig_values_test.go", "aconfig_value_set_test.go", + "java_aconfig_library_test.go", ], pluginFor: ["soong_build"], } diff --git a/aconfig/java_aconfig_library.go b/aconfig/java_aconfig_library.go index 0eeb14ffc..f98498ed4 100644 --- a/aconfig/java_aconfig_library.go +++ b/aconfig/java_aconfig_library.go @@ -51,7 +51,7 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) DepsMutator(module *ja } } -func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuildActions(ctx android.ModuleContext) android.Path { +func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuildActions(module *java.GeneratedJavaLibraryModule, ctx android.ModuleContext) android.Path { // Get the values that came from the global RELEASE_ACONFIG_VALUE_SETS flag declarationsModules := ctx.GetDirectDepsWithTag(declarationsTag) if len(declarationsModules) != 1 { @@ -59,6 +59,7 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuild } declarations := ctx.OtherModuleProvider(declarationsModules[0], declarationsProviderKey).(declarationsProviderData) + // Generate the action to build the srcjar srcJarPath := android.PathForModuleGen(ctx, ctx.ModuleName()+".srcjar") ctx.Build(pctx, android.BuildParams{ Rule: srcJarRule, @@ -67,5 +68,9 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuild Description: "aconfig.srcjar", }) + // Tell the java module about the .aconfig files, so they can be propagated up the dependency chain. + // TODO: It would be nice to have that propagation code here instead of on java.Module and java.JavaInfo. + module.AddAconfigIntermediate(declarations.IntermediatePath) + return srcJarPath } diff --git a/aconfig/java_aconfig_library_test.go b/aconfig/java_aconfig_library_test.go new file mode 100644 index 000000000..1808290ee --- /dev/null +++ b/aconfig/java_aconfig_library_test.go @@ -0,0 +1,154 @@ +// Copyright 2023 Google Inc. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package aconfig + +import ( + "strings" + "testing" + + "android/soong/android" + "android/soong/java" +) + +// Note: These tests cover the code in the java package. It'd be ideal of that code could +// be in the aconfig package. + +// With the bp parameter that defines a my_module, make sure it has the LOCAL_ACONFIG_FILES entries +func runJavaAndroidMkTest(t *testing.T, bp string) { + result := android.GroupFixturePreparers( + PrepareForTestWithAconfigBuildComponents, + java.PrepareForTestWithJavaDefaultModules). + ExtendWithErrorHandler(android.FixtureExpectsNoErrors). + RunTestWithBp(t, bp+` + aconfig_declarations { + name: "my_aconfig_declarations", + package: "com.example.package", + srcs: ["foo.aconfig"], + } + + java_aconfig_library { + name: "my_java_aconfig_library", + aconfig_declarations: "my_aconfig_declarations", + } + `) + + module := result.ModuleForTests("my_module", "android_common").Module() + + entry := android.AndroidMkEntriesForTest(t, result.TestContext, module)[0] + + makeVar := entry.EntryMap["LOCAL_ACONFIG_FILES"] + android.AssertIntEquals(t, "len(LOCAL_ACONFIG_FILES)", 1, len(makeVar)) + if !strings.HasSuffix(makeVar[0], "intermediate.pb") { + t.Errorf("LOCAL_ACONFIG_FILES should end with /intermediates.pb, instead it is: %s", makeVar[0]) + } +} + +func TestAndroidMkJavaLibrary(t *testing.T) { + bp := ` + java_library { + name: "my_module", + srcs: [ + "src/foo.java", + ], + static_libs: [ + "my_java_aconfig_library", + ], + platform_apis: true, + } + ` + + runJavaAndroidMkTest(t, bp) +} + +func TestAndroidMkAndroidApp(t *testing.T) { + bp := ` + android_app { + name: "my_module", + srcs: [ + "src/foo.java", + ], + static_libs: [ + "my_java_aconfig_library", + ], + platform_apis: true, + } + ` + + runJavaAndroidMkTest(t, bp) +} + +func TestAndroidMkBinary(t *testing.T) { + bp := ` + java_binary { + name: "my_module", + srcs: [ + "src/foo.java", + ], + static_libs: [ + "my_java_aconfig_library", + ], + platform_apis: true, + main_class: "foo", + } + ` + + runJavaAndroidMkTest(t, bp) +} + +func TestAndroidMkAndroidLibrary(t *testing.T) { + bp := ` + android_library { + name: "my_module", + srcs: [ + "src/foo.java", + ], + static_libs: [ + "my_java_aconfig_library", + ], + platform_apis: true, + } + ` + + runJavaAndroidMkTest(t, bp) +} + +func TestAndroidMkBinaryThatLinksAgainstAar(t *testing.T) { + // Tests AndroidLibrary's propagation of flags through JavaInfo + bp := ` + android_library { + name: "some_library", + srcs: [ + "src/foo.java", + ], + static_libs: [ + "my_java_aconfig_library", + ], + platform_apis: true, + } + java_binary { + name: "my_module", + srcs: [ + "src/bar.java", + ], + static_libs: [ + "some_library", + ], + platform_apis: true, + main_class: "foo", + } + ` + + runJavaAndroidMkTest(t, bp) +} diff --git a/java/aar.go b/java/aar.go index c9e08e2b5..a682e3ad7 100644 --- a/java/aar.go +++ b/java/aar.go @@ -988,6 +988,7 @@ func (a *AARImport) GenerateAndroidBuildActions(ctx android.ModuleContext) { TransitiveStaticLibsHeaderJars: a.transitiveStaticLibsHeaderJars, ImplementationAndResourcesJars: android.PathsIfNonNil(a.classpathFile), ImplementationJars: android.PathsIfNonNil(a.classpathFile), + // TransitiveAconfigFiles: // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts }) if proptools.Bool(a.properties.Extract_jni) { diff --git a/java/androidmk.go b/java/androidmk.go index 784fa29b5..4fca08d9f 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -123,6 +123,8 @@ func (library *Library) AndroidMkEntries() []android.AndroidMkEntries { if library.dexpreopter.configPath != nil { entries.SetPath("LOCAL_SOONG_DEXPREOPT_CONFIG", library.dexpreopter.configPath) } + + entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", library.getTransitiveAconfigFiles().ToList()) }, }, }) @@ -220,6 +222,7 @@ func (prebuilt *Import) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_SOONG_CLASSES_JAR", prebuilt.combinedClasspathFile) entries.SetString("LOCAL_SDK_VERSION", prebuilt.sdkVersion.String()) entries.SetString("LOCAL_MODULE_STEM", prebuilt.Stem()) + // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts }, }, }} @@ -244,6 +247,7 @@ func (prebuilt *DexImport) AndroidMkEntries() []android.AndroidMkEntries { entries.SetString("LOCAL_SOONG_BUILT_INSTALLED", prebuilt.dexpreopter.builtInstalled) } entries.SetString("LOCAL_MODULE_STEM", prebuilt.Stem()) + // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts }, }, }} @@ -269,6 +273,7 @@ func (prebuilt *AARImport) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_SOONG_STATIC_LIBRARY_EXTRA_PACKAGES", prebuilt.extraAaptPackagesFile) entries.SetPath("LOCAL_FULL_MANIFEST_FILE", prebuilt.manifest) entries.SetString("LOCAL_SDK_VERSION", prebuilt.sdkVersion.String()) + // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts }, }, }} @@ -295,6 +300,7 @@ func (binary *Binary) AndroidMkEntries() []android.AndroidMkEntries { if len(binary.dexpreopter.builtInstalled) > 0 { entries.SetString("LOCAL_SOONG_BUILT_INSTALLED", binary.dexpreopter.builtInstalled) } + entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", binary.getTransitiveAconfigFiles().ToList()) }, }, ExtraFooters: []android.AndroidMkExtraFootersFunc{ @@ -437,6 +443,10 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries { } entries.SetOptionalPaths("LOCAL_SOONG_LINT_REPORTS", app.linter.reports) + + if app.Name() != "framework-res" { + entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", app.getTransitiveAconfigFiles().ToList()) + } }, }, ExtraFooters: []android.AndroidMkExtraFootersFunc{ @@ -512,6 +522,7 @@ func (a *AndroidLibrary) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_FULL_MANIFEST_FILE", a.mergedManifestFile) entries.AddStrings("LOCAL_SOONG_EXPORT_PROGUARD_FLAGS", a.exportedProguardFlagFiles.Strings()...) entries.SetBoolIfTrue("LOCAL_UNINSTALLABLE_MODULE", true) + entries.SetOptionalPaths("LOCAL_ACONFIG_FILES", a.getTransitiveAconfigFiles().ToList()) }) return entriesList @@ -684,6 +695,7 @@ func (a *AndroidAppImport) AndroidMkEntries() []android.AndroidMkEntries { if Bool(a.properties.Export_package_resources) { entries.SetPath("LOCAL_SOONG_RESOURCE_EXPORT_PACKAGE", a.outputFile) } + // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts }, }, }} @@ -717,6 +729,7 @@ func (r *RuntimeResourceOverlay) AndroidMkEntries() []android.AndroidMkEntries { entries.SetString("LOCAL_CERTIFICATE", r.certificate.AndroidMkString()) entries.SetPath("LOCAL_MODULE_PATH", r.installDir) entries.AddStrings("LOCAL_OVERRIDES_PACKAGES", r.properties.Overrides...) + // TODO: LOCAL_ACONFIG_FILES -- Might eventually need aconfig flags? }, }, }} @@ -734,6 +747,7 @@ func (apkSet *AndroidAppSet) AndroidMkEntries() []android.AndroidMkEntries { entries.SetPath("LOCAL_APK_SET_INSTALL_FILE", apkSet.PackedAdditionalOutputs()) entries.SetPath("LOCAL_APKCERTS_FILE", apkSet.apkcertsFile) entries.AddStrings("LOCAL_OVERRIDES_PACKAGES", apkSet.properties.Overrides...) + // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts -- Both declarations and values }, }, }, diff --git a/java/base.go b/java/base.go index 8db716256..6a532da43 100644 --- a/java/base.go +++ b/java/base.go @@ -508,6 +508,14 @@ type Module struct { // This should be set in every ModuleWithStem's GenerateAndroidBuildActions // or the module should override Stem(). stem string + + // Aconfig "cache files" that went directly into this module. Transitive ones are + // tracked via JavaInfo.TransitiveAconfigFiles + // TODO: Extract to something standalone to propagate tags via GeneratedJavaLibraryModule + aconfigIntermediates android.Paths + + // Aconfig files for all transitive deps. Also exposed via JavaInfo + transitiveAconfigFiles *android.DepSet[android.Path] } func (j *Module) CheckStableSdkVersion(ctx android.BaseModuleContext) error { @@ -1623,6 +1631,8 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { ctx.CheckbuildFile(outputFile) + j.collectTransitiveAconfigFiles(ctx) + ctx.SetProvider(JavaInfoProvider, JavaInfo{ HeaderJars: android.PathsIfNonNil(j.headerJarFile), TransitiveLibsHeaderJars: j.transitiveLibsHeaderJars, @@ -1637,6 +1647,7 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) { ExportedPluginClasses: j.exportedPluginClasses, ExportedPluginDisableTurbine: j.exportedDisableTurbine, JacocoReportClassesFile: j.jacocoReportClassesFile, + TransitiveAconfigFiles: j.transitiveAconfigFiles, }) // Save the output file with no relative path so that it doesn't end up in a subdirectory when used as a resource @@ -1917,6 +1928,34 @@ func (j *Module) IsInstallable() bool { return Bool(j.properties.Installable) } +func (j *Module) collectTransitiveAconfigFiles(ctx android.ModuleContext) { + // Aconfig files from this module + mine := j.aconfigIntermediates + + // Aconfig files from transitive dependencies + fromDeps := []*android.DepSet[android.Path]{} + ctx.VisitDirectDeps(func(module android.Module) { + dep := ctx.OtherModuleProvider(module, JavaInfoProvider).(JavaInfo) + if dep.TransitiveAconfigFiles != nil { + fromDeps = append(fromDeps, dep.TransitiveAconfigFiles) + } + }) + + // DepSet containing aconfig files myself and from dependencies + j.transitiveAconfigFiles = android.NewDepSet(android.POSTORDER, mine, fromDeps) +} + +func (j *Module) AddAconfigIntermediate(path android.Path) { + j.aconfigIntermediates = append(j.aconfigIntermediates, path) +} + +func (j *Module) getTransitiveAconfigFiles() *android.DepSet[android.Path] { + if j.transitiveAconfigFiles == nil { + panic(fmt.Errorf("java.Moduile: getTransitiveAconfigFiles called before collectTransitiveAconfigFiles module=%s", j.Name())) + } + return j.transitiveAconfigFiles +} + type sdkLinkType int const ( diff --git a/java/device_host_converter.go b/java/device_host_converter.go index 3581040f8..5460dc993 100644 --- a/java/device_host_converter.go +++ b/java/device_host_converter.go @@ -143,6 +143,8 @@ func (d *DeviceHostConverter) GenerateAndroidBuildActions(ctx android.ModuleCont ResourceJars: d.resourceJars, SrcJarArgs: d.srcJarArgs, SrcJarDeps: d.srcJarDeps, + // TODO: Not sure if aconfig flags that have been moved between device and host variants + // make sense. }) } diff --git a/java/generated_java_library.go b/java/generated_java_library.go index 1b3de9fe0..f9baa85e4 100644 --- a/java/generated_java_library.go +++ b/java/generated_java_library.go @@ -30,7 +30,7 @@ type GeneratedJavaLibraryCallbacks interface { // Called from inside GenerateAndroidBuildActions. Add the build rules to // make the srcjar, and return the path to it. - GenerateSourceJarBuildActions(ctx android.ModuleContext) android.Path + GenerateSourceJarBuildActions(module *GeneratedJavaLibraryModule, ctx android.ModuleContext) android.Path } // GeneratedJavaLibraryModuleFactory provides a utility for modules that are generated @@ -88,7 +88,7 @@ func (module *GeneratedJavaLibraryModule) GenerateAndroidBuildActions(ctx androi checkPropertyEmpty(ctx, module, "plugins", module.Library.properties.Plugins) checkPropertyEmpty(ctx, module, "exported_plugins", module.Library.properties.Exported_plugins) - srcJarPath := module.callbacks.GenerateSourceJarBuildActions(ctx) + srcJarPath := module.callbacks.GenerateSourceJarBuildActions(module, ctx) module.Library.properties.Generated_srcjars = append(module.Library.properties.Generated_srcjars, srcJarPath) module.Library.GenerateAndroidBuildActions(ctx) } diff --git a/java/generated_java_library_test.go b/java/generated_java_library_test.go index 68f1f7edd..7f52fd108 100644 --- a/java/generated_java_library_test.go +++ b/java/generated_java_library_test.go @@ -36,7 +36,8 @@ type JavaGenLibTestCallbacks struct { func (callbacks *JavaGenLibTestCallbacks) DepsMutator(module *GeneratedJavaLibraryModule, ctx android.BottomUpMutatorContext) { } -func (callbacks *JavaGenLibTestCallbacks) GenerateSourceJarBuildActions(ctx android.ModuleContext) android.Path { +func (callbacks *JavaGenLibTestCallbacks) GenerateSourceJarBuildActions(module *GeneratedJavaLibraryModule, ctx android.ModuleContext) android.Path { + module.AddAconfigIntermediate(android.PathForOutput(ctx, "aconfig_cache_file")) return android.PathForOutput(ctx, "blah.srcjar") } diff --git a/java/java.go b/java/java.go index 6388d13e3..ee2069886 100644 --- a/java/java.go +++ b/java/java.go @@ -274,7 +274,14 @@ type JavaInfo struct { // instrumented by jacoco. JacocoReportClassesFile android.Path - // TODO: Add device config declarations here? + // set of aconfig flags for all transitive libs deps + // TODO(joeo): It would be nice if this were over in the aconfig package instead of here. + // In order to do that, generated_java_library would need a way doing + // collectTransitiveAconfigFiles with one of the callbacks, and having that automatically + // propagated. If we were to clean up more of the stuff on JavaInfo that's not part of + // core java rules (e.g. AidlIncludeDirs), then maybe adding more framework to do that would be + // worth it. + TransitiveAconfigFiles *android.DepSet[android.Path] } var JavaInfoProvider = blueprint.NewProvider(JavaInfo{}) @@ -730,6 +737,7 @@ func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) { } }) j.exportedProguardFlagFiles = android.FirstUniquePaths(j.exportedProguardFlagFiles) + } func (j *Library) DepsMutator(ctx android.BottomUpMutatorContext) { @@ -1916,6 +1924,7 @@ func (al *ApiLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) { ImplementationAndResourcesJars: android.PathsIfNonNil(al.stubsJar), ImplementationJars: android.PathsIfNonNil(al.stubsJar), AidlIncludeDirs: android.Paths{}, + // No aconfig libraries on api libraries }) } @@ -2237,6 +2246,7 @@ func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) { ImplementationAndResourcesJars: android.PathsIfNonNil(j.combinedClasspathFile), ImplementationJars: android.PathsIfNonNil(j.combinedClasspathFile), AidlIncludeDirs: j.exportAidlIncludeDirs, + // TODO(b/289117800): LOCAL_ACONFIG_FILES for prebuilts }) } @@ -3305,7 +3315,8 @@ func (i *Import) ProcessBazelQueryResponse(ctx android.ModuleContext) { HeaderJars: android.PathsIfNonNil(i.combinedClasspathFile), ImplementationAndResourcesJars: android.PathsIfNonNil(i.combinedClasspathFile), ImplementationJars: android.PathsIfNonNil(i.combinedClasspathFile), - //TODO(b/240308299) include AIDL information from Bazel + // TODO(b/240308299) include AIDL information from Bazel + // TODO: aconfig files? }) i.maybeInstall(ctx, jarName, outputFile)