From 2057f82161dec05cb23535da713ec0fae44c38d1 Mon Sep 17 00:00:00 2001 From: Sasha Smundak Date: Tue, 16 Apr 2019 17:16:58 -0700 Subject: [PATCH] Fix handling optimize.enabled from java_defaults Some module types (`android_test`, etc.) set `optimize.enabled` by default. If such module happens to have `defaults` attribute which clears `optimize.enabled`, the latter value is ignored. Fixes: 129858282 Test: unit tests in java_test.go, `atest CtsExtendedMockingTestCases` succeeds with aog/936802 reverted (that is, with cts/test/mocking converted to Android.bp) Change-Id: Ib8e3a0ab0bd489d70ed07f626082aeae31c45e7c --- java/app.go | 6 +++--- java/dex.go | 2 +- java/java.go | 8 +++++++- java/java_test.go | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/java/app.go b/java/app.go index ab623e24d..da8024fd2 100644 --- a/java/app.go +++ b/java/app.go @@ -438,7 +438,7 @@ func (a *AndroidApp) getCertString(ctx android.BaseContext) string { func AndroidAppFactory() android.Module { module := &AndroidApp{} - module.Module.deviceProperties.Optimize.Enabled = proptools.BoolPtr(true) + module.Module.deviceProperties.Optimize.EnabledByDefault = true module.Module.deviceProperties.Optimize.Shrink = proptools.BoolPtr(true) module.Module.properties.Instrument = true @@ -508,7 +508,7 @@ func (a *AndroidTest) DepsMutator(ctx android.BottomUpMutatorContext) { func AndroidTestFactory() android.Module { module := &AndroidTest{} - module.Module.deviceProperties.Optimize.Enabled = proptools.BoolPtr(true) + module.Module.deviceProperties.Optimize.EnabledByDefault = true module.Module.properties.Instrument = true module.Module.properties.Installable = proptools.BoolPtr(true) @@ -550,7 +550,7 @@ type AndroidTestHelperApp struct { func AndroidTestHelperAppFactory() android.Module { module := &AndroidTestHelperApp{} - module.Module.deviceProperties.Optimize.Enabled = proptools.BoolPtr(true) + module.Module.deviceProperties.Optimize.EnabledByDefault = true module.Module.properties.Installable = proptools.BoolPtr(true) module.appProperties.Use_embedded_native_libs = proptools.BoolPtr(true) diff --git a/java/dex.go b/java/dex.go index 987129ee7..c8a4fa8ed 100644 --- a/java/dex.go +++ b/java/dex.go @@ -171,7 +171,7 @@ func (j *Module) r8Flags(ctx android.ModuleContext, flags javaBuilderFlags) (r8F func (j *Module) compileDex(ctx android.ModuleContext, flags javaBuilderFlags, classesJar android.Path, jarName string) android.ModuleOutPath { - useR8 := Bool(j.deviceProperties.Optimize.Enabled) + useR8 := j.deviceProperties.EffectiveOptimizeEnabled() // Compile classes.jar into classes.dex and then javalib.jar javalibJar := android.PathForModuleOut(ctx, "dex", jarName) diff --git a/java/java.go b/java/java.go index 7768756ec..2cf9267fb 100644 --- a/java/java.go +++ b/java/java.go @@ -228,6 +228,8 @@ type CompilerDeviceProperties struct { // If false, disable all optimization. Defaults to true for android_app and android_test // modules, false for java_library and java_test modules. Enabled *bool + // True if the module containing this has it set by default. + EnabledByDefault bool `blueprint:"mutated"` // If true, optimize for size by removing unused code. Defaults to true for apps, // false for libraries and tests. @@ -257,6 +259,10 @@ type CompilerDeviceProperties struct { IsSDKLibrary bool `blueprint:"mutated"` } +func (me *CompilerDeviceProperties) EffectiveOptimizeEnabled() bool { + return BoolDefault(me.Optimize.Enabled, me.Optimize.EnabledByDefault) +} + // Module contains the properties and members used by all java module types type Module struct { android.ModuleBase @@ -460,7 +466,7 @@ func (j *Module) deps(ctx android.BottomUpMutatorContext) { } else if sdkDep.useModule { ctx.AddVariationDependencies(nil, systemModulesTag, sdkDep.systemModules) ctx.AddVariationDependencies(nil, bootClasspathTag, sdkDep.modules...) - if Bool(j.deviceProperties.Optimize.Enabled) { + if j.deviceProperties.EffectiveOptimizeEnabled() { ctx.AddVariationDependencies(nil, proguardRaiseTag, config.DefaultBootclasspathLibraries...) ctx.AddVariationDependencies(nil, proguardRaiseTag, config.DefaultLibraries...) } diff --git a/java/java_test.go b/java/java_test.go index 0a1c17c1c..9438ce297 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -377,6 +377,7 @@ func TestDefaults(t *testing.T) { srcs: ["a.java"], libs: ["bar"], static_libs: ["baz"], + optimize: {enabled: false}, } java_library { @@ -393,6 +394,22 @@ func TestDefaults(t *testing.T) { name: "baz", srcs: ["c.java"], } + + android_test { + name: "atestOptimize", + defaults: ["defaults"], + optimize: {enabled: true}, + } + + android_test { + name: "atestNoOptimize", + defaults: ["defaults"], + } + + android_test { + name: "atestDefault", + srcs: ["a.java"], + } `) javac := ctx.ModuleForTests("foo", "android_common").Rule("javac") @@ -411,6 +428,21 @@ func TestDefaults(t *testing.T) { if len(combineJar.Inputs) != 2 || combineJar.Inputs[1].String() != baz { t.Errorf("foo combineJar inputs %v does not contain %q", combineJar.Inputs, baz) } + + atestOptimize := ctx.ModuleForTests("atestOptimize", "android_common").MaybeRule("r8") + if atestOptimize.Output == nil { + t.Errorf("atestOptimize should optimize APK") + } + + atestNoOptimize := ctx.ModuleForTests("atestNoOptimize", "android_common").MaybeRule("d8") + if atestNoOptimize.Output == nil { + t.Errorf("atestNoOptimize should not optimize APK") + } + + atestDefault := ctx.ModuleForTests("atestDefault", "android_common").MaybeRule("r8") + if atestDefault.Output == nil { + t.Errorf("atestDefault should optimize APK") + } } func TestResources(t *testing.T) {