From 2069c3f74d02af1c5db923e210bcdd52f8daf5aa Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Wed, 6 Dec 2023 19:40:24 +0000 Subject: [PATCH 1/2] Move dexpreopt processing from java_*_import to prebuilt_apex dexpreopt of apex system server from prebuilts involves three soong modules 1. prebuilt_apex / apex_set 2. an internal deapexer module created by the prebuilt apex 3. java_import/java_sdk_library (3) acts as a shim for the deapexer to set the dexjar extracted from the prebuilt apex. This methodolody requires a 1:1 correspondence across the three modules This breaks down when we have multiple versions of the same prebuilt apex in the tree. In preparation for this, move the dexpreopt processing from (3) to (1). Each prebuilt_apex will create the necessary rules for dexpreopting the jars deapexed from itself. In the future, apex_contributions will be used to pick which service-foo.{odex|.vdex} to install depending on which prebuilt apex is selected. Implementation details - Embed dexpreopter in prebuiltApex structs so that this module type can register the dexpreopt rules. Since a single apex can have multiple system server jars, this also requires creating an additional scope in dexpreopt.go to prevent name collisions - Add the dexpreopt modules as required in initApexFilesForAndroidMk - Add the depreopt modules to androidMk in AndroidMkEntries. Drop the equivalent from java_import and java_sdk_library_import Bug: 308790457 Test: existing soong unit tests Test: lunch cf_x86_64_phone-next-userdebug && m out/target/product/vsoc_x86_64/system/apex/com.google.android.adservices.apex Test: Verified that the above command installs /out/target/product/vsoc_x86_64/system/framework/oat/x86_64/apex@com.android.adservices@javalib@service-adservices.jar@classes.{odex|vdex} and the equivalent files of service-sdksandbox Test: presubmits Change-Id: I01cea8956d2857fb864b415e73d3d2686d069b5e --- apex/apex_test.go | 2 +- apex/bootclasspath_fragment_test.go | 2 + apex/prebuilt.go | 92 +++++++++++--------- apex/systemserver_classpath_fragment_test.go | 2 + java/androidmk.go | 6 +- java/app_import_test.go | 16 ++-- java/app_test.go | 2 +- java/dexpreopt.go | 40 +++++++-- java/dexpreopt_test.go | 6 +- java/java_test.go | 2 - java/sdk_library.go | 8 -- 11 files changed, 101 insertions(+), 77 deletions(-) diff --git a/apex/apex_test.go b/apex/apex_test.go index abf6b1534..1ced39364 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -3725,7 +3725,7 @@ func ensureExactContents(t *testing.T, ctx *android.TestContext, moduleName, var } func ensureExactDeapexedContents(t *testing.T, ctx *android.TestContext, moduleName string, variant string, files []string) { - deapexer := ctx.ModuleForTests(moduleName+".deapexer", variant).Rule("deapexer") + deapexer := ctx.ModuleForTests(moduleName+".deapexer", variant).Description("deapex") outputs := make([]string, 0, len(deapexer.ImplicitOutputs)+1) if deapexer.Output != nil { outputs = append(outputs, deapexer.Output.String()) diff --git a/apex/bootclasspath_fragment_test.go b/apex/bootclasspath_fragment_test.go index 42f5cd444..159e9e1c4 100644 --- a/apex/bootclasspath_fragment_test.go +++ b/apex/bootclasspath_fragment_test.go @@ -530,6 +530,8 @@ func TestBootclasspathFragmentInPrebuiltArtApex(t *testing.T) { java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art", []string{ `com.android.art.apex.selector`, + `com.android.art.deapexer`, + `dex2oatd`, `prebuilt_art-bootclasspath-fragment`, }) diff --git a/apex/prebuilt.go b/apex/prebuilt.go index 179d90b17..4c44678e7 100644 --- a/apex/prebuilt.go +++ b/apex/prebuilt.go @@ -22,6 +22,7 @@ import ( "strings" "android/soong/android" + "android/soong/dexpreopt" "android/soong/java" "android/soong/provenance" @@ -50,6 +51,7 @@ type prebuilt interface { type prebuiltCommon struct { android.ModuleBase + java.Dexpreopter prebuilt android.Prebuilt // Properties common to both prebuilt_apex and apex_set. @@ -170,50 +172,39 @@ func (p *prebuiltCommon) installable() bool { return proptools.BoolDefault(p.prebuiltCommonProperties.Installable, true) } -// initApexFilesForAndroidMk initializes the prebuiltCommon.apexFilesForAndroidMk field from the -// modules that this depends upon. +// To satisfy java.DexpreopterInterface +func (p *prebuiltCommon) IsInstallable() bool { + return p.installable() +} + +// initApexFilesForAndroidMk initializes the prebuiltCommon.requiredModuleNames field with the install only deps of the prebuilt apex func (p *prebuiltCommon) initApexFilesForAndroidMk(ctx android.ModuleContext) { - // Walk the dependencies of this module looking for the java modules that it exports. - ctx.WalkDeps(func(child, parent android.Module) bool { - tag := ctx.OtherModuleDependencyTag(child) + // If this apex contains a system server jar, then the dexpreopt artifacts should be added as required + for _, install := range p.Dexpreopter.DexpreoptBuiltInstalledForApex() { + p.requiredModuleNames = append(p.requiredModuleNames, install.FullModuleName()) + } +} - name := android.RemoveOptionalPrebuiltPrefix(ctx.OtherModuleName(child)) - if java.IsBootclasspathFragmentContentDepTag(tag) || - java.IsSystemServerClasspathFragmentContentDepTag(tag) || tag == exportedJavaLibTag { - // If the exported java module provides a dex jar path then add it to the list of apexFiles. - path := child.(interface { - DexJarBuildPath() java.OptionalDexJarPath - }).DexJarBuildPath() - if path.IsSet() { - af := apexFile{ - module: child, - moduleDir: ctx.OtherModuleDir(child), - androidMkModuleName: name, - builtFile: path.Path(), - class: javaSharedLib, - } - if module, ok := child.(java.DexpreopterInterface); ok { - for _, install := range module.DexpreoptBuiltInstalledForApex() { - af.requiredModuleNames = append(af.requiredModuleNames, install.FullModuleName()) - } - } - p.apexFilesForAndroidMk = append(p.apexFilesForAndroidMk, af) - } - } else if tag == exportedBootclasspathFragmentTag { - _, ok := child.(*java.PrebuiltBootclasspathFragmentModule) - if !ok { - ctx.PropertyErrorf("exported_bootclasspath_fragments", "%q is not a prebuilt_bootclasspath_fragment module", name) - return false - } - // Visit the children of the bootclasspath_fragment. - return true - } else if tag == exportedSystemserverclasspathFragmentTag { - // Visit the children of the systemserver_fragment. - return true +// If this prebuilt has system server jar, create the rules to dexpreopt it and install it alongside the prebuilt apex +func (p *prebuiltCommon) dexpreoptSystemServerJars(ctx android.ModuleContext) { + // If this apex does not export anything, return + if !p.hasExportedDeps() { + return + } + // Use apex_name to determine the api domain of this prebuilt apex + apexName := p.ApexVariationName() + di := android.FindDeapexerProviderForModule(ctx) + dc := dexpreopt.GetGlobalConfig(ctx) + systemServerJarList := dc.AllApexSystemServerJars(ctx) + + for i := 0; i < systemServerJarList.Len(); i++ { + sscpApex := systemServerJarList.Apex(i) + sscpJar := systemServerJarList.Jar(i) + if apexName != sscpApex { + continue } - - return false - }) + p.Dexpreopter.DexpreoptPrebuiltApexSystemServerJars(ctx, sscpJar, di) + } } func (p *prebuiltCommon) addRequiredModules(entries *android.AndroidMkEntries) { @@ -248,6 +239,11 @@ func (p *prebuiltCommon) AndroidMkEntries() []android.AndroidMkEntries { }, } + // Add the dexpreopt artifacts to androidmk + for _, install := range p.Dexpreopter.DexpreoptBuiltInstalledForApex() { + entriesList = append(entriesList, install.ToMakeEntries()) + } + // Iterate over the apexFilesForAndroidMk list and create an AndroidMkEntries struct for each // file. This provides similar behavior to that provided in apexBundle.AndroidMk() as it makes the // apex specific variants of the exported java modules available for use from within make. @@ -756,6 +752,14 @@ func (p *Prebuilt) ComponentDepsMutator(ctx android.BottomUpMutatorContext) { p.prebuiltApexContentsDeps(ctx) } +func (p *prebuiltCommon) DepsMutator(ctx android.BottomUpMutatorContext) { + if p.hasExportedDeps() { + // Create a dependency from the prebuilt apex (prebuilt_apex/apex_set) to the internal deapexer module + // The deapexer will return a provider that will be bubbled up to the rdeps of apexes (e.g. dex_bootjars) + ctx.AddDependency(ctx.Module(), android.DeapexerTag, deapexerModuleName(p.BaseModuleName())) + } +} + var _ ApexInfoMutator = (*Prebuilt)(nil) func (p *Prebuilt) ApexInfoMutator(mctx android.TopDownMutatorContext) { @@ -783,6 +787,9 @@ func (p *Prebuilt) GenerateAndroidBuildActions(ctx android.ModuleContext) { return } + // dexpreopt any system server jars if present + p.dexpreoptSystemServerJars(ctx) + // Save the files that need to be made available to Make. p.initApexFilesForAndroidMk(ctx) @@ -999,6 +1006,9 @@ func (a *ApexSet) GenerateAndroidBuildActions(ctx android.ModuleContext) { return } + // dexpreopt any system server jars if present + a.dexpreoptSystemServerJars(ctx) + // Save the files that need to be made available to Make. a.initApexFilesForAndroidMk(ctx) diff --git a/apex/systemserver_classpath_fragment_test.go b/apex/systemserver_classpath_fragment_test.go index 40d05814e..90fd2caa9 100644 --- a/apex/systemserver_classpath_fragment_test.go +++ b/apex/systemserver_classpath_fragment_test.go @@ -272,7 +272,9 @@ func TestPrebuiltSystemserverclasspathFragmentContents(t *testing.T) { ctx := result.TestContext java.CheckModuleDependencies(t, ctx, "myapex", "android_common_myapex", []string{ + `dex2oatd`, `myapex.apex.selector`, + `myapex.deapexer`, `prebuilt_mysystemserverclasspathfragment`, }) diff --git a/java/androidmk.go b/java/androidmk.go index 809f9b563..cbf9abb96 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -207,11 +207,7 @@ func (j *TestHelperLibrary) AndroidMkEntries() []android.AndroidMkEntries { func (prebuilt *Import) AndroidMkEntries() []android.AndroidMkEntries { if prebuilt.hideApexVariantFromMake { - // For a library imported from a prebuilt APEX, we don't need a Make module for itself, as we - // don't need to install it. However, we need to add its dexpreopt outputs as sub-modules, if it - // is preopted. - dexpreoptEntries := prebuilt.dexpreopter.AndroidMkEntriesForApex() - return append(dexpreoptEntries, android.AndroidMkEntries{Disabled: true}) + return []android.AndroidMkEntries{} } return []android.AndroidMkEntries{android.AndroidMkEntries{ Class: "JAVA_LIBRARIES", diff --git a/java/app_import_test.go b/java/app_import_test.go index 8f29bb373..ef4626e26 100644 --- a/java/app_import_test.go +++ b/java/app_import_test.go @@ -40,8 +40,8 @@ func TestAndroidAppImport(t *testing.T) { variant := ctx.ModuleForTests("foo", "android_common") // Check dexpreopt outputs. - if variant.MaybeOutput("dexpreopt/oat/arm64/package.vdex").Rule == nil || - variant.MaybeOutput("dexpreopt/oat/arm64/package.odex").Rule == nil { + if variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.vdex").Rule == nil || + variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.odex").Rule == nil { t.Errorf("can't find dexpreopt outputs") } @@ -74,8 +74,8 @@ func TestAndroidAppImport_NoDexPreopt(t *testing.T) { variant := ctx.ModuleForTests("foo", "android_common") // Check dexpreopt outputs. They shouldn't exist. - if variant.MaybeOutput("dexpreopt/oat/arm64/package.vdex").Rule != nil || - variant.MaybeOutput("dexpreopt/oat/arm64/package.odex").Rule != nil { + if variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.vdex").Rule != nil || + variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.odex").Rule != nil { t.Errorf("dexpreopt shouldn't have run.") } @@ -101,8 +101,8 @@ func TestAndroidAppImport_Presigned(t *testing.T) { variant := ctx.ModuleForTests("foo", "android_common") // Check dexpreopt outputs. - if variant.MaybeOutput("dexpreopt/oat/arm64/package.vdex").Rule == nil || - variant.MaybeOutput("dexpreopt/oat/arm64/package.odex").Rule == nil { + if variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.vdex").Rule == nil || + variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.odex").Rule == nil { t.Errorf("can't find dexpreopt outputs") } // Make sure signing was skipped and aligning was done. @@ -210,8 +210,8 @@ func TestAndroidAppImport_DefaultDevCert(t *testing.T) { variant := ctx.ModuleForTests("foo", "android_common") // Check dexpreopt outputs. - if variant.MaybeOutput("dexpreopt/oat/arm64/package.vdex").Rule == nil || - variant.MaybeOutput("dexpreopt/oat/arm64/package.odex").Rule == nil { + if variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.vdex").Rule == nil || + variant.MaybeOutput("dexpreopt/foo/oat/arm64/package.odex").Rule == nil { t.Errorf("can't find dexpreopt outputs") } diff --git a/java/app_test.go b/java/app_test.go index 0936b281a..861c04761 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -3357,7 +3357,7 @@ func TestUsesLibraries(t *testing.T) { cmd := app.Rule("dexpreopt").RuleParams.Command android.AssertStringDoesContain(t, "dexpreopt app cmd context", cmd, "--context-json=") android.AssertStringDoesContain(t, "dexpreopt app cmd product_packages", cmd, - "--product-packages=out/soong/.intermediates/app/android_common/dexpreopt/product_packages.txt") + "--product-packages=out/soong/.intermediates/app/android_common/dexpreopt/app/product_packages.txt") } func TestDexpreoptBcp(t *testing.T) { diff --git a/java/dexpreopt.go b/java/dexpreopt.go index 0f69dc3a4..1e289c537 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -79,18 +79,25 @@ func (install *dexpreopterInstall) SubModuleName() string { func (install dexpreopterInstall) ToMakeEntries() android.AndroidMkEntries { return android.AndroidMkEntries{ Class: "ETC", - SubName: install.SubModuleName(), OutputFile: android.OptionalPathForPath(install.outputPathOnHost), ExtraEntries: []android.AndroidMkExtraEntriesFunc{ func(ctx android.AndroidMkExtraEntriesContext, entries *android.AndroidMkEntries) { + entries.SetString("LOCAL_MODULE", install.FullModuleName()) entries.SetString("LOCAL_MODULE_PATH", install.installDirOnDevice.String()) entries.SetString("LOCAL_INSTALLED_MODULE_STEM", install.installFileOnDevice) entries.SetString("LOCAL_NOT_AVAILABLE_FOR_PLATFORM", "false") + // Unset LOCAL_SOONG_INSTALLED_MODULE so that this does not default to the primary .apex file + // Without this, installation of the dexpreopt artifacts get skipped + entries.SetString("LOCAL_SOONG_INSTALLED_MODULE", "") }, }, } } +type Dexpreopter struct { + dexpreopter +} + type dexpreopter struct { dexpreoptProperties DexpreoptProperties importDexpreoptProperties ImportDexpreoptProperties @@ -258,6 +265,17 @@ func (d *dexpreopter) getInstallPath( return defaultInstallPath } +// DexpreoptPrebuiltApexSystemServerJars generates the dexpreopt artifacts from a jar file that has been deapexed from a prebuilt apex +func (d *Dexpreopter) DexpreoptPrebuiltApexSystemServerJars(ctx android.ModuleContext, libraryName string, di *android.DeapexerInfo) { + // A single prebuilt apex can have multiple apex system jars + // initialize the output path for this dex jar + dc := dexpreopt.GetGlobalConfig(ctx) + d.installPath = android.PathForModuleInPartitionInstall(ctx, "", strings.TrimPrefix(dexpreopt.GetSystemServerDexLocation(ctx, dc, libraryName), "/")) + // generate the rules for creating the .odex and .vdex files for this system server jar + dexJarFile := di.PrebuiltExportPath(apexRootRelativePathToJavaLib(libraryName)) + d.dexpreopt(ctx, dexJarFile) +} + func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.WritablePath) { global := dexpreopt.GetGlobalConfig(ctx) @@ -346,11 +364,15 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr d.dexpreoptProperties.Dex_preopt_result.Profile_guided = profileClassListing.Valid() + // A single apex can have multiple system server jars + // Use the dexJar to create a unique scope for each + dexJarStem := strings.TrimSuffix(dexJarFile.Base(), dexJarFile.Ext()) + // Full dexpreopt config, used to create dexpreopt build rules. dexpreoptConfig := &dexpreopt.ModuleConfig{ Name: moduleName(ctx), DexLocation: dexLocation, - BuildPath: android.PathForModuleOut(ctx, "dexpreopt", moduleName(ctx)+".jar").OutputPath, + BuildPath: android.PathForModuleOut(ctx, "dexpreopt", dexJarStem, moduleName(ctx)+".jar").OutputPath, DexPath: dexJarFile, ManifestPath: android.OptionalPathForPath(d.manifestFile), UncompressedDex: d.uncompressedDex, @@ -380,7 +402,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr PresignedPrebuilt: d.isPresignedPrebuilt, } - d.configPath = android.PathForModuleOut(ctx, "dexpreopt", "dexpreopt.config") + d.configPath = android.PathForModuleOut(ctx, "dexpreopt", dexJarStem, "dexpreopt.config") dexpreopt.WriteModuleConfig(ctx, dexpreoptConfig, d.configPath) if d.dexpreoptDisabled(ctx) { @@ -394,7 +416,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr // dependencies to create a per-app list, and use `rsync --checksum` to prevent the file's mtime // from being changed if the contents don't change. This avoids unnecessary dexpreopt reruns. productPackages := android.PathForModuleInPartitionInstall(ctx, "", "product_packages.txt") - appProductPackages := android.PathForModuleOut(ctx, "dexpreopt", "product_packages.txt") + appProductPackages := android.PathForModuleOut(ctx, "dexpreopt", dexJarStem, "product_packages.txt") appProductPackagesStaging := appProductPackages.ReplaceExtension(ctx, "txt.tmp") clcNames, _ := dexpreopt.ComputeClassLoaderContextDependencies(dexpreoptConfig.ClassLoaderContexts) sort.Strings(clcNames) // The order needs to be deterministic. @@ -416,7 +438,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr Text("rsync --checksum"). Input(appProductPackagesStaging). Output(appProductPackages) - productPackagesRule.Restat().Build("product_packages", "dexpreopt product_packages") + productPackagesRule.Restat().Build("product_packages."+dexJarStem, "dexpreopt product_packages") dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule( ctx, globalSoong, global, dexpreoptConfig, appProductPackages) @@ -425,9 +447,11 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr return } - dexpreoptRule.Build("dexpreopt", "dexpreopt") + dexpreoptRule.Build("dexpreopt"+"."+dexJarStem, "dexpreopt") - isApexSystemServerJar := global.AllApexSystemServerJars(ctx).ContainsJar(moduleName(ctx)) + // The current ctx might be of a deapexer module created by a prebuilt apex + // Use the path of the dex file to determine the library name + isApexSystemServerJar := global.AllApexSystemServerJars(ctx).ContainsJar(dexJarStem) for _, install := range dexpreoptRule.Installs() { // Remove the "/" prefix because the path should be relative to $ANDROID_PRODUCT_OUT. @@ -452,7 +476,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Wr // The installs will be handled by Make as sub-modules of the java library. d.builtInstalledForApex = append(d.builtInstalledForApex, dexpreopterInstall{ name: arch + "-" + installBase, - moduleName: moduleName(ctx), + moduleName: dexJarStem, outputPathOnHost: install.From, installDirOnDevice: installPath, installFileOnDevice: installBase, diff --git a/java/dexpreopt_test.go b/java/dexpreopt_test.go index fedd5640e..73e33f4fb 100644 --- a/java/dexpreopt_test.go +++ b/java/dexpreopt_test.go @@ -410,7 +410,7 @@ func TestAndroidMkEntriesForApex(t *testing.T) { verifyEntries(t, "entriesList[0]", "service-foo-dexpreopt-arm64-apex@com.android.apex1@javalib@service-foo.jar@classes.odex", - "/dexpreopt/oat/arm64/javalib.odex", + "/dexpreopt/service-foo/oat/arm64/javalib.odex", "/system/framework/oat/arm64", "apex@com.android.apex1@javalib@service-foo.jar@classes.odex", entriesList[0]) @@ -418,7 +418,7 @@ func TestAndroidMkEntriesForApex(t *testing.T) { verifyEntries(t, "entriesList[1]", "service-foo-dexpreopt-arm64-apex@com.android.apex1@javalib@service-foo.jar@classes.vdex", - "/dexpreopt/oat/arm64/javalib.vdex", + "/dexpreopt/service-foo/oat/arm64/javalib.vdex", "/system/framework/oat/arm64", "apex@com.android.apex1@javalib@service-foo.jar@classes.vdex", entriesList[1]) @@ -459,7 +459,7 @@ func TestGenerateProfileEvenIfDexpreoptIsDisabled(t *testing.T) { ctx := result.TestContext dexpreopt := ctx.ModuleForTests("foo", "android_common").MaybeRule("dexpreopt") - expected := []string{"out/soong/.intermediates/foo/android_common/dexpreopt/profile.prof"} + expected := []string{"out/soong/.intermediates/foo/android_common/dexpreopt/foo/profile.prof"} android.AssertArrayString(t, "outputs", expected, dexpreopt.AllOutputs()) } diff --git a/java/java_test.go b/java/java_test.go index e21018c39..2368b6cfe 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -618,8 +618,6 @@ func TestPrebuilts(t *testing.T) { android.AssertStringEquals(t, "unexpected LOCAL_SOONG_MODULE_TYPE", "java_library", entries.EntryMap["LOCAL_SOONG_MODULE_TYPE"][0]) entries = android.AndroidMkEntriesForTest(t, ctx, barModule.Module())[0] android.AssertStringEquals(t, "unexpected LOCAL_SOONG_MODULE_TYPE", "java_import", entries.EntryMap["LOCAL_SOONG_MODULE_TYPE"][0]) - entries = android.AndroidMkEntriesForTest(t, ctx, ctx.ModuleForTests("sdklib", "android_common").Module())[0] - android.AssertStringEquals(t, "unexpected LOCAL_SOONG_MODULE_TYPE", "java_sdk_library_import", entries.EntryMap["LOCAL_SOONG_MODULE_TYPE"][0]) } func assertDeepEquals(t *testing.T, message string, expected interface{}, actual interface{}) { diff --git a/java/sdk_library.go b/java/sdk_library.go index 0584281a5..a634b1749 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2591,14 +2591,6 @@ func (module *SdkLibraryImport) DepsMutator(ctx android.BottomUpMutatorContext) } } -func (module *SdkLibraryImport) AndroidMkEntries() []android.AndroidMkEntries { - // For an SDK library imported from a prebuilt APEX, we don't need a Make module for itself, as we - // don't need to install it. However, we need to add its dexpreopt outputs as sub-modules, if it - // is preopted. - dexpreoptEntries := module.dexpreopter.AndroidMkEntriesForApex() - return append(dexpreoptEntries, android.AndroidMkEntries{Disabled: true}) -} - var _ android.ApexModule = (*SdkLibraryImport)(nil) // Implements android.ApexModule From fae468ef14fa7fd902c8e004dbd55a3e79b8a30f Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 12 Dec 2023 23:23:53 +0000 Subject: [PATCH 2/2] Move validation from FindDeapexerProviderForModule to rdeps FindDeapexerProviderForModule raises an exception if multiple apexes in the tree has an export dep on the java module. In prepartation to support multiple prebuilts, move this error check out of FindDeapexerProviderForModule and into rdeps. i.e. raise an exception only if an rdep calls DexJarBuildPath - This should be a no-op for now. - In the short-term future, a java import module will be allowed to have multiple deapexers. An error will be raised if anyone actually tries to depend on the dexjar - In the long-term future, this function will be removed. All processing will be done at the prebuilt apex level and not at the prebuilt java library level Since this check now happens in the moduleCtx of rdeps, add some additional props to unit tests to ensure that it does not exit early on unrelated validation checks (e.g. hidden_api prop is not set) Test: go test ./apex ./java Bug: 308790457 Change-Id: I3323d993c1ea8f43305834cae8e65b6fe41dfefd --- android/deapexer.go | 22 +++++++++++++++------- apex/apex_test.go | 9 +++++++++ apex/prebuilt.go | 5 ++++- java/bootclasspath_fragment.go | 13 ++++++++++--- java/java.go | 13 ++++++++++--- java/sdk_library.go | 15 +++++++++++---- 6 files changed, 59 insertions(+), 18 deletions(-) diff --git a/android/deapexer.go b/android/deapexer.go index de933d1a4..fb2073dcd 100644 --- a/android/deapexer.go +++ b/android/deapexer.go @@ -15,6 +15,7 @@ package android import ( + "fmt" "strings" "github.com/google/blueprint" @@ -146,10 +147,16 @@ type RequiresFilesFromPrebuiltApexTag interface { // FindDeapexerProviderForModule searches through the direct dependencies of the current context // module for a DeapexerTag dependency and returns its DeapexerInfo. If a single nonambiguous -// deapexer module isn't found then errors are reported with ctx.ModuleErrorf and nil is returned. -func FindDeapexerProviderForModule(ctx ModuleContext) *DeapexerInfo { +// deapexer module isn't found then it returns it an error +// clients should check the value of error and call ctx.ModuleErrof if a non nil error is received +func FindDeapexerProviderForModule(ctx ModuleContext) (*DeapexerInfo, error) { var di *DeapexerInfo + var err error ctx.VisitDirectDepsWithTag(DeapexerTag, func(m Module) { + if err != nil { + // An err has been found. Do not visit further. + return + } c, _ := OtherModuleProvider(ctx, m, DeapexerProvider) p := &c if di != nil { @@ -159,17 +166,18 @@ func FindDeapexerProviderForModule(ctx ModuleContext) *DeapexerInfo { di = selected return } - ctx.ModuleErrorf("Multiple installable prebuilt APEXes provide ambiguous deapexers: %s and %s", - di.ApexModuleName(), p.ApexModuleName()) + err = fmt.Errorf("Multiple installable prebuilt APEXes provide ambiguous deapexers: %s and %s", di.ApexModuleName(), p.ApexModuleName()) } di = p }) + if err != nil { + return nil, err + } if di != nil { - return di + return di, nil } ai, _ := ModuleProvider(ctx, ApexInfoProvider) - ctx.ModuleErrorf("No prebuilt APEX provides a deapexer module for APEX variant %s", ai.ApexVariationName) - return nil + return nil, fmt.Errorf("No prebuilt APEX provides a deapexer module for APEX variant %s", ai.ApexVariationName) } // removeCompressedApexSuffix removes the _compressed suffix from the name if present. diff --git a/apex/apex_test.go b/apex/apex_test.go index 1ced39364..6c7dabccb 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -8432,6 +8432,13 @@ func TestDuplicateDeapexersFromPrebuiltApexes(t *testing.T) { prebuilt_bootclasspath_fragment { name: "my-bootclasspath-fragment", apex_available: ["com.android.myapex"], + hidden_api: { + annotation_flags: "my-bootclasspath-fragment/annotation-flags.csv", + metadata: "my-bootclasspath-fragment/metadata.csv", + index: "my-bootclasspath-fragment/index.csv", + stub_flags: "my-bootclasspath-fragment/stub-flags.csv", + all_flags: "my-bootclasspath-fragment/all-flags.csv", + }, %s } ` @@ -8453,6 +8460,7 @@ func TestDuplicateDeapexersFromPrebuiltApexes(t *testing.T) { public: { jars: ["libbar.jar"], }, + shared_library: false, apex_available: ["com.android.myapex"], } `) @@ -8468,6 +8476,7 @@ func TestDuplicateDeapexersFromPrebuiltApexes(t *testing.T) { public: { jars: ["libbar.jar"], }, + shared_library: false, apex_available: ["com.android.myapex"], } `) diff --git a/apex/prebuilt.go b/apex/prebuilt.go index 4c44678e7..b13ecc2fd 100644 --- a/apex/prebuilt.go +++ b/apex/prebuilt.go @@ -193,7 +193,10 @@ func (p *prebuiltCommon) dexpreoptSystemServerJars(ctx android.ModuleContext) { } // Use apex_name to determine the api domain of this prebuilt apex apexName := p.ApexVariationName() - di := android.FindDeapexerProviderForModule(ctx) + di, err := android.FindDeapexerProviderForModule(ctx) + if err != nil { + ctx.ModuleErrorf(err.Error()) + } dc := dexpreopt.GetGlobalConfig(ctx) systemServerJarList := dc.AllApexSystemServerJars(ctx) diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go index d2bb52315..83030b51e 100644 --- a/java/bootclasspath_fragment.go +++ b/java/bootclasspath_fragment.go @@ -240,7 +240,8 @@ type BootclasspathFragmentModule struct { sourceOnlyProperties SourceOnlyBootclasspathProperties // Path to the boot image profile. - profilePath android.WritablePath + profilePath android.WritablePath + profilePathErr error } // commonBootclasspathFragment defines the methods that are implemented by both source and prebuilt @@ -1065,8 +1066,11 @@ func (module *PrebuiltBootclasspathFragmentModule) produceBootImageProfile(ctx a return nil } - di := android.FindDeapexerProviderForModule(ctx) - if di == nil { + di, err := android.FindDeapexerProviderForModule(ctx) + if err != nil { + // An error was found, possibly due to multiple apexes in the tree that export this library + // Defer the error till a client tries to call getProfilePath + module.profilePathErr = err return nil // An error has been reported by FindDeapexerProviderForModule. } @@ -1074,6 +1078,9 @@ func (module *PrebuiltBootclasspathFragmentModule) produceBootImageProfile(ctx a } func (b *PrebuiltBootclasspathFragmentModule) getProfilePath() android.Path { + if b.profilePathErr != nil { + panic(b.profilePathErr.Error()) + } return b.profilePath } diff --git a/java/java.go b/java/java.go index 630318e6d..51e8c344c 100644 --- a/java/java.go +++ b/java/java.go @@ -2100,6 +2100,7 @@ type Import struct { // output file containing classes.dex and resources dexJarFile OptionalDexJarPath + dexJarFileErr error dexJarInstallFile android.Path combinedClasspathFile android.Path @@ -2250,9 +2251,12 @@ func (j *Import) GenerateAndroidBuildActions(ctx android.ModuleContext) { ai, _ := android.ModuleProvider(ctx, android.ApexInfoProvider) if ai.ForPrebuiltApex { // Get the path of the dex implementation jar from the `deapexer` module. - di := android.FindDeapexerProviderForModule(ctx) - if di == nil { - return // An error has been reported by FindDeapexerProviderForModule. + di, err := android.FindDeapexerProviderForModule(ctx) + if err != nil { + // An error was found, possibly due to multiple apexes in the tree that export this library + // Defer the error till a client tries to call DexJarBuildPath + j.dexJarFileErr = err + return } dexJarFileApexRootRelative := apexRootRelativePathToJavaLib(j.BaseModuleName()) if dexOutputPath := di.PrebuiltExportPath(dexJarFileApexRootRelative); dexOutputPath != nil { @@ -2375,6 +2379,9 @@ func (j *Import) ImplementationAndResourcesJars() android.Paths { } func (j *Import) DexJarBuildPath() OptionalDexJarPath { + if j.dexJarFileErr != nil { + panic(j.dexJarFileErr.Error()) + } return j.dexJarFile } diff --git a/java/sdk_library.go b/java/sdk_library.go index a634b1749..38bd301f4 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -2378,7 +2378,8 @@ type SdkLibraryImport struct { xmlPermissionsFileModule *sdkLibraryXml // Build path to the dex implementation jar obtained from the prebuilt_apex, if any. - dexJarFile OptionalDexJarPath + dexJarFile OptionalDexJarPath + dexJarFileErr error // Expected install file path of the source module(sdk_library) // or dex implementation jar obtained from the prebuilt_apex, if any. @@ -2687,9 +2688,12 @@ func (module *SdkLibraryImport) GenerateAndroidBuildActions(ctx android.ModuleCo ai, _ := android.ModuleProvider(ctx, android.ApexInfoProvider) if ai.ForPrebuiltApex { // Get the path of the dex implementation jar from the `deapexer` module. - di := android.FindDeapexerProviderForModule(ctx) - if di == nil { - return // An error has been reported by FindDeapexerProviderForModule. + di, err := android.FindDeapexerProviderForModule(ctx) + if err != nil { + // An error was found, possibly due to multiple apexes in the tree that export this library + // Defer the error till a client tries to call DexJarBuildPath + module.dexJarFileErr = err + return } dexJarFileApexRootRelative := apexRootRelativePathToJavaLib(module.BaseModuleName()) if dexOutputPath := di.PrebuiltExportPath(dexJarFileApexRootRelative); dexOutputPath != nil { @@ -2751,6 +2755,9 @@ func (module *SdkLibraryImport) SdkImplementationJars(ctx android.BaseModuleCont func (module *SdkLibraryImport) DexJarBuildPath() OptionalDexJarPath { // The dex implementation jar extracted from the .apex file should be used in preference to the // source. + if module.dexJarFileErr != nil { + panic(module.dexJarFileErr.Error()) + } if module.dexJarFile.IsSet() { return module.dexJarFile }