From 0727ba76b11f8387797a2d412c0b1c415d2d4468 Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 13 Feb 2024 16:37:43 +0000 Subject: [PATCH] Disable dexpreopt if optional_uses_libs does not have an impl At ToT, an optional_uses_libs is not added to the build time CLC if it does not exist in the tree. One edge case here is java_sdk_library_import, which might exist in the tree, but without an implementation. This cause issues during analysis when we try to verify the correctness of the build time CLC. This CL disables dexpreopt if a dependency does not have an implementation. To limit inadvertent side effects, this is restricted to java_sdk_library(_import) module types. (more precisely, it is restricted to java_sdk_library_import, since the source module type will always have an impl) Bug: 315802285 Test: Added a unit test Test: m nothing Test: printf debugging in internal main, verified that this CL does not disable dexpreopt on any android app inadvertently Change-Id: I173fc2f3ff654fe4091e9a43322164afd3222ee7 --- java/aar.go | 3 +++ java/app.go | 15 +++++++++++++++ java/app_import.go | 3 +++ java/app_test.go | 23 +++++++++++++++++++++++ java/dexpreopt.go | 13 +++++++++++++ java/java.go | 3 +++ 6 files changed, 60 insertions(+) diff --git a/java/aar.go b/java/aar.go index f61fc8374..a26b5e18c 100644 --- a/java/aar.go +++ b/java/aar.go @@ -808,6 +808,9 @@ func (a *AndroidLibrary) DepsMutator(ctx android.BottomUpMutatorContext) { func (a *AndroidLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) { a.aapt.isLibrary = true a.classLoaderContexts = a.usesLibrary.classLoaderContextForUsesLibDeps(ctx) + if a.usesLibrary.shouldDisableDexpreopt { + a.dexpreopter.disableDexpreopt() + } a.aapt.buildActions(ctx, aaptBuildActionOptions{ sdkContext: android.SdkContext(a), diff --git a/java/app.go b/java/app.go index 0c56d81fc..32c69a173 100755 --- a/java/app.go +++ b/java/app.go @@ -777,6 +777,9 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { a.onDeviceDir = android.InstallPathToOnDevicePath(ctx, a.installDir) a.classLoaderContexts = a.usesLibrary.classLoaderContextForUsesLibDeps(ctx) + if a.usesLibrary.shouldDisableDexpreopt { + a.dexpreopter.disableDexpreopt() + } var noticeAssetPath android.WritablePath if Bool(a.appProperties.Embed_notices) || ctx.Config().IsEnvTrue("ALWAYS_EMBED_NOTICES") { @@ -1547,6 +1550,9 @@ type usesLibrary struct { // Whether to enforce verify_uses_library check. enforce bool + + // Whether dexpreopt should be disabled + shouldDisableDexpreopt bool } func (u *usesLibrary) addLib(lib string, optional bool) { @@ -1628,6 +1634,15 @@ func (u *usesLibrary) classLoaderContextForUsesLibDeps(ctx android.ModuleContext } } + // Skip java_sdk_library dependencies that provide stubs, but not an implementation. + // This will be restricted to optional_uses_libs + if sdklib, ok := m.(SdkLibraryDependency); ok { + if tag == usesLibOptTag && sdklib.DexJarBuildPath(ctx).PathOrNil() == nil { + u.shouldDisableDexpreopt = true + return + } + } + if lib, ok := m.(UsesLibraryDependency); ok { libName := dep if ulib, ok := m.(ProvidesUsesLib); ok && ulib.ProvidesUsesLib() != nil { diff --git a/java/app_import.go b/java/app_import.go index 12ead0aa2..74255b787 100644 --- a/java/app_import.go +++ b/java/app_import.go @@ -319,6 +319,9 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext a.dexpreopter.enforceUsesLibs = a.usesLibrary.enforceUsesLibraries() a.dexpreopter.classLoaderContexts = a.usesLibrary.classLoaderContextForUsesLibDeps(ctx) + if a.usesLibrary.shouldDisableDexpreopt { + a.dexpreopter.disableDexpreopt() + } if a.usesLibrary.enforceUsesLibraries() { a.usesLibrary.verifyUsesLibrariesAPK(ctx, srcApk) diff --git a/java/app_test.go b/java/app_test.go index 362bef922..125c9716c 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -4378,3 +4378,26 @@ func TestAppFlagsPackages(t *testing.T) { "--feature-flags @out/soong/.intermediates/bar/intermediate.txt --feature-flags @out/soong/.intermediates/baz/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 := ` + java_sdk_library_import { + name: "sdklib_noimpl", + public: { + jars: ["stub.jar"], + }, + } + android_app { + name: "app", + srcs: ["a.java"], + sdk_version: "current", + optional_uses_libs: [ + "sdklib_noimpl", + ], + } + ` + result := prepareForJavaTest.RunTestWithBp(t, bp) + dexpreopt := result.ModuleForTests("app", "android_common").MaybeRule("dexpreopt").Rule + android.AssertBoolEquals(t, "dexpreopt should be disabled if optional_uses_libs does not have an implementation", true, dexpreopt == nil) +} diff --git a/java/dexpreopt.go b/java/dexpreopt.go index 9db9b1b46..1cfa64245 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -102,6 +102,11 @@ type dexpreopter struct { dexpreoptProperties DexpreoptProperties importDexpreoptProperties ImportDexpreoptProperties + // If true, the dexpreopt rules will not be generated + // Unlike Dex_preopt.Enabled which is user-facing, + // shouldDisableDexpreopt is a mutated propery. + shouldDisableDexpreopt bool + installPath android.InstallPath uncompressedDex bool isSDKLibrary bool @@ -197,6 +202,10 @@ func (d *dexpreopter) dexpreoptDisabled(ctx android.BaseModuleContext, libName s return true } + if d.shouldDisableDexpreopt { + return true + } + // If the module is from a prebuilt APEX, it shouldn't be installable, but it can still be // dexpreopted. if !ctx.Module().(DexpreopterInterface).IsInstallable() && !forPrebuiltApex(ctx) { @@ -528,3 +537,7 @@ func (d *dexpreopter) AndroidMkEntriesForApex() []android.AndroidMkEntries { func (d *dexpreopter) OutputProfilePathOnHost() android.Path { return d.outputProfilePathOnHost } + +func (d *dexpreopter) disableDexpreopt() { + d.shouldDisableDexpreopt = true +} diff --git a/java/java.go b/java/java.go index d7d271cca..576b73b68 100644 --- a/java/java.go +++ b/java/java.go @@ -719,6 +719,9 @@ func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) { setUncompressDex(ctx, &j.dexpreopter, &j.dexer) j.dexpreopter.uncompressedDex = *j.dexProperties.Uncompress_dex j.classLoaderContexts = j.usesLibrary.classLoaderContextForUsesLibDeps(ctx) + if j.usesLibrary.shouldDisableDexpreopt { + j.dexpreopter.disableDexpreopt() + } } j.compile(ctx, nil, nil, nil)