From dd63d6d7bdae32a1bf89a1ce8ade15ee10afd6cc Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Wed, 3 Feb 2021 18:34:00 +0000 Subject: [PATCH 1/2] Improve module filtering in hiddenapi stubFlagsRule() Previously, it ignored any module that was not a platform apex variant however that required every module that was referenced from the boot jars to have a platform variant which is not the case when building from prebuilts. This change switches it to explicitly check that the variant is for either the appropriate apex or the platform depending on what is configured in the BootJars or UpdatableBootJars. It partially duplicates some logic from the getBootImageJar() function. It intentionally does not refactor the getBootImageJar() to allow for reuse because coupling the hiddenapi and dexpreopt logic would make refactoring either of them more difficult. Any duplicated code will be deduped once they have both been refactored. Bug: 178361284 Test: m droid Change-Id: I4b4e0dc8ee40c1ba1713d7ef72df13d175e84af6 --- java/dexpreopt_bootjars.go | 5 ++++ java/hiddenapi_singleton.go | 52 +++++++++++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go index 2a7eb42dc..86b189558 100644 --- a/java/dexpreopt_bootjars.go +++ b/java/dexpreopt_bootjars.go @@ -435,6 +435,11 @@ func (d *dexpreoptBootJars) GenerateSingletonBuildActions(ctx android.SingletonC // Inspect this module to see if it contains a bootclasspath dex jar. // Note that the same jar may occur in multiple modules. // This logic is tested in the apex package to avoid import cycle apex <-> java. +// +// This is similar to logic in isModuleInConfiguredList() so any changes needed here are likely to +// be needed there too. +// +// TODO(b/177892522): Avoid having to perform this type of check or if necessary dedup it. func getBootImageJar(ctx android.SingletonContext, image *bootImageConfig, module android.Module) (int, android.Path) { name := ctx.ModuleName(module) diff --git a/java/hiddenapi_singleton.go b/java/hiddenapi_singleton.go index ccb874506..32d1e3faa 100644 --- a/java/hiddenapi_singleton.go +++ b/java/hiddenapi_singleton.go @@ -213,6 +213,10 @@ func stubFlagsRule(ctx android.SingletonContext) { var bootDexJars android.Paths + // Get the configured non-updatable and updatable boot jars. + nonUpdatableBootJars := ctx.Config().NonUpdatableBootJars() + updatableBootJars := ctx.Config().UpdatableBootJars() + ctx.VisitAllModules(func(module android.Module) { // Collect dex jar paths for the modules listed above. if j, ok := module.(Dependency); ok { @@ -227,11 +231,8 @@ func stubFlagsRule(ctx android.SingletonContext) { // Collect dex jar paths for modules that had hiddenapi encode called on them. if h, ok := module.(hiddenAPIIntf); ok { if jar := h.bootDexJar(); jar != nil { - // For a java lib included in an APEX, only take the one built for - // the platform variant, and skip the variants for APEXes. - // Otherwise, the hiddenapi tool will complain about duplicated classes - apexInfo := ctx.ModuleProvider(module, android.ApexInfoProvider).(android.ApexInfo) - if !apexInfo.IsForPlatform() { + if !isModuleInConfiguredList(ctx, module, nonUpdatableBootJars) && + !isModuleInConfiguredList(ctx, module, updatableBootJars) { return } @@ -280,6 +281,47 @@ func stubFlagsRule(ctx android.SingletonContext) { rule.Build("hiddenAPIStubFlagsFile", "hiddenapi stub flags") } +// Checks to see whether the supplied module variant is in the list of boot jars. +// +// This is similar to logic in getBootImageJar() so any changes needed here are likely to be needed +// there too. +// +// TODO(b/179354495): Avoid having to perform this type of check or if necessary dedup it. +func isModuleInConfiguredList(ctx android.SingletonContext, module android.Module, configuredBootJars android.ConfiguredJarList) bool { + name := ctx.ModuleName(module) + + // Strip a prebuilt_ prefix so that this can match a prebuilt module that has not been renamed. + name = android.RemoveOptionalPrebuiltPrefix(name) + + // Ignore any module that is not listed in the boot image configuration. + index := configuredBootJars.IndexOfJar(name) + if index == -1 { + return false + } + + // It is an error if the module is not an ApexModule. + if _, ok := module.(android.ApexModule); !ok { + ctx.Errorf("module %q configured in boot jars does not support being added to an apex", module) + return false + } + + apexInfo := ctx.ModuleProvider(module, android.ApexInfoProvider).(android.ApexInfo) + + // Now match the apex part of the boot image configuration. + requiredApex := configuredBootJars.Apex(index) + if requiredApex == "platform" { + if len(apexInfo.InApexes) != 0 { + // A platform variant is required but this is for an apex so ignore it. + return false + } + } else if !apexInfo.InApexByBaseName(requiredApex) { + // An apex variant for a specific apex is required but this is the wrong apex. + return false + } + + return true +} + func prebuiltFlagsRule(ctx android.SingletonContext) android.Path { outputPath := hiddenAPISingletonPaths(ctx).flags inputPath := android.PathForSource(ctx, ctx.Config().PrebuiltHiddenApiDir(ctx), "hiddenapi-flags.csv") From 22ff0aaf51bdd9409eafe74151decb58f47308bc Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Thu, 4 Feb 2021 11:15:34 +0000 Subject: [PATCH 2/2] Export implementation class jars for java_boot_libs Hiddenapi processing currently requires access to the class implementation jars for libraries on the bootclasspath which means that they need to be provided as part of the prebuilts. This change modifies the java_boot_libs property on the sdk to make those files available. Modularization of the hiddenapi processing will hopefully remove the need for these to be exported so this should be temporary. Bug: 178361284 Test: m art-module-sdk check generated snapshot zip contains implementation jars Change-Id: I9e94662dddb0ddb85a477ae6d27e533085147e88 --- android/sdk.go | 6 ------ java/java.go | 35 +++++++++++++++-------------------- sdk/java_sdk_test.go | 7 +++---- sdk/update.go | 16 ---------------- 4 files changed, 18 insertions(+), 46 deletions(-) diff --git a/android/sdk.go b/android/sdk.go index bab0ed8fd..f2cdc880b 100644 --- a/android/sdk.go +++ b/android/sdk.go @@ -177,12 +177,6 @@ type SnapshotBuilder interface { // to the zip CopyToSnapshot(src Path, dest string) - // Return the path to an empty file. - // - // This can be used by sdk member types that need to create an empty file in the snapshot, simply - // pass the value returned from this to the CopyToSnapshot() method. - EmptyFile() Path - // Unzip the supplied zip into the snapshot relative directory destDir. UnzipToSnapshot(zipPath Path, destDir string) diff --git a/java/java.go b/java/java.go index 78d974b42..7d0b206ac 100644 --- a/java/java.go +++ b/java/java.go @@ -40,18 +40,21 @@ func init() { // Register sdk member types. android.RegisterSdkMemberType(javaHeaderLibsSdkMemberType) + // Export implementation classes jar as part of the sdk. + exportImplementationClassesJar := func(_ android.SdkMemberContext, j *Library) android.Path { + implementationJars := j.ImplementationAndResourcesJars() + if len(implementationJars) != 1 { + panic(fmt.Errorf("there must be only one implementation jar from %q", j.Name())) + } + return implementationJars[0] + } + // Register java implementation libraries for use only in module_exports (not sdk). android.RegisterSdkMemberType(&librarySdkMemberType{ android.SdkMemberTypeBase{ PropertyName: "java_libs", }, - func(_ android.SdkMemberContext, j *Library) android.Path { - implementationJars := j.ImplementationAndResourcesJars() - if len(implementationJars) != 1 { - panic(fmt.Errorf("there must be only one implementation jar from %q", j.Name())) - } - return implementationJars[0] - }, + exportImplementationClassesJar, sdkSnapshotFilePathForJar, copyEverythingToSnapshot, }) @@ -72,19 +75,11 @@ func init() { PropertyName: "java_boot_libs", SupportsSdk: true, }, - func(ctx android.SdkMemberContext, j *Library) android.Path { - // Java boot libs are only provided in the SDK to provide access to their dex implementation - // jar for use by dexpreopting and boot jars package check. They do not need to provide an - // actual implementation jar but the java_import will need a file that exists so just copy an - // empty file. Any attempt to use that file as a jar will cause a build error. - return ctx.SnapshotBuilder().EmptyFile() - }, - func(osPrefix, name string) string { - // Create a special name for the implementation jar to try and provide some useful information - // to a developer that attempts to compile against this. - // TODO(b/175714559): Provide a proper error message in Soong not ninja. - return filepath.Join(osPrefix, "java_boot_libs", "snapshot", "jars", "are", "invalid", name+jarFileSuffix) - }, + // Temporarily export implementation classes jar for java_boot_libs as it is required for the + // hiddenapi processing. + // TODO(b/179354495): Revert once hiddenapi processing has been modularized. + exportImplementationClassesJar, + sdkSnapshotFilePathForJar, onlyCopyJarToSnapshot, }) diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go index 488afd84e..17a6ca9b8 100644 --- a/sdk/java_sdk_test.go +++ b/sdk/java_sdk_test.go @@ -503,7 +503,7 @@ java_import { sdk_member_name: "myjavalib", visibility: ["//visibility:public"], apex_available: ["//apex_available:platform"], - jars: ["java_boot_libs/snapshot/jars/are/invalid/myjavalib.jar"], + jars: ["java/myjavalib.jar"], } java_import { @@ -511,7 +511,7 @@ java_import { prefer: false, visibility: ["//visibility:public"], apex_available: ["//apex_available:platform"], - jars: ["java_boot_libs/snapshot/jars/are/invalid/myjavalib.jar"], + jars: ["java/myjavalib.jar"], } module_exports_snapshot { @@ -519,10 +519,9 @@ module_exports_snapshot { visibility: ["//visibility:public"], java_boot_libs: ["myexports_myjavalib@current"], } - `), checkAllCopyRules(` -.intermediates/myexports/common_os/empty -> java_boot_libs/snapshot/jars/are/invalid/myjavalib.jar +.intermediates/myjavalib/android_common/withres/myjavalib.jar -> java/myjavalib.jar `), ) } diff --git a/sdk/update.go b/sdk/update.go index b5bc9f434..377aaae76 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -653,9 +653,6 @@ type snapshotBuilder struct { filesToZip android.Paths zipsToMerge android.Paths - // The path to an empty file. - emptyFile android.WritablePath - prebuiltModules map[string]*bpModule prebuiltOrder []*bpModule @@ -706,19 +703,6 @@ func (s *snapshotBuilder) UnzipToSnapshot(zipPath android.Path, destDir string) s.zipsToMerge = append(s.zipsToMerge, tmpZipPath) } -func (s *snapshotBuilder) EmptyFile() android.Path { - if s.emptyFile == nil { - ctx := s.ctx - s.emptyFile = android.PathForModuleOut(ctx, "empty") - s.ctx.Build(pctx, android.BuildParams{ - Rule: android.Touch, - Output: s.emptyFile, - }) - } - - return s.emptyFile -} - func (s *snapshotBuilder) AddPrebuiltModule(member android.SdkMember, moduleType string) android.BpModule { name := member.Name() if s.prebuiltModules[name] != nil {