From 75a48d8ae23c017dadc7ac025b45f5a1aebfbe4c Mon Sep 17 00:00:00 2001
From: Martin Stjernholm <mast@google.com>
Date: Fri, 10 Jan 2020 20:32:59 +0000
Subject: [PATCH 1/5] Reland: Separate dexpreopt.GlobalSoongConfig to allow
 independent caching of it.

Introduce a Once cache for GlobalSoongConfig to allow it to get binary
tool paths from ordinary module dependencies (coming in a future CL)
that are then reused in singletons.

This relands https://r.android.com/1205729.

Bug: 145934348
Test: m
Change-Id: I039d6e204bee5ddc16d8e2d85057fbec20e326fe
---
 dexpreopt/config.go                      | 65 ++++++++++++++++--------
 dexpreopt/dexpreopt.go                   | 35 ++++++-------
 dexpreopt/dexpreopt_gen/dexpreopt_gen.go | 18 +++----
 dexpreopt/dexpreopt_test.go              | 24 ++++++---
 java/dexpreopt.go                        |  3 +-
 java/dexpreopt_bootjars.go               |  9 ++--
 java/dexpreopt_config.go                 |  3 +-
 java/java_test.go                        |  8 ++-
 8 files changed, 104 insertions(+), 61 deletions(-)

diff --git a/dexpreopt/config.go b/dexpreopt/config.go
index e35387841..2ba6bb484 100644
--- a/dexpreopt/config.go
+++ b/dexpreopt/config.go
@@ -22,8 +22,7 @@ import (
 )
 
 // GlobalConfig stores the configuration for dex preopting. The fields are set
-// from product variables via dex_preopt_config.mk, except for SoongConfig
-// which come from CreateGlobalSoongConfig.
+// from product variables via dex_preopt_config.mk.
 type GlobalConfig struct {
 	DisablePreopt        bool     // disable preopt for all modules
 	DisablePreoptModules []string // modules with preopt disabled by product-specific config
@@ -81,8 +80,6 @@ type GlobalConfig struct {
 	BootFlags         string               // extra flags to pass to dex2oat for the boot image
 	Dex2oatImageXmx   string               // max heap size for dex2oat for the boot image
 	Dex2oatImageXms   string               // initial heap size for dex2oat for the boot image
-
-	SoongConfig GlobalSoongConfig // settings read from dexpreopt_soong.config
 }
 
 // GlobalSoongConfig contains the global config that is generated from Soong,
@@ -179,11 +176,9 @@ func constructWritablePath(ctx android.PathContext, path string) android.Writabl
 }
 
 // LoadGlobalConfig reads the global dexpreopt.config file into a GlobalConfig
