From 3921f0b3560a0dbbac3f5590dc639caf9492a2e8 Mon Sep 17 00:00:00 2001 From: Jihoon Kang Date: Tue, 12 Mar 2024 23:51:37 +0000 Subject: [PATCH] Collect aconfig_declarations of the dependent java_aconfig_library modules droidstubs module require aconfig_declarations modules to be specified when the module depends on the java_aconfig_library module in order to generate the "exportable" stubs. This adds burden to the droidstubs or java_sdk_library module owners, as module should specify both the java_aconfig_library and aconfig_declarations modules to genreate the "exportable" stubs, although the necessary information from the aconfig_declarations module can be provided from the java_aconfig_library modules. In order to resolve such burden, this change enables the intermediate cache files from the associated aconfig_declarations module of a java_aconfig_library module to be propagated to its reverse dependencies without having the specify the aconfig_declarations modules in the droidstubs or java_sdk_library modules definitions. This does not mean that the intermediate cache files of every transitive dependencies of the java_sdk_library or the droidstubs will be passed to aconfig to retrieve the state of the aconfig flags. Specifically, only the java_aconfig_library modules or the java_library modules that have static dependency on java_aconfig_library modules that are passed to droidstubs via `libs` or `srcs` (using ":module_name{.tag}" syntax) will provide the intermediate cache files to generate the "exportable" stubs. For java_sdk_library, all modules listed as `libs`, `static_libs`, `stub_only_libs`, and `apiScope.libs` are passed as `libs` and all modules listed as `api_srcs` and `srcs` are passed as `srcs` to the droidstubs modules dynamically generated in java_sdk_library module per api scope, thus these properties will be affected. Note that the test is being added in the apex package. This is because trying to register the codegen package build components in the java package leads to circular dependency between the codegen and the java package, as codegen package imports the java package. Test: m nothing --no-skip-soong-tests Bug: 329284345 Change-Id: I7953ab64776f6947808321ce8a3598154501bcfe --- aconfig/codegen/java_aconfig_library.go | 8 +- apex/apex_test.go | 118 ++++++++++++++++++++++++ java/base.go | 17 +++- java/droiddoc.go | 15 +++ java/generated_java_library.go | 6 +- java/generated_java_library_test.go | 4 +- 6 files changed, 160 insertions(+), 8 deletions(-) diff --git a/aconfig/codegen/java_aconfig_library.go b/aconfig/codegen/java_aconfig_library.go index 1378dfece..a46ce5245 100644 --- a/aconfig/codegen/java_aconfig_library.go +++ b/aconfig/codegen/java_aconfig_library.go @@ -76,7 +76,7 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) DepsMutator(module *ja } } -func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuildActions(module *java.GeneratedJavaLibraryModule, ctx android.ModuleContext) android.Path { +func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuildActions(module *java.GeneratedJavaLibraryModule, ctx android.ModuleContext) (android.Path, android.Path) { // Get the values that came from the global RELEASE_ACONFIG_VALUE_SETS flag declarationsModules := ctx.GetDirectDepsWithTag(declarationsTag) if len(declarationsModules) != 1 { @@ -129,7 +129,11 @@ func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) GenerateSourceJarBuild }}, }) - return srcJarPath + return srcJarPath, declarations.IntermediateCacheOutputPath +} + +func (callbacks *JavaAconfigDeclarationsLibraryCallbacks) AconfigDeclarations() *string { + return proptools.StringPtr(callbacks.properties.Aconfig_declarations) } func isModeSupported(mode string) bool { diff --git a/apex/apex_test.go b/apex/apex_test.go index 535805b4e..221bd4c86 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -11930,3 +11930,121 @@ func TestInstallationRulesForMultipleApexPrebuilts(t *testing.T) { checkHideFromMake(t, ctx, tc.expectedVisibleModuleName, tc.expectedHiddenModuleNames) } } + +func TestAconfifDeclarationsValidation(t *testing.T) { + aconfigDeclarationLibraryString := func(moduleNames []string) (ret string) { + for _, moduleName := range moduleNames { + ret += fmt.Sprintf(` + aconfig_declarations { + name: "%[1]s", + package: "com.example.package", + srcs: [ + "%[1]s.aconfig", + ], + } + java_aconfig_library { + name: "%[1]s-lib", + aconfig_declarations: "%[1]s", + } + `, moduleName) + } + return ret + } + + result := android.GroupFixturePreparers( + prepareForApexTest, + java.PrepareForTestWithJavaSdkLibraryFiles, + java.FixtureWithLastReleaseApis("foo"), + android.FixtureModifyConfig(func(config android.Config) { + config.SetApiLibraries([]string{"foo"}) + }), + ).RunTestWithBp(t, ` + java_library { + name: "baz-java-lib", + static_libs: [ + "baz-lib", + ], + } + filegroup { + name: "qux-filegroup", + srcs: [ + ":qux-lib{.generated_srcjars}", + ], + } + filegroup { + name: "qux-another-filegroup", + srcs: [ + ":qux-filegroup", + ], + } + java_library { + name: "quux-java-lib", + srcs: [ + "a.java", + ], + libs: [ + "quux-lib", + ], + } + java_sdk_library { + name: "foo", + srcs: [ + ":qux-another-filegroup", + ], + api_packages: ["foo"], + system: { + enabled: true, + }, + module_lib: { + enabled: true, + }, + test: { + enabled: true, + }, + static_libs: [ + "bar-lib", + ], + libs: [ + "baz-java-lib", + "quux-java-lib", + ], + aconfig_declarations: [ + "bar", + ], + } + `+aconfigDeclarationLibraryString([]string{"bar", "baz", "qux", "quux"})) + + m := result.ModuleForTests("foo.stubs.source", "android_common") + outDir := "out/soong/.intermediates" + + // Arguments passed to aconfig to retrieve the state of the flags defined in the + // textproto files + aconfigFlagArgs := m.Output("released-flagged-apis-exportable.txt").Args["flags_path"] + + // "bar-lib" is a static_lib of "foo" and is passed to metalava as classpath. Thus the + // cache file provided by the associated aconfig_declarations module "bar" should be passed + // to aconfig. + android.AssertStringDoesContain(t, "cache file of a java_aconfig_library static_lib "+ + "passed as an input", + aconfigFlagArgs, fmt.Sprintf("%s/%s/intermediate.pb", outDir, "bar")) + + // "baz-java-lib", which statically depends on "baz-lib", is a lib of "foo" and is passed + // to metalava as classpath. Thus the cache file provided by the associated + // aconfig_declarations module "baz" should be passed to aconfig. + android.AssertStringDoesContain(t, "cache file of a lib that statically depends on "+ + "java_aconfig_library passed as an input", + aconfigFlagArgs, fmt.Sprintf("%s/%s/intermediate.pb", outDir, "baz")) + + // "qux-lib" is passed to metalava as src via the filegroup, thus the cache file provided by + // the associated aconfig_declarations module "qux" should be passed to aconfig. + android.AssertStringDoesContain(t, "cache file of srcs java_aconfig_library passed as an "+ + "input", + aconfigFlagArgs, fmt.Sprintf("%s/%s/intermediate.pb", outDir, "qux")) + + // "quux-java-lib" is a lib of "foo" and is passed to metalava as classpath, but does not + // statically depend on "quux-lib". Therefore, the cache file provided by the associated + // aconfig_declarations module "quux" should not be passed to aconfig. + android.AssertStringDoesNotContain(t, "cache file of a lib that does not statically "+ + "depend on java_aconfig_library not passed as an input", + aconfigFlagArgs, fmt.Sprintf("%s/%s/intermediate.pb", outDir, "quux")) +} diff --git a/java/base.go b/java/base.go index 9bb133dc3..dcc510484 100644 --- a/java/base.go +++ b/java/base.go @@ -200,6 +200,9 @@ type CommonProperties struct { // Additional srcJars tacked in by GeneratedJavaLibraryModule Generated_srcjars []android.Path `android:"mutated"` + // intermediate aconfig cache file tacked in by GeneratedJavaLibraryModule + Aconfig_Cache_files []android.Path `android:"mutated"` + // If true, then only the headers are built and not the implementation jar. Headers_only *bool @@ -544,6 +547,11 @@ type Module struct { jarjarRenameRules map[string]string stubsLinkType StubsLinkType + + // Paths to the aconfig intermediate cache files that are provided by the + // java_aconfig_library or java_library modules that are statically linked + // to this module. Does not contain cache files from all transitive dependencies. + aconfigCacheFiles android.Paths } func (j *Module) CheckStableSdkVersion(ctx android.BaseModuleContext) error { @@ -1202,6 +1210,8 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath // final R classes from the app. flags.classpath = append(android.CopyOf(extraClasspathJars), flags.classpath...) + j.aconfigCacheFiles = append(deps.aconfigProtoFiles, j.properties.Aconfig_Cache_files...) + // If compiling headers then compile them and skip the rest if proptools.Bool(j.properties.Headers_only) { if srcFiles.HasExt(".kt") { @@ -1746,7 +1756,7 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars, extraClasspath ExportedPluginDisableTurbine: j.exportedDisableTurbine, JacocoReportClassesFile: j.jacocoReportClassesFile, StubsLinkType: j.stubsLinkType, - AconfigIntermediateCacheOutputPaths: deps.aconfigProtoFiles, + AconfigIntermediateCacheOutputPaths: j.aconfigCacheFiles, }) // Save the output file with no relative path so that it doesn't end up in a subdirectory when used as a resource @@ -2360,7 +2370,10 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { deps.staticHeaderJars = append(deps.staticHeaderJars, dep.Srcs()...) } } else if dep, ok := android.OtherModuleProvider(ctx, module, android.CodegenInfoProvider); ok { - deps.aconfigProtoFiles = append(deps.aconfigProtoFiles, dep.IntermediateCacheOutputPaths...) + switch tag { + case staticLibTag: + deps.aconfigProtoFiles = append(deps.aconfigProtoFiles, dep.IntermediateCacheOutputPaths...) + } } else { switch tag { case bootClasspathTag: diff --git a/java/droiddoc.go b/java/droiddoc.go index aec40b312..176779eb4 100644 --- a/java/droiddoc.go +++ b/java/droiddoc.go @@ -391,12 +391,14 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps { } else if dep, ok := android.OtherModuleProvider(ctx, module, JavaInfoProvider); ok { deps.classpath = append(deps.classpath, dep.HeaderJars...) deps.aidlIncludeDirs = append(deps.aidlIncludeDirs, dep.AidlIncludeDirs...) + deps.aconfigProtoFiles = append(deps.aconfigProtoFiles, dep.AconfigIntermediateCacheOutputPaths...) } else if dep, ok := module.(android.SourceFileProducer); ok { checkProducesJars(ctx, dep) deps.classpath = append(deps.classpath, dep.Srcs()...) } else { ctx.ModuleErrorf("depends on non-java module %q", otherName) } + case java9LibTag: if dep, ok := android.OtherModuleProvider(ctx, module, JavaInfoProvider); ok { deps.java9Classpath = append(deps.java9Classpath, dep.HeaderJars...) @@ -429,6 +431,19 @@ func (j *Javadoc) collectDeps(ctx android.ModuleContext) deps { srcFiles := android.PathsForModuleSrcExcludes(ctx, j.properties.Srcs, j.properties.Exclude_srcs) j.implicits = append(j.implicits, srcFiles...) + // Module can depend on a java_aconfig_library module using the ":module_name{.tag}" syntax. + // Find the corresponding aconfig_declarations module name for such case. + for _, src := range j.properties.Srcs { + if moduleName, tag := android.SrcIsModuleWithTag(src); moduleName != "" { + otherModule := android.GetModuleFromPathDep(ctx, moduleName, tag) + if otherModule != nil { + if dep, ok := android.OtherModuleProvider(ctx, otherModule, android.CodegenInfoProvider); ok { + deps.aconfigProtoFiles = append(deps.aconfigProtoFiles, dep.IntermediateCacheOutputPaths...) + } + } + } + } + filterByPackage := func(srcs []android.Path, filterPackages []string) []android.Path { if filterPackages == nil { return srcs diff --git a/java/generated_java_library.go b/java/generated_java_library.go index e8316ccc4..d5e6d8fec 100644 --- a/java/generated_java_library.go +++ b/java/generated_java_library.go @@ -34,7 +34,7 @@ type GeneratedJavaLibraryCallbacks interface { // Called from inside GenerateAndroidBuildActions. Add the build rules to // make the srcjar, and return the path to it. - GenerateSourceJarBuildActions(module *GeneratedJavaLibraryModule, ctx android.ModuleContext) android.Path + GenerateSourceJarBuildActions(module *GeneratedJavaLibraryModule, ctx android.ModuleContext) (android.Path, android.Path) } // GeneratedJavaLibraryModuleFactory provides a utility for modules that are generated @@ -103,8 +103,10 @@ 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(module, ctx) + srcJarPath, cacheOutputPath := module.callbacks.GenerateSourceJarBuildActions(module, ctx) + module.Library.properties.Generated_srcjars = append(module.Library.properties.Generated_srcjars, srcJarPath) + module.Library.properties.Aconfig_Cache_files = append(module.Library.properties.Aconfig_Cache_files, cacheOutputPath) module.Library.GenerateAndroidBuildActions(ctx) } diff --git a/java/generated_java_library_test.go b/java/generated_java_library_test.go index be816cda9..a5c4be111 100644 --- a/java/generated_java_library_test.go +++ b/java/generated_java_library_test.go @@ -36,8 +36,8 @@ type JavaGenLibTestCallbacks struct { func (callbacks *JavaGenLibTestCallbacks) DepsMutator(module *GeneratedJavaLibraryModule, ctx android.BottomUpMutatorContext) { } -func (callbacks *JavaGenLibTestCallbacks) GenerateSourceJarBuildActions(module *GeneratedJavaLibraryModule, ctx android.ModuleContext) android.Path { - return android.PathForOutput(ctx, "blah.srcjar") +func (callbacks *JavaGenLibTestCallbacks) GenerateSourceJarBuildActions(module *GeneratedJavaLibraryModule, ctx android.ModuleContext) (android.Path, android.Path) { + return android.PathForOutput(ctx, "blah.srcjar"), android.PathForOutput(ctx, "blah.pb") } func testGenLib(t *testing.T, errorHandler android.FixtureErrorHandler, bp string) *android.TestResult {