-// struct, except the SoongConfig field which is set from the provided
-// soongConfig argument. LoadGlobalConfig is used directly in Soong and in
-// dexpreopt_gen called from Make to read the $OUT/dexpreopt.config written by
-// Make.
-func LoadGlobalConfig(ctx android.PathContext, data []byte, soongConfig GlobalSoongConfig) (GlobalConfig, error) {
+// struct. LoadGlobalConfig is used directly in Soong and in dexpreopt_gen
+// called from Make to read the $OUT/dexpreopt.config written by Make.
+func LoadGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, error) {
 	type GlobalJSONConfig struct {
 		GlobalConfig
 
@@ -203,10 +198,6 @@ func LoadGlobalConfig(ctx android.PathContext, data []byte, soongConfig GlobalSo
 	config.GlobalConfig.DirtyImageObjects = android.OptionalPathForPath(constructPath(ctx, config.DirtyImageObjects))
 	config.GlobalConfig.BootImageProfiles = constructPaths(ctx, config.BootImageProfiles)
 
-	// Set this here to force the caller to provide a value for this struct (from
-	// either CreateGlobalSoongConfig or LoadGlobalSoongConfig).
-	config.GlobalConfig.SoongConfig = soongConfig
-
 	return config.GlobalConfig, nil
 }
 
@@ -252,9 +243,16 @@ func LoadModuleConfig(ctx android.PathContext, data []byte) (ModuleConfig, error
 	return config.ModuleConfig, nil
 }
 
-// CreateGlobalSoongConfig creates a GlobalSoongConfig from the current context.
+// createGlobalSoongConfig creates a GlobalSoongConfig from the current context.
 // Should not be used in dexpreopt_gen.
-func CreateGlobalSoongConfig(ctx android.PathContext) GlobalSoongConfig {
+func createGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
+	if ctx.Config().TestProductVariables != nil {
+		// If we're called in a test there'll be a confusing error from the path
+		// functions below that gets reported without a stack trace, so let's panic
+		// properly with a more helpful message.
+		panic("This should not be called from tests. Please call GlobalSoongConfigForTests somewhere in the test setup.")
+	}
+
 	// Default to debug version to help find bugs.
 	// Set USE_DEX2OAT_DEBUG to false for only building non-debug versions.
 	var dex2oatBinary string
@@ -275,6 +273,26 @@ func CreateGlobalSoongConfig(ctx android.PathContext) GlobalSoongConfig {
 	}
 }
 
+var globalSoongConfigOnceKey = android.NewOnceKey("DexpreoptGlobalSoongConfig")
+
+// GetGlobalSoongConfig creates a GlobalSoongConfig the first time it's called,
+// and later returns the same cached instance.
+func GetGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
+	globalSoong := ctx.Config().Once(globalSoongConfigOnceKey, func() interface{} {
+		return createGlobalSoongConfig(ctx)
+	}).(GlobalSoongConfig)
+	return globalSoong
+}
+
+// GetCachedGlobalSoongConfig returns a cached GlobalSoongConfig created by an
+// earlier GetGlobalSoongConfig call. This function works with any context
+// compatible with a basic PathContext, since it doesn't try to create a
+// GlobalSoongConfig (which requires a full ModuleContext). It will panic if
+// called before the first GetGlobalSoongConfig call.
+func GetCachedGlobalSoongConfig(ctx android.PathContext) GlobalSoongConfig {
+	return ctx.Config().Get(globalSoongConfigOnceKey).(GlobalSoongConfig)
+}
+
 type globalJsonSoongConfig struct {
 	Profman          string
 	Dex2oat          string
@@ -309,7 +327,7 @@ func LoadGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongCon
 }
 
 func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonContext) {
-	config := CreateGlobalSoongConfig(ctx)
+	config := GetCachedGlobalSoongConfig(ctx)
 	jc := globalJsonSoongConfig{
 		Profman:          config.Profman.String(),
 		Dex2oat:          config.Dex2oat.String(),
@@ -336,7 +354,7 @@ func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonC
 }
 
 func (s *globalSoongConfigSingleton) MakeVars(ctx android.MakeVarsContext) {
-	config := CreateGlobalSoongConfig(ctx)
+	config := GetCachedGlobalSoongConfig(ctx)
 
 	ctx.Strict("DEX2OAT", config.Dex2oat.String())
 	ctx.Strict("DEXPREOPT_GEN_DEPS", strings.Join([]string{
@@ -389,7 +407,14 @@ func GlobalConfigForTests(ctx android.PathContext) GlobalConfig {
 		BootFlags:                          "",
 		Dex2oatImageXmx:                    "",
 		Dex2oatImageXms:                    "",
-		SoongConfig: GlobalSoongConfig{
+	}
+}
+
+func GlobalSoongConfigForTests(config android.Config) GlobalSoongConfig {
+	// Install the test GlobalSoongConfig in the Once cache so that later calls to
+	// Get(Cached)GlobalSoongConfig returns it without trying to create a real one.
+	return config.Once(globalSoongConfigOnceKey, func() interface{} {
+		return GlobalSoongConfig{
 			Profman:          android.PathForTesting("profman"),
 			Dex2oat:          android.PathForTesting("dex2oat"),
 			Aapt:             android.PathForTesting("aapt"),
@@ -397,6 +422,6 @@ func GlobalConfigForTests(ctx android.PathContext) GlobalConfig {
 			Zip2zip:          android.PathForTesting("zip2zip"),
 			ManifestCheck:    android.PathForTesting("manifest_check"),
 			ConstructContext: android.PathForTesting("construct_context.sh"),
-		},
-	}
+		}
+	}).(GlobalSoongConfig)
 }
diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go
index 4d8ccb5fb..dbf08f2c1 100644
--- a/dexpreopt/dexpreopt.go
+++ b/dexpreopt/dexpreopt.go
@@ -58,7 +58,7 @@ var SystemServerForcedDepTag = dependencyTag{name: "system-server-forced-dep"}
 
 // GenerateDexpreoptRule generates a set of commands that will preopt a module based on a GlobalConfig and a
 // ModuleConfig.  The produced files and their install locations will be available through rule.Installs().
-func GenerateDexpreoptRule(ctx android.PathContext,
+func GenerateDexpreoptRule(ctx android.PathContext, globalSoong GlobalSoongConfig,
 	global GlobalConfig, module ModuleConfig) (rule *android.RuleBuilder, err error) {
 
 	defer func() {
@@ -81,10 +81,10 @@ func GenerateDexpreoptRule(ctx android.PathContext,
 
 	var profile android.WritablePath
 	if generateProfile {
-		profile = profileCommand(ctx, global, module, rule)
+		profile = profileCommand(ctx, globalSoong, global, module, rule)
 	}
 	if generateBootProfile {
-		bootProfileCommand(ctx, global, module, rule)
+		bootProfileCommand(ctx, globalSoong, global, module, rule)
 	}
 
 	if !dexpreoptDisabled(ctx, global, module) {
@@ -96,7 +96,7 @@ func GenerateDexpreoptRule(ctx android.PathContext,
 			generateDM := shouldGenerateDM(module, global)
 
 			for archIdx, _ := range module.Archs {
-				dexpreoptCommand(ctx, global, module, rule, archIdx, profile, appImage, generateDM)
+				dexpreoptCommand(ctx, globalSoong, global, module, rule, archIdx, profile, appImage, generateDM)
 			}
 		}
 	}
@@ -135,8 +135,8 @@ func dexpreoptDisabled(ctx android.PathContext, global GlobalConfig, module Modu
 	return false
 }
 
-func profileCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig,
-	rule *android.RuleBuilder) android.WritablePath {
+func profileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
+	module ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
 
 	profilePath := module.BuildPath.InSameDir(ctx, "profile.prof")
 	profileInstalledPath := module.DexLocation + ".prof"
@@ -147,7 +147,7 @@ func profileCommand(ctx android.PathContext, global GlobalConfig, module ModuleC
 
 	cmd := rule.Command().
 		Text(`ANDROID_LOG_TAGS="*:e"`).
-		Tool(global.SoongConfig.Profman)
+		Tool(globalSoong.Profman)
 
 	if module.ProfileIsTextListing {
 		// The profile is a test listing of classes (used for framework jars).
@@ -174,8 +174,8 @@ func profileCommand(ctx android.PathContext, global GlobalConfig, module ModuleC
 	return profilePath
 }
 
-func bootProfileCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig,
-	rule *android.RuleBuilder) android.WritablePath {
+func bootProfileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
+	module ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
 
 	profilePath := module.BuildPath.InSameDir(ctx, "profile.bprof")
 	profileInstalledPath := module.DexLocation + ".bprof"
@@ -186,7 +186,7 @@ func bootProfileCommand(ctx android.PathContext, global GlobalConfig, module Mod
 
 	cmd := rule.Command().
 		Text(`ANDROID_LOG_TAGS="*:e"`).
-		Tool(global.SoongConfig.Profman)
+		Tool(globalSoong.Profman)
 
 	// The profile is a test listing of methods.
 	// We need to generate the actual binary profile.
@@ -206,8 +206,9 @@ func bootProfileCommand(ctx android.PathContext, global GlobalConfig, module Mod
 	return profilePath
 }
 
-func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig, rule *android.RuleBuilder,
-	archIdx int, profile android.WritablePath, appImage bool, generateDM bool) {
+func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
+	module ModuleConfig, rule *android.RuleBuilder, archIdx int, profile android.WritablePath,
+	appImage bool, generateDM bool) {
 
 	arch := module.Archs[archIdx]
 
@@ -345,14 +346,14 @@ func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module Modul
 	if module.EnforceUsesLibraries {
 		if module.ManifestPath != nil {
 			rule.Command().Text(`target_sdk_version="$(`).
-				Tool(global.SoongConfig.ManifestCheck).
+				Tool(globalSoong.ManifestCheck).
 				Flag("--extract-target-sdk-version").
 				Input(module.ManifestPath).
 				Text(`)"`)
 		} else {
 			// No manifest to extract targetSdkVersion from, hope that DexJar is an APK
 			rule.Command().Text(`target_sdk_version="$(`).
-				Tool(global.SoongConfig.Aapt).
+				Tool(globalSoong.Aapt).
 				Flag("dump badging").
 				Input(module.DexPath).
 				Text(`| grep "targetSdkVersion" | sed -n "s/targetSdkVersion:'\(.*\)'/\1/p"`).
@@ -373,7 +374,7 @@ func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module Modul
 			Implicits(conditionalClassLoaderContextHost29)
 		rule.Command().Textf(`conditional_target_libs_29="%s"`,
 			strings.Join(conditionalClassLoaderContextTarget29, " "))
-		rule.Command().Text("source").Tool(global.SoongConfig.ConstructContext).Input(module.DexPath)
+		rule.Command().Text("source").Tool(globalSoong.ConstructContext).Input(module.DexPath)
 	}
 
 	// Devices that do not have a product partition use a symlink from /product to /system/product.
@@ -386,7 +387,7 @@ func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module Modul
 
 	cmd := rule.Command().
 		Text(`ANDROID_LOG_TAGS="*:e"`).
-		Tool(global.SoongConfig.Dex2oat).
+		Tool(globalSoong.Dex2oat).
 		Flag("--avoid-storing-invocation").
 		FlagWithOutput("--write-invocation-to=", invocationPath).ImplicitOutput(invocationPath).
 		Flag("--runtime-arg").FlagWithArg("-Xms", global.Dex2oatXms).
@@ -455,7 +456,7 @@ func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module Modul
 		dmInstalledPath := pathtools.ReplaceExtension(module.DexLocation, "dm")
 		tmpPath := module.BuildPath.InSameDir(ctx, "primary.vdex")
 		rule.Command().Text("cp -f").Input(vdexPath).Output(tmpPath)
-		rule.Command().Tool(global.SoongConfig.SoongZip).
+		rule.Command().Tool(globalSoong.SoongZip).
 			FlagWithArg("-L", "9").
 			FlagWithOutput("-o", dmPath).
 			Flag("-j").
diff --git a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
index e2818bb61..708bb9b23 100644
--- a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
+++ b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
@@ -80,13 +80,13 @@ func main() {
 
 	globalSoongConfigData, err := ioutil.ReadFile(*globalSoongConfigPath)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error reading global config %q: %s\n", *globalSoongConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error reading global Soong config %q: %s\n", *globalSoongConfigPath, err)
 		os.Exit(2)
 	}
 
 	globalSoongConfig, err := dexpreopt.LoadGlobalSoongConfig(ctx, globalSoongConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error loading global config %q: %s\n", *globalSoongConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error loading global Soong config %q: %s\n", *globalSoongConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -96,9 +96,9 @@ func main() {
 		os.Exit(2)
 	}
 
-	globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, globalConfigData, globalSoongConfig)
+	globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, globalConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error parse global config %q: %s\n", *globalConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error loading global config %q: %s\n", *globalConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -130,12 +130,12 @@ func main() {
 		}
 	}()
 
-	writeScripts(ctx, globalConfig, moduleConfig, *dexpreoptScriptPath)
+	writeScripts(ctx, globalSoongConfig, globalConfig, moduleConfig, *dexpreoptScriptPath)
 }
 
-func writeScripts(ctx android.PathContext, global dexpreopt.GlobalConfig, module dexpreopt.ModuleConfig,
-	dexpreoptScriptPath string) {
-	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, global, module)
+func writeScripts(ctx android.PathContext, globalSoong dexpreopt.GlobalSoongConfig,
+	global dexpreopt.GlobalConfig, module dexpreopt.ModuleConfig, dexpreoptScriptPath string) {
+	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, globalSoong, global, module)
 	if err != nil {
 		panic(err)
 	}
@@ -150,7 +150,7 @@ func writeScripts(ctx android.PathContext, global dexpreopt.GlobalConfig, module
 		dexpreoptRule.Command().Text("mkdir -p").Flag(filepath.Dir(installPath.String()))
 		dexpreoptRule.Command().Text("cp -f").Input(install.From).Output(installPath)
 	}
-	dexpreoptRule.Command().Tool(global.SoongConfig.SoongZip).
+	dexpreoptRule.Command().Tool(globalSoong.SoongZip).
 		FlagWithArg("-o ", "$2").
 		FlagWithArg("-C ", installDir.String()).
 		FlagWithArg("-D ", installDir.String())
diff --git a/dexpreopt/dexpreopt_test.go b/dexpreopt/dexpreopt_test.go
index a128dc009..44bbbc247 100644
--- a/dexpreopt/dexpreopt_test.go
+++ b/dexpreopt/dexpreopt_test.go
@@ -61,10 +61,13 @@ func testModuleConfig(ctx android.PathContext, name, partition string) ModuleCon
 }
 
 func TestDexPreopt(t *testing.T) {
-	ctx := android.PathContextForTesting(android.TestConfig("out", nil, "", nil))
-	global, module := GlobalConfigForTests(ctx), testSystemModuleConfig(ctx, "test")
+	config := android.TestConfig("out", nil, "", nil)
+	ctx := android.PathContextForTesting(config)
+	globalSoong := GlobalSoongConfigForTests(config)
+	global := GlobalConfigForTests(ctx)
+	module := testSystemModuleConfig(ctx, "test")
 
-	rule, err := GenerateDexpreoptRule(ctx, global, module)
+	rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -80,7 +83,9 @@ func TestDexPreopt(t *testing.T) {
 }
 
 func TestDexPreoptSystemOther(t *testing.T) {
-	ctx := android.PathContextForTesting(android.TestConfig("out", nil, "", nil))
+	config := android.TestConfig("out", nil, "", nil)
+	ctx := android.PathContextForTesting(config)
+	globalSoong := GlobalSoongConfigForTests(config)
 	global := GlobalConfigForTests(ctx)
 	systemModule := testSystemModuleConfig(ctx, "Stest")
 	systemProductModule := testSystemProductModuleConfig(ctx, "SPtest")
@@ -118,7 +123,7 @@ func TestDexPreoptSystemOther(t *testing.T) {
 	for _, test := range tests {
 		global.PatternsOnSystemOther = test.patterns
 		for _, mt := range test.moduleTests {
-			rule, err := GenerateDexpreoptRule(ctx, global, mt.module)
+			rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, mt.module)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -138,12 +143,15 @@ func TestDexPreoptSystemOther(t *testing.T) {
 }
 
 func TestDexPreoptProfile(t *testing.T) {
-	ctx := android.PathContextForTesting(android.TestConfig("out", nil, "", nil))
-	global, module := GlobalConfigForTests(ctx), testSystemModuleConfig(ctx, "test")
+	config := android.TestConfig("out", nil, "", nil)
+	ctx := android.PathContextForTesting(config)
+	globalSoong := GlobalSoongConfigForTests(config)
+	global := GlobalConfigForTests(ctx)
+	module := testSystemModuleConfig(ctx, "test")
 
 	module.ProfileClassListing = android.OptionalPathForPath(android.PathForTesting("profile"))
 
-	rule, err := GenerateDexpreoptRule(ctx, global, module)
+	rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module)
 	if err != nil {
 		t.Fatal(err)
 	}
diff --git a/java/dexpreopt.go b/java/dexpreopt.go
index c81e199c6..ec0b5c659 100644
--- a/java/dexpreopt.go
+++ b/java/dexpreopt.go
@@ -104,6 +104,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 		return dexJarFile
 	}
 
+	globalSoong := dexpreopt.GetGlobalSoongConfig(ctx)
 	global := dexpreoptGlobalConfig(ctx)
 	bootImage := defaultBootImageConfig(ctx)
 	dexFiles := bootImage.dexPathsDeps.Paths()
@@ -191,7 +192,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 		PresignedPrebuilt: d.isPresignedPrebuilt,
 	}
 
-	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, global, dexpreoptConfig)
+	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, globalSoong, global, dexpreoptConfig)
 	if err != nil {
 		ctx.ModuleErrorf("error generating dexpreopt rule: %s", err.Error())
 		return dexJarFile
diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go
index 607a43781..25da25860 100644
--- a/java/dexpreopt_bootjars.go
+++ b/java/dexpreopt_bootjars.go
@@ -295,6 +295,7 @@ func buildBootImage(ctx android.SingletonContext, config bootImageConfig) *bootI
 func buildBootImageRuleForArch(ctx android.SingletonContext, image *bootImage,
 	arch android.ArchType, profile android.Path, missingDeps []string) android.WritablePaths {
 
+	globalSoong := dexpreopt.GetCachedGlobalSoongConfig(ctx)
 	global := dexpreoptGlobalConfig(ctx)
 
 	symbolsDir := image.symbolsDir.Join(ctx, image.installSubdir, arch.String())
@@ -330,7 +331,7 @@ func buildBootImageRuleForArch(ctx android.SingletonContext, image *bootImage,
 
 	invocationPath := outputPath.ReplaceExtension(ctx, "invocation")
 
-	cmd.Tool(global.SoongConfig.Dex2oat).
+	cmd.Tool(globalSoong.Dex2oat).
 		Flag("--avoid-storing-invocation").
 		FlagWithOutput("--write-invocation-to=", invocationPath).ImplicitOutput(invocationPath).
 		Flag("--runtime-arg").FlagWithArg("-Xms", global.Dex2oatImageXms).
@@ -433,6 +434,7 @@ It is likely that the boot classpath is inconsistent.
 Rebuild with ART_BOOT_IMAGE_EXTRA_ARGS="--runtime-arg -verbose:verifier" to see verification errors.`
 
 func bootImageProfileRule(ctx android.SingletonContext, image *bootImage, missingDeps []string) android.WritablePath {
+	globalSoong := dexpreopt.GetCachedGlobalSoongConfig(ctx)
 	global := dexpreoptGlobalConfig(ctx)
 
 	if global.DisableGenerateProfile || ctx.Config().IsPdkBuild() || ctx.Config().UnbundledBuild() {
@@ -464,7 +466,7 @@ func bootImageProfileRule(ctx android.SingletonContext, image *bootImage, missin
 
 		rule.Command().
 			Text(`ANDROID_LOG_TAGS="*:e"`).
-			Tool(global.SoongConfig.Profman).
+			Tool(globalSoong.Profman).
 			FlagWithInput("--create-profile-from=", bootImageProfile).
 			FlagForEachInput("--apk=", image.dexPathsDeps.Paths()).
 			FlagForEachArg("--dex-location=", image.dexLocationsDeps).
@@ -487,6 +489,7 @@ func bootImageProfileRule(ctx android.SingletonContext, image *bootImage, missin
 var bootImageProfileRuleKey = android.NewOnceKey("bootImageProfileRule")
 
 func bootFrameworkProfileRule(ctx android.SingletonContext, image *bootImage, missingDeps []string) android.WritablePath {
+	globalSoong := dexpreopt.GetCachedGlobalSoongConfig(ctx)
 	global := dexpreoptGlobalConfig(ctx)
 
 	if global.DisableGenerateProfile || ctx.Config().IsPdkBuild() || ctx.Config().UnbundledBuild() {
@@ -513,7 +516,7 @@ func bootFrameworkProfileRule(ctx android.SingletonContext, image *bootImage, mi
 
 		rule.Command().
 			Text(`ANDROID_LOG_TAGS="*:e"`).
-			Tool(global.SoongConfig.Profman).
+			Tool(globalSoong.Profman).
 			Flag("--generate-boot-profile").
 			FlagWithInput("--create-profile-from=", bootFrameworkProfile).
 			FlagForEachInput("--apk=", image.dexPathsDeps.Paths()).
diff --git a/java/dexpreopt_config.go b/java/dexpreopt_config.go
index 7d0bd8fb7..0aa9005f6 100644
--- a/java/dexpreopt_config.go
+++ b/java/dexpreopt_config.go
@@ -39,8 +39,7 @@ func dexpreoptGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw {
 		if data, err := ctx.Config().DexpreoptGlobalConfig(ctx); err != nil {
 			panic(err)
 		} else if data != nil {
-			soongConfig := dexpreopt.CreateGlobalSoongConfig(ctx)
-			globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, data, soongConfig)
+			globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, data)
 			if err != nil {
 				panic(err)
 			}
diff --git a/java/java_test.go b/java/java_test.go
index 17921ca44..9c5680b0c 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -57,7 +57,13 @@ func TestMain(m *testing.M) {
 }
 
 func testConfig(env map[string]string, bp string, fs map[string][]byte) android.Config {
-	return TestConfig(buildDir, env, bp, fs)
+	config := TestConfig(buildDir, env, bp, fs)
+
+	// Set up the global Once cache used for dexpreopt.GlobalSoongConfig, so that
+	// it doesn't create a real one, which would fail.
+	_ = dexpreopt.GlobalSoongConfigForTests(config)
+
+	return config
 }
 
 func testContext() *android.TestContext {

From 40f9f3c061e30af857a54f35a5dd670d75e1c87d Mon Sep 17 00:00:00 2001
From: Martin Stjernholm <mast@google.com>
Date: Mon, 20 Jan 2020 18:12:23 +0000
Subject: [PATCH 2/5] Reland: Move the Once cache for dexpreopt.GlobalConfig
 into the dexpreopt package.

Preparation for a future CL that will need to get the make-written
GlobalConfig from within dexpreopt.

Also rename the Load*Config functions to Parse*Config, since they don't
actually load the config files anymore.

This relands https://r.android.com/1211982.

Bug: 145934348
Test: m
Change-Id: Icb9332a93811d77d2d8b06e983b92501b180a358
---
 dexpreopt/config.go                      | 77 ++++++++++++++++++++----
 dexpreopt/dexpreopt_gen/dexpreopt_gen.go | 12 ++--
 java/dexpreopt.go                        |  6 +-
 java/dexpreopt_bootjars.go               | 12 ++--
 java/dexpreopt_bootjars_test.go          |  2 +-
 java/dexpreopt_config.go                 | 50 +--------------
 java/java.go                             |  2 +-
 java/java_test.go                        |  4 +-
 8 files changed, 88 insertions(+), 77 deletions(-)

diff --git a/dexpreopt/config.go b/dexpreopt/config.go
index 2ba6bb484..7a404f5a6 100644
--- a/dexpreopt/config.go
+++ b/dexpreopt/config.go
@@ -175,10 +175,9 @@ func constructWritablePath(ctx android.PathContext, path string) android.Writabl
 	return constructPath(ctx, path).(android.WritablePath)
 }
 
-// LoadGlobalConfig reads the global dexpreopt.config file into a GlobalConfig
-// struct. LoadGlobalConfig is used directly in Soong and in dexpreopt_gen
-// called from Make to read the $OUT/dexpreopt.config written by Make.
-func LoadGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, error) {
+// ParseGlobalConfig parses the given data assumed to be read from the global
+// dexpreopt.config file into a GlobalConfig struct.
+func ParseGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, error) {
 	type GlobalJSONConfig struct {
 		GlobalConfig
 
@@ -201,10 +200,65 @@ func LoadGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, error
 	return config.GlobalConfig, nil
 }
 
-// LoadModuleConfig reads a per-module dexpreopt.config file into a ModuleConfig struct.  It is not used in Soong, which
-// receives a ModuleConfig struct directly from java/dexpreopt.go.  It is used in dexpreopt_gen called from oMake to
-// read the module dexpreopt.config written by Make.
-func LoadModuleConfig(ctx android.PathContext, data []byte) (ModuleConfig, error) {
+type globalConfigAndRaw struct {
+	global GlobalConfig
+	data   []byte
+}
+
+// GetGlobalConfig returns the global dexpreopt.config that's created in the
+// make config phase. It is loaded once the first time it is called for any
+// ctx.Config(), and returns the same data for all future calls with the same
+// ctx.Config(). A value can be inserted for tests using
+// setDexpreoptTestGlobalConfig.
+func GetGlobalConfig(ctx android.PathContext) GlobalConfig {
+	return getGlobalConfigRaw(ctx).global
+}
+
+// GetGlobalConfigRawData is the same as GetGlobalConfig, except that it returns
+// the literal content of dexpreopt.config.
+func GetGlobalConfigRawData(ctx android.PathContext) []byte {
+	return getGlobalConfigRaw(ctx).data
+}
+
+var globalConfigOnceKey = android.NewOnceKey("DexpreoptGlobalConfig")
+var testGlobalConfigOnceKey = android.NewOnceKey("TestDexpreoptGlobalConfig")
+
+func getGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw {
+	return ctx.Config().Once(globalConfigOnceKey, func() interface{} {
+		if data, err := ctx.Config().DexpreoptGlobalConfig(ctx); err != nil {
+			panic(err)
+		} else if data != nil {
+			globalConfig, err := ParseGlobalConfig(ctx, data)
+			if err != nil {
+				panic(err)
+			}
+			return globalConfigAndRaw{globalConfig, data}
+		}
+
+		// No global config filename set, see if there is a test config set
+		return ctx.Config().Once(testGlobalConfigOnceKey, func() interface{} {
+			// Nope, return a config with preopting disabled
+			return globalConfigAndRaw{GlobalConfig{
+				DisablePreopt:          true,
+				DisableGenerateProfile: true,
+			}, nil}
+		})
+	}).(globalConfigAndRaw)
+}
+
+// SetTestGlobalConfig sets a GlobalConfig that future calls to GetGlobalConfig
+// will return. It must be called before the first call to GetGlobalConfig for
+// the config.
+func SetTestGlobalConfig(config android.Config, globalConfig GlobalConfig) {
+	config.Once(testGlobalConfigOnceKey, func() interface{} { return globalConfigAndRaw{globalConfig, nil} })
+}
+
+// ParseModuleConfig parses a per-module dexpreopt.config file into a
+// ModuleConfig struct. It is not used in Soong, which receives a ModuleConfig
+// struct directly from java/dexpreopt.go. It is used in dexpreopt_gen called
+// from Make to read the module dexpreopt.config written in the Make config
+// stage.
+func ParseModuleConfig(ctx android.PathContext, data []byte) (ModuleConfig, error) {
 	type ModuleJSONConfig struct {
 		ModuleConfig
 
@@ -303,9 +357,10 @@ type globalJsonSoongConfig struct {
 	ConstructContext string
 }
 
-// LoadGlobalSoongConfig reads the dexpreopt_soong.config file into a
-// GlobalSoongConfig struct. It is only used in dexpreopt_gen.
-func LoadGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongConfig, error) {
+// ParseGlobalSoongConfig parses the given data assumed to be read from the
+// global dexpreopt_soong.config file into a GlobalSoongConfig struct. It is
+// only used in dexpreopt_gen.
+func ParseGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongConfig, error) {
 	var jc globalJsonSoongConfig
 
 	err := json.Unmarshal(data, &jc)
diff --git a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
index 708bb9b23..4da003e11 100644
--- a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
+++ b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
@@ -84,9 +84,9 @@ func main() {
 		os.Exit(2)
 	}
 
-	globalSoongConfig, err := dexpreopt.LoadGlobalSoongConfig(ctx, globalSoongConfigData)
+	globalSoongConfig, err := dexpreopt.ParseGlobalSoongConfig(ctx, globalSoongConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error loading global Soong config %q: %s\n", *globalSoongConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error parsing global Soong config %q: %s\n", *globalSoongConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -96,9 +96,9 @@ func main() {
 		os.Exit(2)
 	}
 
-	globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, globalConfigData)
+	globalConfig, err := dexpreopt.ParseGlobalConfig(ctx, globalConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error loading global config %q: %s\n", *globalConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error parsing global config %q: %s\n", *globalConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -108,9 +108,9 @@ func main() {
 		os.Exit(2)
 	}
 
-	moduleConfig, err := dexpreopt.LoadModuleConfig(ctx, moduleConfigData)
+	moduleConfig, err := dexpreopt.ParseModuleConfig(ctx, moduleConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error loading module config %q: %s\n", *moduleConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error parsing module config %q: %s\n", *moduleConfigPath, err)
 		os.Exit(2)
 	}
 
diff --git a/java/dexpreopt.go b/java/dexpreopt.go
index ec0b5c659..428aee2aa 100644
--- a/java/dexpreopt.go
+++ b/java/dexpreopt.go
@@ -59,7 +59,7 @@ type DexpreoptProperties struct {
 }
 
 func (d *dexpreopter) dexpreoptDisabled(ctx android.ModuleContext) bool {
-	global := dexpreoptGlobalConfig(ctx)
+	global := dexpreopt.GetGlobalConfig(ctx)
 
 	if global.DisablePreopt {
 		return true
@@ -96,7 +96,7 @@ func (d *dexpreopter) dexpreoptDisabled(ctx android.ModuleContext) bool {
 }
 
 func odexOnSystemOther(ctx android.ModuleContext, installPath android.InstallPath) bool {
-	return dexpreopt.OdexOnSystemOtherByName(ctx.ModuleName(), android.InstallPathToOnDevicePath(ctx, installPath), dexpreoptGlobalConfig(ctx))
+	return dexpreopt.OdexOnSystemOtherByName(ctx.ModuleName(), android.InstallPathToOnDevicePath(ctx, installPath), dexpreopt.GetGlobalConfig(ctx))
 }
 
 func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.ModuleOutPath) android.ModuleOutPath {
@@ -105,7 +105,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 	}
 
 	globalSoong := dexpreopt.GetGlobalSoongConfig(ctx)
-	global := dexpreoptGlobalConfig(ctx)
+	global := dexpreopt.GetGlobalConfig(ctx)
 	bootImage := defaultBootImageConfig(ctx)
 	dexFiles := bootImage.dexPathsDeps.Paths()
 	dexLocations := bootImage.dexLocationsDeps
diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go
index 25da25860..2a2e6dae5 100644
--- a/java/dexpreopt_bootjars.go
+++ b/java/dexpreopt_bootjars.go
@@ -165,7 +165,7 @@ func dexpreoptBootJarsFactory() android.Singleton {
 }
 
 func skipDexpreoptBootJars(ctx android.PathContext) bool {
-	if dexpreoptGlobalConfig(ctx).DisablePreopt {
+	if dexpreopt.GetGlobalConfig(ctx).DisablePreopt {
 		return true
 	}
 
@@ -209,7 +209,7 @@ func (d *dexpreoptBootJars) GenerateBuildActions(ctx android.SingletonContext) {
 	d.dexpreoptConfigForMake = android.PathForOutput(ctx, ctx.Config().DeviceName(), "dexpreopt.config")
 	writeGlobalConfigForMake(ctx, d.dexpreoptConfigForMake)
 
-	global := dexpreoptGlobalConfig(ctx)
+	global := dexpreopt.GetGlobalConfig(ctx)
 
 	// Skip recompiling the boot image for the second sanitization phase. We'll get separate paths
 	// and invalidate first-stage artifacts which are crucial to SANITIZE_LITE builds.
@@ -296,7 +296,7 @@ func buildBootImageRuleForArch(ctx android.SingletonContext, image *bootImage,
 	arch android.ArchType, profile android.Path, missingDeps []string) android.WritablePaths {
 
 	globalSoong := dexpreopt.GetCachedGlobalSoongConfig(ctx)
-	global := dexpreoptGlobalConfig(ctx)
+	global := dexpreopt.GetGlobalConfig(ctx)
 
 	symbolsDir := image.symbolsDir.Join(ctx, image.installSubdir, arch.String())
 	symbolsFile := symbolsDir.Join(ctx, image.stem+".oat")
@@ -435,7 +435,7 @@ Rebuild with ART_BOOT_IMAGE_EXTRA_ARGS="--runtime-arg -verbose:verifier" to see
 
 func bootImageProfileRule(ctx android.SingletonContext, image *bootImage, missingDeps []string) android.WritablePath {
 	globalSoong := dexpreopt.GetCachedGlobalSoongConfig(ctx)
-	global := dexpreoptGlobalConfig(ctx)
+	global := dexpreopt.GetGlobalConfig(ctx)
 
 	if global.DisableGenerateProfile || ctx.Config().IsPdkBuild() || ctx.Config().UnbundledBuild() {
 		return nil
@@ -490,7 +490,7 @@ var bootImageProfileRuleKey = android.NewOnceKey("bootImageProfileRule")
 
 func bootFrameworkProfileRule(ctx android.SingletonContext, image *bootImage, missingDeps []string) android.WritablePath {
 	globalSoong := dexpreopt.GetCachedGlobalSoongConfig(ctx)
-	global := dexpreoptGlobalConfig(ctx)
+	global := dexpreopt.GetGlobalConfig(ctx)
 
 	if global.DisableGenerateProfile || ctx.Config().IsPdkBuild() || ctx.Config().UnbundledBuild() {
 		return nil
@@ -578,7 +578,7 @@ func dumpOatRules(ctx android.SingletonContext, image *bootImage) {
 }
 
 func writeGlobalConfigForMake(ctx android.SingletonContext, path android.WritablePath) {
-	data := dexpreoptGlobalConfigRaw(ctx).data
+	data := dexpreopt.GetGlobalConfigRawData(ctx)
 
 	ctx.Build(pctx, android.BuildParams{
 		Rule:   android.WriteFile,
diff --git a/java/dexpreopt_bootjars_test.go b/java/dexpreopt_bootjars_test.go
index 4ce30f678..c3b2133a9 100644
--- a/java/dexpreopt_bootjars_test.go
+++ b/java/dexpreopt_bootjars_test.go
@@ -49,7 +49,7 @@ func TestDexpreoptBootJars(t *testing.T) {
 	pathCtx := android.PathContextForTesting(config)
 	dexpreoptConfig := dexpreopt.GlobalConfigForTests(pathCtx)
 	dexpreoptConfig.BootJars = []string{"foo", "bar", "baz"}
-	setDexpreoptTestGlobalConfig(config, dexpreoptConfig)
+	dexpreopt.SetTestGlobalConfig(config, dexpreoptConfig)
 
 	ctx := testContext()
 
diff --git a/java/dexpreopt_config.go b/java/dexpreopt_config.go
index 0aa9005f6..96f8042a0 100644
--- a/java/dexpreopt_config.go
+++ b/java/dexpreopt_config.go
@@ -22,56 +22,12 @@ import (
 	"android/soong/dexpreopt"
 )
 
-// dexpreoptGlobalConfig returns the global dexpreopt.config.  It is loaded once the first time it is called for any
-// ctx.Config(), and returns the same data for all future calls with the same ctx.Config().  A value can be inserted
-// for tests using setDexpreoptTestGlobalConfig.
-func dexpreoptGlobalConfig(ctx android.PathContext) dexpreopt.GlobalConfig {
-	return dexpreoptGlobalConfigRaw(ctx).global
-}
-
-type globalConfigAndRaw struct {
-	global dexpreopt.GlobalConfig
-	data   []byte
-}
-
-func dexpreoptGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw {
-	return ctx.Config().Once(dexpreoptGlobalConfigKey, func() interface{} {
-		if data, err := ctx.Config().DexpreoptGlobalConfig(ctx); err != nil {
-			panic(err)
-		} else if data != nil {
-			globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, data)
-			if err != nil {
-				panic(err)
-			}
-			return globalConfigAndRaw{globalConfig, data}
-		}
-
-		// No global config filename set, see if there is a test config set
-		return ctx.Config().Once(dexpreoptTestGlobalConfigKey, func() interface{} {
-			// Nope, return a config with preopting disabled
-			return globalConfigAndRaw{dexpreopt.GlobalConfig{
-				DisablePreopt:          true,
-				DisableGenerateProfile: true,
-			}, nil}
-		})
-	}).(globalConfigAndRaw)
-}
-
-// setDexpreoptTestGlobalConfig sets a GlobalConfig that future calls to dexpreoptGlobalConfig will return.  It must
-// be called before the first call to dexpreoptGlobalConfig for the config.
-func setDexpreoptTestGlobalConfig(config android.Config, globalConfig dexpreopt.GlobalConfig) {
-	config.Once(dexpreoptTestGlobalConfigKey, func() interface{} { return globalConfigAndRaw{globalConfig, nil} })
-}
-
-var dexpreoptGlobalConfigKey = android.NewOnceKey("DexpreoptGlobalConfig")
-var dexpreoptTestGlobalConfigKey = android.NewOnceKey("TestDexpreoptGlobalConfig")
-
 // systemServerClasspath returns the on-device locations of the modules in the system server classpath.  It is computed
 // once the first time it is called for any ctx.Config(), and returns the same slice for all future calls with the same
 // ctx.Config().
 func systemServerClasspath(ctx android.MakeVarsContext) []string {
 	return ctx.Config().OnceStringSlice(systemServerClasspathKey, func() []string {
-		global := dexpreoptGlobalConfig(ctx)
+		global := dexpreopt.GetGlobalConfig(ctx)
 		var systemServerClasspathLocations []string
 		for _, m := range *DexpreoptedSystemServerJars(ctx.Config()) {
 			systemServerClasspathLocations = append(systemServerClasspathLocations,
@@ -120,7 +76,7 @@ var (
 func genBootImageConfigs(ctx android.PathContext) map[string]*bootImageConfig {
 	return ctx.Config().Once(bootImageConfigKey, func() interface{} {
 
-		global := dexpreoptGlobalConfig(ctx)
+		global := dexpreopt.GetGlobalConfig(ctx)
 		targets := dexpreoptTargets(ctx)
 		deviceDir := android.PathForOutput(ctx, ctx.Config().DeviceName())
 
@@ -226,7 +182,7 @@ func defaultBootImageConfig(ctx android.PathContext) bootImageConfig {
 
 func defaultBootclasspath(ctx android.PathContext) []string {
 	return ctx.Config().OnceStringSlice(defaultBootclasspathKey, func() []string {
-		global := dexpreoptGlobalConfig(ctx)
+		global := dexpreopt.GetGlobalConfig(ctx)
 		image := defaultBootImageConfig(ctx)
 
 		updatableBootclasspath := make([]string, len(global.UpdatableBootJars))
diff --git a/java/java.go b/java/java.go
index d84d16272..b20f5f244 100644
--- a/java/java.go
+++ b/java/java.go
@@ -105,7 +105,7 @@ func DexpreoptedSystemServerJars(config android.Config) *[]string {
 // order (which is partial and non-deterministic). This pass adds additional dependencies between
 // jars, making the order total and deterministic. It also constructs a global ordered list.
 func systemServerJarsDepsMutator(ctx android.BottomUpMutatorContext) {
-	jars := dexpreopt.NonUpdatableSystemServerJars(ctx, dexpreoptGlobalConfig(ctx))
+	jars := dexpreopt.NonUpdatableSystemServerJars(ctx, dexpreopt.GetGlobalConfig(ctx))
 	name := ctx.ModuleName()
 	if android.InList(name, jars) {
 		dexpreoptedSystemServerJarsLock.Lock()
diff --git a/java/java_test.go b/java/java_test.go
index 9c5680b0c..129b17bf7 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -99,7 +99,7 @@ func run(t *testing.T, ctx *android.TestContext, config android.Config) {
 	t.Helper()
 
 	pathCtx := android.PathContextForTesting(config)
-	setDexpreoptTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
+	dexpreopt.SetTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
 
 	ctx.Register(config)
 	_, errs := ctx.ParseBlueprintsFiles("Android.bp")
@@ -118,7 +118,7 @@ func testJavaErrorWithConfig(t *testing.T, pattern string, config android.Config
 	ctx := testContext()
 
 	pathCtx := android.PathContextForTesting(config)
-	setDexpreoptTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
+	dexpreopt.SetTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
 
 	ctx.Register(config)
 	_, errs := ctx.ParseBlueprintsFiles("Android.bp")

From d90676fdde67b6e3f76819092c86882aa9306a7c Mon Sep 17 00:00:00 2001
From: Martin Stjernholm <mast@google.com>
Date: Sat, 11 Jan 2020 00:37:30 +0000
Subject: [PATCH 3/5] Reland: Get the dex2oat host tool path from module
 dependency on the binary module.

This uses the Once cache for GlobalSoongConfig to propagate the dex2oat
path from a module dependency to the singletons (both the one that
writes out dexpreopt_soong.config and the one that creates the
dexpreopted boot images). Unless dexpreopting is disabled altogether
through DisablePreopt in dexpreopt.config, that means:

- We must ensure at least one module registers a dex2oat tool
  dependency and resolves a GlobalSoongConfig using it, or else the
  singletons fail. That means we litter dex2oat dependencies in java
  modules even when they won't get dexpreopted and hence don't really
  need them.

- We still assume there's at least one java_library or android_app in
  the build.

This relands https://r.android.com/1205730 without changes - the
necessary fixes are in the child CLs.

Bug: 145934348
Test: m
  (check that out/soong/dexpreopt_soong.config points to dex2oatd64)
Test: env USE_DEX2OAT_DEBUG=false m
  (check that out/soong/dexpreopt_soong.config points to dex2oat64)
Test: env OUT_DIR=out-tools prebuilts/build-tools/build-prebuilts.sh
  on the aosp-build-tools branch
Change-Id: I66661711b317d1e4ec434861982919bdde19b575
---
 dexpreopt/Android.bp |   1 +
 dexpreopt/config.go  | 105 ++++++++++++++++++++++++++++++++++++++-----
 dexpreopt/testing.go |  47 +++++++++++++++++++
 java/app.go          |   2 +
 java/java.go         |  15 +++++++
 java/java_test.go    |   4 ++
 6 files changed, 164 insertions(+), 10 deletions(-)
 create mode 100644 dexpreopt/testing.go

diff --git a/dexpreopt/Android.bp b/dexpreopt/Android.bp
index c5f24e273..b8f7ea682 100644
--- a/dexpreopt/Android.bp
+++ b/dexpreopt/Android.bp
@@ -4,6 +4,7 @@ bootstrap_go_package {
     srcs: [
         "config.go",
         "dexpreopt.go",
+        "testing.go",
     ],
     testSrcs: [
         "dexpreopt_test.go",
diff --git a/dexpreopt/config.go b/dexpreopt/config.go
index 7a404f5a6..94f128383 100644
--- a/dexpreopt/config.go
+++ b/dexpreopt/config.go
@@ -16,8 +16,11 @@ package dexpreopt
 
 import (
 	"encoding/json"
+	"fmt"
 	"strings"
 
+	"github.com/google/blueprint"
+
 	"android/soong/android"
 )
 
@@ -297,6 +300,71 @@ func ParseModuleConfig(ctx android.PathContext, data []byte) (ModuleConfig, erro
 	return config.ModuleConfig, nil
 }
 
+// dex2oatModuleName returns the name of the module to use for the dex2oat host
+// tool. It should be a binary module with public visibility that is compiled
+// and installed for host.
+func dex2oatModuleName(config android.Config) string {
+	// Default to the debug variant of dex2oat to help find bugs.
+	// Set USE_DEX2OAT_DEBUG to false for only building non-debug versions.
+	if config.Getenv("USE_DEX2OAT_DEBUG") == "false" {
+		return "dex2oat"
+	} else {
+		return "dex2oatd"
+	}
+}
+
+var dex2oatDepTag = struct {
+	blueprint.BaseDependencyTag
+}{}
+
+type DexPreoptModule interface {
+	dexPreoptModuleSignature() // Not called - only for type detection.
+}
+
+// RegisterToolDepsMutator registers a mutator that adds the necessary
+// dependencies to binary modules for tools that are required later when
+// Get(Cached)GlobalSoongConfig is called. It should be passed to
+// android.RegistrationContext.FinalDepsMutators, and module types that need
+// dependencies also need to embed DexPreoptModule.
+func RegisterToolDepsMutator(ctx android.RegisterMutatorsContext) {
+	ctx.BottomUp("dexpreopt_tool_deps", toolDepsMutator).Parallel()
+}
+
+func toolDepsMutator(ctx android.BottomUpMutatorContext) {
+	if GetGlobalConfig(ctx).DisablePreopt {
+		// Only register dependencies if dexpreopting is enabled. Necessary to avoid
+		// them in non-platform builds where dex2oat etc isn't available.
+		//
+		// It would be nice to not register this mutator at all then, but
+		// RegisterMutatorsContext available at registration doesn't have the state
+		// necessary to pass as PathContext to constructPath etc.
+		return
+	}
+	if _, ok := ctx.Module().(DexPreoptModule); !ok {
+		return
+	}
+	dex2oatBin := dex2oatModuleName(ctx.Config())
+	v := ctx.Config().BuildOSTarget.Variations()
+	ctx.AddFarVariationDependencies(v, dex2oatDepTag, dex2oatBin)
+}
+
+func dex2oatPathFromDep(ctx android.ModuleContext) android.Path {
+	dex2oatBin := dex2oatModuleName(ctx.Config())
+
+	dex2oatModule := ctx.GetDirectDepWithTag(dex2oatBin, dex2oatDepTag)
+	if dex2oatModule == nil {
+		// If this happens there's probably a missing call to AddToolDeps in DepsMutator.
+		panic(fmt.Sprintf("Failed to lookup %s dependency", dex2oatBin))
+	}
+
+	dex2oatPath := dex2oatModule.(android.HostToolProvider).HostToolPath()
+	if !dex2oatPath.Valid() {
+		panic(fmt.Sprintf("Failed to find host tool path in %s", dex2oatModule))
+	}
+
+	return dex2oatPath.Path()
+}
+
 // createGlobalSoongConfig creates a GlobalSoongConfig from the current context.
 // Should not be used in dexpreopt_gen.
 func createGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
@@ -307,18 +375,9 @@ func createGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
 		panic("This should not be called from tests. Please call GlobalSoongConfigForTests somewhere in the test setup.")
 	}
 
-	// Default to debug version to help find bugs.
-	// Set USE_DEX2OAT_DEBUG to false for only building non-debug versions.
-	var dex2oatBinary string
-	if ctx.Config().Getenv("USE_DEX2OAT_DEBUG") == "false" {
-		dex2oatBinary = "dex2oat"
-	} else {
-		dex2oatBinary = "dex2oatd"
-	}
-
 	return GlobalSoongConfig{
 		Profman:          ctx.Config().HostToolPath(ctx, "profman"),
-		Dex2oat:          ctx.Config().HostToolPath(ctx, dex2oatBinary),
+		Dex2oat:          dex2oatPathFromDep(ctx),
 		Aapt:             ctx.Config().HostToolPath(ctx, "aapt"),
 		SoongZip:         ctx.Config().HostToolPath(ctx, "soong_zip"),
 		Zip2zip:          ctx.Config().HostToolPath(ctx, "zip2zip"),
@@ -327,6 +386,16 @@ func createGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
 	}
 }
 
+// The main reason for this Once cache for GlobalSoongConfig is to make the
+// dex2oat path available to singletons. In ordinary modules we get it through a
+// dex2oatDepTag dependency, but in singletons there's no simple way to do the
+// same thing and ensure the right variant is selected, hence this cache to make
+// the resolved path available to singletons. This means we depend on there
+// being at least one ordinary module with a dex2oatDepTag dependency.
+//
+// TODO(b/147613152): Implement a way to deal with dependencies from singletons,
+// and then possibly remove this cache altogether (but the use in
+// GlobalSoongConfigForTests also needs to be rethought).
 var globalSoongConfigOnceKey = android.NewOnceKey("DexpreoptGlobalSoongConfig")
 
 // GetGlobalSoongConfig creates a GlobalSoongConfig the first time it's called,
@@ -335,6 +404,14 @@ func GetGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
 	globalSoong := ctx.Config().Once(globalSoongConfigOnceKey, func() interface{} {
 		return createGlobalSoongConfig(ctx)
 	}).(GlobalSoongConfig)
+
+	// Always resolve the tool path from the dependency, to ensure that every
+	// module has the dependency added properly.
+	myDex2oat := dex2oatPathFromDep(ctx)
+	if myDex2oat != globalSoong.Dex2oat {
+		panic(fmt.Sprintf("Inconsistent dex2oat path in cached config: expected %s, got %s", globalSoong.Dex2oat, myDex2oat))
+	}
+
 	return globalSoong
 }
 
@@ -382,6 +459,10 @@ func ParseGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongCo
 }
 
 func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonContext) {
+	if GetGlobalConfig(ctx).DisablePreopt {
+		return
+	}
+
 	config := GetCachedGlobalSoongConfig(ctx)
 	jc := globalJsonSoongConfig{
 		Profman:          config.Profman.String(),
@@ -409,6 +490,10 @@ func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonC
 }
 
 func (s *globalSoongConfigSingleton) MakeVars(ctx android.MakeVarsContext) {
+	if GetGlobalConfig(ctx).DisablePreopt {
+		return
+	}
+
 	config := GetCachedGlobalSoongConfig(ctx)
 
 	ctx.Strict("DEX2OAT", config.Dex2oat.String())
diff --git a/dexpreopt/testing.go b/dexpreopt/testing.go
new file mode 100644
index 000000000..b572eb351
--- /dev/null
+++ b/dexpreopt/testing.go
@@ -0,0 +1,47 @@
+// Copyright 2020 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package dexpreopt
+
+import (
+	"android/soong/android"
+)
+
+type dummyToolBinary struct {
+	android.ModuleBase
+}
+
+func (m *dummyToolBinary) GenerateAndroidBuildActions(ctx android.ModuleContext) {}
+
+func (m *dummyToolBinary) HostToolPath() android.OptionalPath {
+	return android.OptionalPathForPath(android.PathForTesting("dex2oat"))
+}
+
+func dummyToolBinaryFactory() android.Module {
+	module := &dummyToolBinary{}
+	android.InitAndroidArchModule(module, android.HostSupported, android.MultilibFirst)
+	return module
+}
+
+func RegisterToolModulesForTest(ctx *android.TestContext) {
+	ctx.RegisterModuleType("dummy_tool_binary", dummyToolBinaryFactory)
+}
+
+func BpToolModulesForTest() string {
+	return `
+		dummy_tool_binary {
+			name: "dex2oatd",
+		}
+	`
+}
diff --git a/java/app.go b/java/app.go
index 9503ec455..f55fc45a1 100755
--- a/java/app.go
+++ b/java/app.go
@@ -27,6 +27,7 @@ import (
 
 	"android/soong/android"
 	"android/soong/cc"
+	"android/soong/dexpreopt"
 	"android/soong/tradefed"
 )
 
@@ -875,6 +876,7 @@ type AndroidAppImport struct {
 	android.ModuleBase
 	android.DefaultableModuleBase
 	prebuilt android.Prebuilt
+	dexpreopt.DexPreoptModule
 
 	properties   AndroidAppImportProperties
 	dpiVariants  interface{}
diff --git a/java/java.go b/java/java.go
index b20f5f244..2e53654cc 100644
--- a/java/java.go
+++ b/java/java.go
@@ -76,6 +76,8 @@ func RegisterJavaBuildComponents(ctx android.RegistrationContext) {
 	ctx.RegisterModuleType("java_host_for_device", HostForDeviceFactory)
 	ctx.RegisterModuleType("dex_import", DexImportFactory)
 
+	ctx.FinalDepsMutators(dexpreopt.RegisterToolDepsMutator)
+
 	ctx.RegisterSingletonType("logtags", LogtagsSingleton)
 	ctx.RegisterSingletonType("kythe_java_extract", kytheExtractJavaFactory)
 }
@@ -369,6 +371,7 @@ type Module struct {
 	android.DefaultableModuleBase
 	android.ApexModuleBase
 	android.SdkBase
+	dexpreopt.DexPreoptModule
 
 	properties       CompilerProperties
 	protoProperties  android.ProtoProperties
@@ -1574,6 +1577,16 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) {
 		}
 	} else {
 		outputFile = implementationAndResourcesJar
+
+		// dexpreopt.GetGlobalSoongConfig needs to be called at least once even if
+		// no module actually is dexpreopted, to ensure there's a cached
+		// GlobalSoongConfig for the dexpreopt singletons, which will run
+		// regardless.
+		// TODO(b/147613152): Remove when the singletons no longer rely on the
+		// cached GlobalSoongConfig.
+		if !dexpreopt.GetGlobalConfig(ctx).DisablePreopt {
+			_ = dexpreopt.GetGlobalSoongConfig(ctx)
+		}
 	}
 
 	ctx.CheckbuildFile(outputFile)
@@ -2340,6 +2353,7 @@ type Import struct {
 	android.ApexModuleBase
 	prebuilt android.Prebuilt
 	android.SdkBase
+	dexpreopt.DexPreoptModule
 
 	properties ImportProperties
 
@@ -2550,6 +2564,7 @@ type DexImport struct {
 	android.DefaultableModuleBase
 	android.ApexModuleBase
 	prebuilt android.Prebuilt
+	dexpreopt.DexPreoptModule
 
 	properties DexImportProperties
 
diff --git a/java/java_test.go b/java/java_test.go
index 129b17bf7..a2226b59e 100644
--- a/java/java_test.go
+++ b/java/java_test.go
@@ -57,6 +57,8 @@ func TestMain(m *testing.M) {
 }
 
 func testConfig(env map[string]string, bp string, fs map[string][]byte) android.Config {
+	bp += dexpreopt.BpToolModulesForTest()
+
 	config := TestConfig(buildDir, env, bp, fs)
 
 	// Set up the global Once cache used for dexpreopt.GlobalSoongConfig, so that
@@ -92,6 +94,8 @@ func testContext() *android.TestContext {
 	cc.RegisterRequiredBuildComponentsForTest(ctx)
 	ctx.RegisterModuleType("ndk_prebuilt_shared_stl", cc.NdkPrebuiltSharedStlFactory)
 
+	dexpreopt.RegisterToolModulesForTest(ctx)
+
 	return ctx
 }
 

From 8d80ceeb1271974669d1ea19ab0908c9ef64db79 Mon Sep 17 00:00:00 2001
From: Martin Stjernholm <mast@google.com>
Date: Fri, 31 Jan 2020 17:44:54 +0000
Subject: [PATCH 4/5] Pass dexpreopt config structs by reference.

Should cut down on a bit of copying, and also required for an upcoming
CL that'll change GetCachedGlobalSoongConfig.

Test: m nothing
Bug: 145934348
Change-Id: I6bed737d9b061b5239cc603ad881f4ccd4312e43
---
 dexpreopt/config.go                      | 44 ++++++++++++------------
 dexpreopt/dexpreopt.go                   | 28 +++++++--------
 dexpreopt/dexpreopt_gen/dexpreopt_gen.go |  4 +--
 dexpreopt/dexpreopt_test.go              | 12 +++----
 java/dexpreopt.go                        |  2 +-
 5 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/dexpreopt/config.go b/dexpreopt/config.go
index 94f128383..3ab341438 100644
--- a/dexpreopt/config.go
+++ b/dexpreopt/config.go
@@ -180,9 +180,9 @@ func constructWritablePath(ctx android.PathContext, path string) android.Writabl
 
 // ParseGlobalConfig parses the given data assumed to be read from the global
 // dexpreopt.config file into a GlobalConfig struct.
-func ParseGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, error) {
+func ParseGlobalConfig(ctx android.PathContext, data []byte) (*GlobalConfig, error) {
 	type GlobalJSONConfig struct {
-		GlobalConfig
+		*GlobalConfig
 
 		// Copies of entries in GlobalConfig that are not constructable without extra parameters.  They will be
 		// used to construct the real value manually below.
@@ -204,7 +204,7 @@ func ParseGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, erro
 }
 
 type globalConfigAndRaw struct {
-	global GlobalConfig
+	global *GlobalConfig
 	data   []byte
 }
 
@@ -213,7 +213,7 @@ type globalConfigAndRaw struct {
 // ctx.Config(), and returns the same data for all future calls with the same
 // ctx.Config(). A value can be inserted for tests using
 // setDexpreoptTestGlobalConfig.
-func GetGlobalConfig(ctx android.PathContext) GlobalConfig {
+func GetGlobalConfig(ctx android.PathContext) *GlobalConfig {
 	return getGlobalConfigRaw(ctx).global
 }
 
@@ -241,7 +241,7 @@ func getGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw {
 		// No global config filename set, see if there is a test config set
 		return ctx.Config().Once(testGlobalConfigOnceKey, func() interface{} {
 			// Nope, return a config with preopting disabled
-			return globalConfigAndRaw{GlobalConfig{
+			return globalConfigAndRaw{&GlobalConfig{
 				DisablePreopt:          true,
 				DisableGenerateProfile: true,
 			}, nil}
@@ -252,7 +252,7 @@ func getGlobalConfigRaw(ctx android.PathContext) globalConfigAndRaw {
 // SetTestGlobalConfig sets a GlobalConfig that future calls to GetGlobalConfig
 // will return. It must be called before the first call to GetGlobalConfig for
 // the config.
-func SetTestGlobalConfig(config android.Config, globalConfig GlobalConfig) {
+func SetTestGlobalConfig(config android.Config, globalConfig *GlobalConfig) {
 	config.Once(testGlobalConfigOnceKey, func() interface{} { return globalConfigAndRaw{globalConfig, nil} })
 }
 
@@ -261,9 +261,9 @@ func SetTestGlobalConfig(config android.Config, globalConfig GlobalConfig) {
 // struct directly from java/dexpreopt.go. It is used in dexpreopt_gen called
 // from Make to read the module dexpreopt.config written in the Make config
 // stage.
-func ParseModuleConfig(ctx android.PathContext, data []byte) (ModuleConfig, error) {
+func ParseModuleConfig(ctx android.PathContext, data []byte) (*ModuleConfig, error) {
 	type ModuleJSONConfig struct {
-		ModuleConfig
+		*ModuleConfig
 
 		// Copies of entries in ModuleConfig that are not constructable without extra parameters.  They will be
 		// used to construct the real value manually below.
@@ -367,7 +367,7 @@ func dex2oatPathFromDep(ctx android.ModuleContext) android.Path {
 
 // createGlobalSoongConfig creates a GlobalSoongConfig from the current context.
 // Should not be used in dexpreopt_gen.
-func createGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
+func createGlobalSoongConfig(ctx android.ModuleContext) *GlobalSoongConfig {
 	if ctx.Config().TestProductVariables != nil {
 		// If we're called in a test there'll be a confusing error from the path
 		// functions below that gets reported without a stack trace, so let's panic
@@ -375,7 +375,7 @@ func createGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
 		panic("This should not be called from tests. Please call GlobalSoongConfigForTests somewhere in the test setup.")
 	}
 
-	return GlobalSoongConfig{
+	return &GlobalSoongConfig{
 		Profman:          ctx.Config().HostToolPath(ctx, "profman"),
 		Dex2oat:          dex2oatPathFromDep(ctx),
 		Aapt:             ctx.Config().HostToolPath(ctx, "aapt"),
@@ -400,10 +400,10 @@ var globalSoongConfigOnceKey = android.NewOnceKey("DexpreoptGlobalSoongConfig")
 
 // GetGlobalSoongConfig creates a GlobalSoongConfig the first time it's called,
 // and later returns the same cached instance.
-func GetGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
+func GetGlobalSoongConfig(ctx android.ModuleContext) *GlobalSoongConfig {
 	globalSoong := ctx.Config().Once(globalSoongConfigOnceKey, func() interface{} {
 		return createGlobalSoongConfig(ctx)
-	}).(GlobalSoongConfig)
+	}).(*GlobalSoongConfig)
 
 	// Always resolve the tool path from the dependency, to ensure that every
 	// module has the dependency added properly.
@@ -420,8 +420,8 @@ func GetGlobalSoongConfig(ctx android.ModuleContext) GlobalSoongConfig {
 // compatible with a basic PathContext, since it doesn't try to create a
 // GlobalSoongConfig (which requires a full ModuleContext). It will panic if
 // called before the first GetGlobalSoongConfig call.
-func GetCachedGlobalSoongConfig(ctx android.PathContext) GlobalSoongConfig {
-	return ctx.Config().Get(globalSoongConfigOnceKey).(GlobalSoongConfig)
+func GetCachedGlobalSoongConfig(ctx android.PathContext) *GlobalSoongConfig {
+	return ctx.Config().Get(globalSoongConfigOnceKey).(*GlobalSoongConfig)
 }
 
 type globalJsonSoongConfig struct {
@@ -437,15 +437,15 @@ type globalJsonSoongConfig struct {
 // ParseGlobalSoongConfig parses the given data assumed to be read from the
 // global dexpreopt_soong.config file into a GlobalSoongConfig struct. It is
 // only used in dexpreopt_gen.
-func ParseGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongConfig, error) {
+func ParseGlobalSoongConfig(ctx android.PathContext, data []byte) (*GlobalSoongConfig, error) {
 	var jc globalJsonSoongConfig
 
 	err := json.Unmarshal(data, &jc)
 	if err != nil {
-		return GlobalSoongConfig{}, err
+		return &GlobalSoongConfig{}, err
 	}
 
-	config := GlobalSoongConfig{
+	config := &GlobalSoongConfig{
 		Profman:          constructPath(ctx, jc.Profman),
 		Dex2oat:          constructPath(ctx, jc.Dex2oat),
 		Aapt:             constructPath(ctx, jc.Aapt),
@@ -508,8 +508,8 @@ func (s *globalSoongConfigSingleton) MakeVars(ctx android.MakeVarsContext) {
 	}, " "))
 }
 
-func GlobalConfigForTests(ctx android.PathContext) GlobalConfig {
-	return GlobalConfig{
+func GlobalConfigForTests(ctx android.PathContext) *GlobalConfig {
+	return &GlobalConfig{
 		DisablePreopt:                      false,
 		DisablePreoptModules:               nil,
 		OnlyPreoptBootImageAndSystemServer: false,
@@ -550,11 +550,11 @@ func GlobalConfigForTests(ctx android.PathContext) GlobalConfig {
 	}
 }
 
-func GlobalSoongConfigForTests(config android.Config) GlobalSoongConfig {
+func GlobalSoongConfigForTests(config android.Config) *GlobalSoongConfig {
 	// Install the test GlobalSoongConfig in the Once cache so that later calls to
 	// Get(Cached)GlobalSoongConfig returns it without trying to create a real one.
 	return config.Once(globalSoongConfigOnceKey, func() interface{} {
-		return GlobalSoongConfig{
+		return &GlobalSoongConfig{
 			Profman:          android.PathForTesting("profman"),
 			Dex2oat:          android.PathForTesting("dex2oat"),
 			Aapt:             android.PathForTesting("aapt"),
@@ -563,5 +563,5 @@ func GlobalSoongConfigForTests(config android.Config) GlobalSoongConfig {
 			ManifestCheck:    android.PathForTesting("manifest_check"),
 			ConstructContext: android.PathForTesting("construct_context.sh"),
 		}
-	}).(GlobalSoongConfig)
+	}).(*GlobalSoongConfig)
 }
diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go
index dbf08f2c1..9b0e7a5d8 100644
--- a/dexpreopt/dexpreopt.go
+++ b/dexpreopt/dexpreopt.go
@@ -58,8 +58,8 @@ var SystemServerForcedDepTag = dependencyTag{name: "system-server-forced-dep"}
 
 // GenerateDexpreoptRule generates a set of commands that will preopt a module based on a GlobalConfig and a
 // ModuleConfig.  The produced files and their install locations will be available through rule.Installs().
-func GenerateDexpreoptRule(ctx android.PathContext, globalSoong GlobalSoongConfig,
-	global GlobalConfig, module ModuleConfig) (rule *android.RuleBuilder, err error) {
+func GenerateDexpreoptRule(ctx android.PathContext, globalSoong *GlobalSoongConfig,
+	global *GlobalConfig, module *ModuleConfig) (rule *android.RuleBuilder, err error) {
 
 	defer func() {
 		if r := recover(); r != nil {
@@ -104,7 +104,7 @@ func GenerateDexpreoptRule(ctx android.PathContext, globalSoong GlobalSoongConfi
 	return rule, nil
 }
 
-func dexpreoptDisabled(ctx android.PathContext, global GlobalConfig, module ModuleConfig) bool {
+func dexpreoptDisabled(ctx android.PathContext, global *GlobalConfig, module *ModuleConfig) bool {
 	if contains(global.DisablePreoptModules, module.Name) {
 		return true
 	}
@@ -135,8 +135,8 @@ func dexpreoptDisabled(ctx android.PathContext, global GlobalConfig, module Modu
 	return false
 }
 
-func profileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
-	module ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
+func profileCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, global *GlobalConfig,
+	module *ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
 
 	profilePath := module.BuildPath.InSameDir(ctx, "profile.prof")
 	profileInstalledPath := module.DexLocation + ".prof"
@@ -174,8 +174,8 @@ func profileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, glob
 	return profilePath
 }
 
-func bootProfileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
-	module ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
+func bootProfileCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, global *GlobalConfig,
+	module *ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
 
 	profilePath := module.BuildPath.InSameDir(ctx, "profile.bprof")
 	profileInstalledPath := module.DexLocation + ".bprof"
@@ -206,8 +206,8 @@ func bootProfileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig,
 	return profilePath
 }
 
-func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
-	module ModuleConfig, rule *android.RuleBuilder, archIdx int, profile android.WritablePath,
+func dexpreoptCommand(ctx android.PathContext, globalSoong *GlobalSoongConfig, global *GlobalConfig,
+	module *ModuleConfig, rule *android.RuleBuilder, archIdx int, profile android.WritablePath,
 	appImage bool, generateDM bool) {
 
 	arch := module.Archs[archIdx]
@@ -521,14 +521,14 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, gl
 	rule.Install(vdexPath, vdexInstallPath)
 }
 
-func shouldGenerateDM(module ModuleConfig, global GlobalConfig) bool {
+func shouldGenerateDM(module *ModuleConfig, global *GlobalConfig) bool {
 	// Generating DM files only makes sense for verify, avoid doing for non verify compiler filter APKs.
 	// No reason to use a dm file if the dex is already uncompressed.
 	return global.GenerateDMFiles && !module.UncompressedDex &&
 		contains(module.PreoptFlags, "--compiler-filter=verify")
 }
 
-func OdexOnSystemOtherByName(name string, dexLocation string, global GlobalConfig) bool {
+func OdexOnSystemOtherByName(name string, dexLocation string, global *GlobalConfig) bool {
 	if !global.HasSystemOther {
 		return false
 	}
@@ -550,7 +550,7 @@ func OdexOnSystemOtherByName(name string, dexLocation string, global GlobalConfi
 	return false
 }
 
-func odexOnSystemOther(module ModuleConfig, global GlobalConfig) bool {
+func odexOnSystemOther(module *ModuleConfig, global *GlobalConfig) bool {
 	return OdexOnSystemOtherByName(module.Name, module.DexLocation, global)
 }
 
@@ -563,7 +563,7 @@ func PathToLocation(path android.Path, arch android.ArchType) string {
 	return filepath.Join(filepath.Dir(filepath.Dir(path.String())), filepath.Base(path.String()))
 }
 
-func pathForLibrary(module ModuleConfig, lib string) android.Path {
+func pathForLibrary(module *ModuleConfig, lib string) android.Path {
 	path, ok := module.LibraryPaths[lib]
 	if !ok {
 		panic(fmt.Errorf("unknown library path for %q", lib))
@@ -602,7 +602,7 @@ var nonUpdatableSystemServerJarsKey = android.NewOnceKey("nonUpdatableSystemServ
 
 // TODO: eliminate the superficial global config parameter by moving global config definition
 // from java subpackage to dexpreopt.
-func NonUpdatableSystemServerJars(ctx android.PathContext, global GlobalConfig) []string {
+func NonUpdatableSystemServerJars(ctx android.PathContext, global *GlobalConfig) []string {
 	return ctx.Config().Once(nonUpdatableSystemServerJarsKey, func() interface{} {
 		return android.RemoveListFromList(global.SystemServerJars,
 			GetJarsFromApexJarPairs(global.UpdatableSystemServerJars))
diff --git a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
index 4da003e11..e89f04591 100644
--- a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
+++ b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go
@@ -133,8 +133,8 @@ func main() {
 	writeScripts(ctx, globalSoongConfig, globalConfig, moduleConfig, *dexpreoptScriptPath)
 }
 
-func writeScripts(ctx android.PathContext, globalSoong dexpreopt.GlobalSoongConfig,
-	global dexpreopt.GlobalConfig, module dexpreopt.ModuleConfig, dexpreoptScriptPath string) {
+func writeScripts(ctx android.PathContext, globalSoong *dexpreopt.GlobalSoongConfig,
+	global *dexpreopt.GlobalConfig, module *dexpreopt.ModuleConfig, dexpreoptScriptPath string) {
 	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, globalSoong, global, module)
 	if err != nil {
 		panic(err)
diff --git a/dexpreopt/dexpreopt_test.go b/dexpreopt/dexpreopt_test.go
index 44bbbc247..d23999328 100644
--- a/dexpreopt/dexpreopt_test.go
+++ b/dexpreopt/dexpreopt_test.go
@@ -20,20 +20,20 @@ import (
 	"testing"
 )
 
-func testSystemModuleConfig(ctx android.PathContext, name string) ModuleConfig {
+func testSystemModuleConfig(ctx android.PathContext, name string) *ModuleConfig {
 	return testModuleConfig(ctx, name, "system")
 }
 
-func testSystemProductModuleConfig(ctx android.PathContext, name string) ModuleConfig {
+func testSystemProductModuleConfig(ctx android.PathContext, name string) *ModuleConfig {
 	return testModuleConfig(ctx, name, "system/product")
 }
 
-func testProductModuleConfig(ctx android.PathContext, name string) ModuleConfig {
+func testProductModuleConfig(ctx android.PathContext, name string) *ModuleConfig {
 	return testModuleConfig(ctx, name, "product")
 }
 
-func testModuleConfig(ctx android.PathContext, name, partition string) ModuleConfig {
-	return ModuleConfig{
+func testModuleConfig(ctx android.PathContext, name, partition string) *ModuleConfig {
+	return &ModuleConfig{
 		Name:                            name,
 		DexLocation:                     fmt.Sprintf("/%s/app/test/%s.apk", partition, name),
 		BuildPath:                       android.PathForOutput(ctx, fmt.Sprintf("%s/%s.apk", name, name)),
@@ -94,7 +94,7 @@ func TestDexPreoptSystemOther(t *testing.T) {
 	global.HasSystemOther = true
 
 	type moduleTest struct {
-		module            ModuleConfig
+		module            *ModuleConfig
 		expectedPartition string
 	}
 	tests := []struct {
diff --git a/java/dexpreopt.go b/java/dexpreopt.go
index 428aee2aa..2b87cf797 100644
--- a/java/dexpreopt.go
+++ b/java/dexpreopt.go
@@ -157,7 +157,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 		}
 	}
 
-	dexpreoptConfig := dexpreopt.ModuleConfig{
+	dexpreoptConfig := &dexpreopt.ModuleConfig{
 		Name:            ctx.ModuleName(),
 		DexLocation:     dexLocation,
 		BuildPath:       android.PathForModuleOut(ctx, "dexpreopt", ctx.ModuleName()+".jar").OutputPath,

From 6d415273c04e5b5fd449b14094c0fe4431b94a43 Mon Sep 17 00:00:00 2001
From: Martin Stjernholm <mast@google.com>
Date: Fri, 31 Jan 2020 17:10:36 +0000
Subject: [PATCH 5/5] Do all dexpreoptDisabled checks before registering a
 dex2oat host dep.

Also disable dexpreopting for host. These are necessary to avoid adding
dependencies on dex2oat in various non-platform builds where they will
break.

Since we cannot assume at least one module enables dexpreopting now,
the two dexpreopt singletons are silently disabled if there has been no
call to dexpreopt.GetGlobalSoongConfig.

Bug: 145934348
Bug: 148312086
Bug: 148319588
Bug: 148690468
Test: m
Test: env OUT_DIR=out-tools prebuilts/build-tools/build-prebuilts.sh
  on the aosp-build-tools branch
Test: build/soong/soong_ui.bash --make-mode static_sdk_tools dist DIST_DIR=out-dist BUILD_HOST_static=1
  on internal (cf b/148312086#comment8)
Test: build/soong/soong_ui.bash --make-mode dist DIST_DIR=out-apps TARGET_BUILD_APPS=Launcher3 TARGET_BUILD_VARIANT=userdebug
  on internal without art/ and external/vixl/ (cf b/148319588)
Change-Id: I240dade7204b87fc2d12181534ab23439eca8b46
---
 dexpreopt/config.go        | 49 ++++++++++++++++----------------------
 java/app.go                | 12 ++++++----
 java/dexpreopt.go          | 27 +++++++++++++++++----
 java/dexpreopt_bootjars.go |  4 ++++
 java/java.go               | 27 +++++++++------------
 5 files changed, 66 insertions(+), 53 deletions(-)

diff --git a/dexpreopt/config.go b/dexpreopt/config.go
index 3ab341438..98850e51a 100644
--- a/dexpreopt/config.go
+++ b/dexpreopt/config.go
@@ -317,32 +317,11 @@ var dex2oatDepTag = struct {
 	blueprint.BaseDependencyTag
 }{}
 
-type DexPreoptModule interface {
-	dexPreoptModuleSignature() // Not called - only for type detection.
-}
-
-// RegisterToolDepsMutator registers a mutator that adds the necessary
-// dependencies to binary modules for tools that are required later when
-// Get(Cached)GlobalSoongConfig is called. It should be passed to
-// android.RegistrationContext.FinalDepsMutators, and module types that need
-// dependencies also need to embed DexPreoptModule.
-func RegisterToolDepsMutator(ctx android.RegisterMutatorsContext) {
-	ctx.BottomUp("dexpreopt_tool_deps", toolDepsMutator).Parallel()
-}
-
-func toolDepsMutator(ctx android.BottomUpMutatorContext) {
-	if GetGlobalConfig(ctx).DisablePreopt {
-		// Only register dependencies if dexpreopting is enabled. Necessary to avoid
-		// them in non-platform builds where dex2oat etc isn't available.
-		//
-		// It would be nice to not register this mutator at all then, but
-		// RegisterMutatorsContext available at registration doesn't have the state
-		// necessary to pass as PathContext to constructPath etc.
-		return
-	}
-	if _, ok := ctx.Module().(DexPreoptModule); !ok {
-		return
-	}
+// RegisterToolDeps adds the necessary dependencies to binary modules for tools
+// that are required later when Get(Cached)GlobalSoongConfig is called. It
+// should be called from a mutator that's registered with
+// android.RegistrationContext.FinalDepsMutators.
+func RegisterToolDeps(ctx android.BottomUpMutatorContext) {
 	dex2oatBin := dex2oatModuleName(ctx.Config())
 	v := ctx.Config().BuildOSTarget.Variations()
 	ctx.AddFarVariationDependencies(v, dex2oatDepTag, dex2oatBin)
@@ -418,10 +397,13 @@ func GetGlobalSoongConfig(ctx android.ModuleContext) *GlobalSoongConfig {
 // GetCachedGlobalSoongConfig returns a cached GlobalSoongConfig created by an
 // earlier GetGlobalSoongConfig call. This function works with any context
 // compatible with a basic PathContext, since it doesn't try to create a
-// GlobalSoongConfig (which requires a full ModuleContext). It will panic if
-// called before the first GetGlobalSoongConfig call.
+// GlobalSoongConfig with the proper paths (which requires a full
+// ModuleContext). If there has been no prior call to GetGlobalSoongConfig, nil
+// is returned.
 func GetCachedGlobalSoongConfig(ctx android.PathContext) *GlobalSoongConfig {
-	return ctx.Config().Get(globalSoongConfigOnceKey).(*GlobalSoongConfig)
+	return ctx.Config().Once(globalSoongConfigOnceKey, func() interface{} {
+		return (*GlobalSoongConfig)(nil)
+	}).(*GlobalSoongConfig)
 }
 
 type globalJsonSoongConfig struct {
@@ -464,6 +446,12 @@ func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonC
 	}
 
 	config := GetCachedGlobalSoongConfig(ctx)
+	if config == nil {
+		// No module has enabled dexpreopting, so we assume there will be no calls
+		// to dexpreopt_gen.
+		return
+	}
+
 	jc := globalJsonSoongConfig{
 		Profman:          config.Profman.String(),
 		Dex2oat:          config.Dex2oat.String(),
@@ -495,6 +483,9 @@ func (s *globalSoongConfigSingleton) MakeVars(ctx android.MakeVarsContext) {
 	}
 
 	config := GetCachedGlobalSoongConfig(ctx)
+	if config == nil {
+		return
+	}
 
 	ctx.Strict("DEX2OAT", config.Dex2oat.String())
 	ctx.Strict("DEXPREOPT_GEN_DEPS", strings.Join([]string{
diff --git a/java/app.go b/java/app.go
index f55fc45a1..02f3e7fc0 100755
--- a/java/app.go
+++ b/java/app.go
@@ -27,7 +27,6 @@ import (
 
 	"android/soong/android"
 	"android/soong/cc"
-	"android/soong/dexpreopt"
 	"android/soong/tradefed"
 )
 
@@ -143,6 +142,10 @@ type AndroidApp struct {
 	noticeOutputs android.NoticeOutputs
 }
 
+func (a *AndroidApp) IsInstallable() bool {
+	return Bool(a.properties.Installable)
+}
+
 func (a *AndroidApp) ExportedProguardFlagFiles() android.Paths {
 	return nil
 }
@@ -339,7 +342,6 @@ func (a *AndroidApp) dexBuildActions(ctx android.ModuleContext) android.Path {
 		installDir = filepath.Join("app", a.installApkName)
 	}
 	a.dexpreopter.installPath = android.PathForModuleInstall(ctx, installDir, a.installApkName+".apk")
-	a.dexpreopter.isInstallable = Bool(a.properties.Installable)
 	a.dexpreopter.uncompressedDex = a.shouldUncompressDex(ctx)
 
 	a.dexpreopter.enforceUsesLibs = a.usesLibrary.enforceUsesLibraries()
@@ -876,7 +878,6 @@ type AndroidAppImport struct {
 	android.ModuleBase
 	android.DefaultableModuleBase
 	prebuilt android.Prebuilt
-	dexpreopt.DexPreoptModule
 
 	properties   AndroidAppImportProperties
 	dpiVariants  interface{}
@@ -924,6 +925,10 @@ type AndroidAppImportProperties struct {
 	Filename *string
 }
 
+func (a *AndroidAppImport) IsInstallable() bool {
+	return true
+}
+
 // Updates properties with variant-specific values.
 func (a *AndroidAppImport) processVariants(ctx android.LoadHookContext) {
 	config := ctx.Config()
@@ -1066,7 +1071,6 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext
 	}
 
 	a.dexpreopter.installPath = installDir.Join(ctx, a.BaseModuleName()+".apk")
-	a.dexpreopter.isInstallable = true
 	a.dexpreopter.isPresignedPrebuilt = Bool(a.properties.Presigned)
 	a.dexpreopter.uncompressedDex = a.shouldUncompressDex(ctx)
 
diff --git a/java/dexpreopt.go b/java/dexpreopt.go
index 2b87cf797..4313964aa 100644
--- a/java/dexpreopt.go
+++ b/java/dexpreopt.go
@@ -19,6 +19,11 @@ import (
 	"android/soong/dexpreopt"
 )
 
+type dexpreopterInterface interface {
+	IsInstallable() bool // Structs that embed dexpreopter must implement this.
+	dexpreoptDisabled(ctx android.BaseModuleContext) bool
+}
+
 type dexpreopter struct {
 	dexpreoptProperties DexpreoptProperties
 
@@ -26,7 +31,6 @@ type dexpreopter struct {
 	uncompressedDex     bool
 	isSDKLibrary        bool
 	isTest              bool
-	isInstallable       bool
 	isPresignedPrebuilt bool
 
 	manifestFile     android.Path
@@ -58,7 +62,7 @@ type DexpreoptProperties struct {
 	}
 }
 
-func (d *dexpreopter) dexpreoptDisabled(ctx android.ModuleContext) bool {
+func (d *dexpreopter) dexpreoptDisabled(ctx android.BaseModuleContext) bool {
 	global := dexpreopt.GetGlobalConfig(ctx)
 
 	if global.DisablePreopt {
@@ -81,7 +85,11 @@ func (d *dexpreopter) dexpreoptDisabled(ctx android.ModuleContext) bool {
 		return true
 	}
 
-	if !d.isInstallable {
+	if !ctx.Module().(dexpreopterInterface).IsInstallable() {
+		return true
+	}
+
+	if ctx.Host() {
 		return true
 	}
 
@@ -95,12 +103,23 @@ func (d *dexpreopter) dexpreoptDisabled(ctx android.ModuleContext) bool {
 	return false
 }
 
+func dexpreoptToolDepsMutator(ctx android.BottomUpMutatorContext) {
+	if d, ok := ctx.Module().(dexpreopterInterface); !ok || d.dexpreoptDisabled(ctx) {
+		return
+	}
+	dexpreopt.RegisterToolDeps(ctx)
+}
+
 func odexOnSystemOther(ctx android.ModuleContext, installPath android.InstallPath) bool {
 	return dexpreopt.OdexOnSystemOtherByName(ctx.ModuleName(), android.InstallPathToOnDevicePath(ctx, installPath), dexpreopt.GetGlobalConfig(ctx))
 }
 
 func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.ModuleOutPath) android.ModuleOutPath {
-	if d.dexpreoptDisabled(ctx) {
+	// TODO(b/148690468): The check on d.installPath is to bail out in cases where
+	// the dexpreopter struct hasn't been fully initialized before we're called,
+	// e.g. in aar.go. This keeps the behaviour that dexpreopting is effectively
+	// disabled, even if installable is true.
+	if d.dexpreoptDisabled(ctx) || d.installPath.Base() == "." {
 		return dexJarFile
 	}
 
diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go
index 2a2e6dae5..655a47644 100644
--- a/java/dexpreopt_bootjars.go
+++ b/java/dexpreopt_bootjars.go
@@ -205,6 +205,10 @@ func (d *dexpreoptBootJars) GenerateBuildActions(ctx android.SingletonContext) {
 	if skipDexpreoptBootJars(ctx) {
 		return
 	}
+	if dexpreopt.GetCachedGlobalSoongConfig(ctx) == nil {
+		// No module has enabled dexpreopting, so we assume there will be no boot image to make.
+		return
+	}
 
 	d.dexpreoptConfigForMake = android.PathForOutput(ctx, ctx.Config().DeviceName(), "dexpreopt.config")
 	writeGlobalConfigForMake(ctx, d.dexpreoptConfigForMake)
diff --git a/java/java.go b/java/java.go
index 2e53654cc..ceedd8971 100644
--- a/java/java.go
+++ b/java/java.go
@@ -76,7 +76,9 @@ func RegisterJavaBuildComponents(ctx android.RegistrationContext) {
 	ctx.RegisterModuleType("java_host_for_device", HostForDeviceFactory)
 	ctx.RegisterModuleType("dex_import", DexImportFactory)
 
-	ctx.FinalDepsMutators(dexpreopt.RegisterToolDepsMutator)
+	ctx.FinalDepsMutators(func(ctx android.RegisterMutatorsContext) {
+		ctx.BottomUp("dexpreopt_tool_deps", dexpreoptToolDepsMutator).Parallel()
+	})
 
 	ctx.RegisterSingletonType("logtags", LogtagsSingleton)
 	ctx.RegisterSingletonType("kythe_java_extract", kytheExtractJavaFactory)
@@ -371,7 +373,6 @@ type Module struct {
 	android.DefaultableModuleBase
 	android.ApexModuleBase
 	android.SdkBase
-	dexpreopt.DexPreoptModule
 
 	properties       CompilerProperties
 	protoProperties  android.ProtoProperties
@@ -1577,16 +1578,6 @@ func (j *Module) compile(ctx android.ModuleContext, aaptSrcJar android.Path) {
 		}
 	} else {
 		outputFile = implementationAndResourcesJar
-
-		// dexpreopt.GetGlobalSoongConfig needs to be called at least once even if
-		// no module actually is dexpreopted, to ensure there's a cached
-		// GlobalSoongConfig for the dexpreopt singletons, which will run
-		// regardless.
-		// TODO(b/147613152): Remove when the singletons no longer rely on the
-		// cached GlobalSoongConfig.
-		if !dexpreopt.GetGlobalConfig(ctx).DisablePreopt {
-			_ = dexpreopt.GetGlobalSoongConfig(ctx)
-		}
 	}
 
 	ctx.CheckbuildFile(outputFile)
@@ -1794,6 +1785,10 @@ func (j *Module) JacocoReportClassesFile() android.Path {
 	return j.jacocoReportClassesFile
 }
 
+func (j *Module) IsInstallable() bool {
+	return Bool(j.properties.Installable)
+}
+
 //
 // Java libraries (.jar file)
 //
@@ -1831,7 +1826,6 @@ func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	j.checkSdkVersion(ctx)
 	j.dexpreopter.installPath = android.PathForModuleInstall(ctx, "framework", j.Stem()+".jar")
 	j.dexpreopter.isSDKLibrary = j.deviceProperties.IsSDKLibrary
-	j.dexpreopter.isInstallable = Bool(j.properties.Installable)
 	j.dexpreopter.uncompressedDex = shouldUncompressDex(ctx, &j.dexpreopter)
 	j.deviceProperties.UncompressDex = j.dexpreopter.uncompressedDex
 	j.compile(ctx, nil)
@@ -2353,7 +2347,6 @@ type Import struct {
 	android.ApexModuleBase
 	prebuilt android.Prebuilt
 	android.SdkBase
-	dexpreopt.DexPreoptModule
 
 	properties ImportProperties
 
@@ -2564,7 +2557,6 @@ type DexImport struct {
 	android.DefaultableModuleBase
 	android.ApexModuleBase
 	prebuilt android.Prebuilt
-	dexpreopt.DexPreoptModule
 
 	properties DexImportProperties
 
@@ -2590,13 +2582,16 @@ func (j *DexImport) Stem() string {
 	return proptools.StringDefault(j.properties.Stem, j.ModuleBase.Name())
 }
 
+func (j *DexImport) IsInstallable() bool {
+	return true
+}
+
 func (j *DexImport) GenerateAndroidBuildActions(ctx android.ModuleContext) {
 	if len(j.properties.Jars) != 1 {
 		ctx.PropertyErrorf("jars", "exactly one jar must be provided")
 	}
 
 	j.dexpreopter.installPath = android.PathForModuleInstall(ctx, "framework", j.Stem()+".jar")
-	j.dexpreopter.isInstallable = true
 	j.dexpreopter.uncompressedDex = shouldUncompressDex(ctx, &j.dexpreopter)
 
 	inputJar := ctx.ExpandSource(j.properties.Jars[0], "jars")