Merge changes from topic "revert-1211982-dex2oat-soong-dep-LLLKNULXYJ"

* changes:
  Revert "Separate dexpreopt.GlobalSoongConfig to allow independen..."
  Revert "Move the Once cache for dexpreopt.GlobalConfig into the ..."
  Revert "Get the dex2oat host tool path from module dependency on..."

dexpreopt/Android.bp

@@ -4,7 +4,6 @@ bootstrap_go_package {
     srcs: [
         "config.go",
         "dexpreopt.go",
-        "testing.go",
     ],
     testSrcs: [
         "dexpreopt_test.go",

dexpreopt/config.go

@@ -16,16 +16,14 @@ package dexpreopt
 
 import (
 	"encoding/json"
-	"fmt"
 	"strings"
 
-	"github.com/google/blueprint"
-
 	"android/soong/android"
 )
 
 // GlobalConfig stores the configuration for dex preopting. The fields are set
-// from product variables via dex_preopt_config.mk.
+// from product variables via dex_preopt_config.mk, except for SoongConfig
+// which come from CreateGlobalSoongConfig.
 type GlobalConfig struct {
 	DisablePreopt        bool     // disable preopt for all modules
 	DisablePreoptModules []string // modules with preopt disabled by product-specific config
@@ -84,6 +82,8 @@ 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,9 +179,12 @@ func constructWritablePath(ctx android.PathContext, path string) android.Writabl
 	return constructPath(ctx, path).(android.WritablePath)
 }
 
-// ParseGlobalConfig parses the given data assumed to be read from the global
+// LoadGlobalConfig reads the global dexpreopt.config file into a GlobalConfig
-// dexpreopt.config file into a GlobalConfig struct.
+// struct, except the SoongConfig field which is set from the provided
-func ParseGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, error) {
+// 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) {
 	type GlobalJSONConfig struct {
 		GlobalConfig
 
@@ -201,68 +204,17 @@ func ParseGlobalConfig(ctx android.PathContext, data []byte) (GlobalConfig, erro
 	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
 }
 
-type globalConfigAndRaw struct {
+// LoadModuleConfig reads a per-module dexpreopt.config file into a ModuleConfig struct.  It is not used in Soong, which
-	global GlobalConfig
+// receives a ModuleConfig struct directly from java/dexpreopt.go.  It is used in dexpreopt_gen called from oMake to
-	data   []byte
+// read the module dexpreopt.config written by Make.
-}
+func LoadModuleConfig(ctx android.PathContext, data []byte) (ModuleConfig, error) {
-
-// 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
 
@@ -301,84 +253,21 @@ 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
+// CreateGlobalSoongConfig creates a GlobalSoongConfig from the current context.
-// 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 {
+func CreateGlobalSoongConfig(ctx android.PathContext) GlobalSoongConfig {
-	if ctx.Config().TestProductVariables != nil {
+	// Default to debug version to help find bugs.
-		// If we're called in a test there'll be a confusing error from the path
+	// Set USE_DEX2OAT_DEBUG to false for only building non-debug versions.
-		// functions below that gets reported without a stack trace, so let's panic
+	var dex2oatBinary string
-		// properly with a more helpful message.
+	if ctx.Config().Getenv("USE_DEX2OAT_DEBUG") == "false" {
-		panic("This should not be called from tests. Please call GlobalSoongConfigForTests somewhere in the test setup.")
+		dex2oatBinary = "dex2oat"
+	} else {
+		dex2oatBinary = "dex2oatd"
 	}
 
 	return GlobalSoongConfig{
 		Profman:          ctx.Config().HostToolPath(ctx, "profman"),
-		Dex2oat:          dex2oatPathFromDep(ctx),
+		Dex2oat:          ctx.Config().HostToolPath(ctx, dex2oatBinary),
 		Aapt:             ctx.Config().HostToolPath(ctx, "aapt"),
 		SoongZip:         ctx.Config().HostToolPath(ctx, "soong_zip"),
 		Zip2zip:          ctx.Config().HostToolPath(ctx, "zip2zip"),
@@ -387,44 +276,6 @@ 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,
-// and later returns the same cached instance.
-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
-}
-
-// 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
@@ -435,10 +286,9 @@ type globalJsonSoongConfig struct {
 	ConstructContext string
 }
 
-// ParseGlobalSoongConfig parses the given data assumed to be read from the
+// LoadGlobalSoongConfig reads the dexpreopt_soong.config file into a
-// global dexpreopt_soong.config file into a GlobalSoongConfig struct. It is
+// GlobalSoongConfig struct. It is only used in dexpreopt_gen.
-// only used in dexpreopt_gen.
+func LoadGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongConfig, error) {
-func ParseGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongConfig, error) {
 	var jc globalJsonSoongConfig
 
 	err := json.Unmarshal(data, &jc)
@@ -460,11 +310,7 @@ func ParseGlobalSoongConfig(ctx android.PathContext, data []byte) (GlobalSoongCo
 }
 
 func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonContext) {
-	if GetGlobalConfig(ctx).DisablePreopt {
+	config := CreateGlobalSoongConfig(ctx)
-		return
-	}
-
-	config := GetCachedGlobalSoongConfig(ctx)
 	jc := globalJsonSoongConfig{
 		Profman:          config.Profman.String(),
 		Dex2oat:          config.Dex2oat.String(),
@@ -491,11 +337,7 @@ func (s *globalSoongConfigSingleton) GenerateBuildActions(ctx android.SingletonC
 }
 
 func (s *globalSoongConfigSingleton) MakeVars(ctx android.MakeVarsContext) {
-	if GetGlobalConfig(ctx).DisablePreopt {
+	config := CreateGlobalSoongConfig(ctx)
-		return
-	}
-
-	config := GetCachedGlobalSoongConfig(ctx)
 
 	ctx.Strict("DEX2OAT", config.Dex2oat.String())
 	ctx.Strict("DEXPREOPT_GEN_DEPS", strings.Join([]string{
@@ -548,14 +390,7 @@ 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"),
@@ -563,6 +398,6 @@ func GlobalSoongConfigForTests(config android.Config) GlobalSoongConfig {
 			Zip2zip:          android.PathForTesting("zip2zip"),
 			ManifestCheck:    android.PathForTesting("manifest_check"),
 			ConstructContext: android.PathForTesting("construct_context.sh"),
-		}
+		},
-	}).(GlobalSoongConfig)
+	}
 }

dexpreopt/dexpreopt.go

@@ -49,7 +49,7 @@ const SystemOtherPartition = "/system_other/"
 
 // 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,
+func GenerateDexpreoptRule(ctx android.PathContext,
 	global GlobalConfig, module ModuleConfig) (rule *android.RuleBuilder, err error) {
 
 	defer func() {
@@ -72,10 +72,10 @@ func GenerateDexpreoptRule(ctx android.PathContext, globalSoong GlobalSoongConfi
 
 	var profile android.WritablePath
 	if generateProfile {
-		profile = profileCommand(ctx, globalSoong, global, module, rule)
+		profile = profileCommand(ctx, global, module, rule)
 	}
 	if generateBootProfile {
-		bootProfileCommand(ctx, globalSoong, global, module, rule)
+		bootProfileCommand(ctx, global, module, rule)
 	}
 
 	if !dexpreoptDisabled(global, module) {
@@ -87,7 +87,7 @@ func GenerateDexpreoptRule(ctx android.PathContext, globalSoong GlobalSoongConfi
 			generateDM := shouldGenerateDM(module, global)
 
 			for archIdx, _ := range module.Archs {
-				dexpreoptCommand(ctx, globalSoong, global, module, rule, archIdx, profile, appImage, generateDM)
+				dexpreoptCommand(ctx, global, module, rule, archIdx, profile, appImage, generateDM)
 			}
 		}
 	}
@@ -119,8 +119,8 @@ func dexpreoptDisabled(global GlobalConfig, module ModuleConfig) bool {
 	return false
 }
 
-func profileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
+func profileCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig,
-	module ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
+	rule *android.RuleBuilder) android.WritablePath {
 
 	profilePath := module.BuildPath.InSameDir(ctx, "profile.prof")
 	profileInstalledPath := module.DexLocation + ".prof"
@@ -131,7 +131,7 @@ func profileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, glob
 
 	cmd := rule.Command().
 		Text(`ANDROID_LOG_TAGS="*:e"`).
-		Tool(globalSoong.Profman)
+		Tool(global.SoongConfig.Profman)
 
 	if module.ProfileIsTextListing {
 		// The profile is a test listing of classes (used for framework jars).
@@ -158,8 +158,8 @@ func profileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, glob
 	return profilePath
 }
 
-func bootProfileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
+func bootProfileCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig,
-	module ModuleConfig, rule *android.RuleBuilder) android.WritablePath {
+	rule *android.RuleBuilder) android.WritablePath {
 
 	profilePath := module.BuildPath.InSameDir(ctx, "profile.bprof")
 	profileInstalledPath := module.DexLocation + ".bprof"
@@ -170,7 +170,7 @@ func bootProfileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig,
 
 	cmd := rule.Command().
 		Text(`ANDROID_LOG_TAGS="*:e"`).
-		Tool(globalSoong.Profman)
+		Tool(global.SoongConfig.Profman)
 
 	// The profile is a test listing of methods.
 	// We need to generate the actual binary profile.
@@ -190,9 +190,8 @@ func bootProfileCommand(ctx android.PathContext, globalSoong GlobalSoongConfig,
 	return profilePath
 }
 
-func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, global GlobalConfig,
+func dexpreoptCommand(ctx android.PathContext, global GlobalConfig, module ModuleConfig, rule *android.RuleBuilder,
-	module ModuleConfig, rule *android.RuleBuilder, archIdx int, profile android.WritablePath,
+	archIdx int, profile android.WritablePath, appImage bool, generateDM bool) {
-	appImage bool, generateDM bool) {
 
 	arch := module.Archs[archIdx]
 
@@ -300,14 +299,14 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, gl
 	if module.EnforceUsesLibraries {
 		if module.ManifestPath != nil {
 			rule.Command().Text(`target_sdk_version="$(`).
-				Tool(globalSoong.ManifestCheck).
+				Tool(global.SoongConfig.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(globalSoong.Aapt).
+				Tool(global.SoongConfig.Aapt).
 				Flag("dump badging").
 				Input(module.DexPath).
 				Text(`| grep "targetSdkVersion" | sed -n "s/targetSdkVersion:'\(.*\)'/\1/p"`).
@@ -328,7 +327,7 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, gl
 			Implicits(conditionalClassLoaderContextHost29)
 		rule.Command().Textf(`conditional_target_libs_29="%s"`,
 			strings.Join(conditionalClassLoaderContextTarget29, " "))
-		rule.Command().Text("source").Tool(globalSoong.ConstructContext).Input(module.DexPath)
+		rule.Command().Text("source").Tool(global.SoongConfig.ConstructContext).Input(module.DexPath)
 	}
 
 	// Devices that do not have a product partition use a symlink from /product to /system/product.
@@ -341,7 +340,7 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, gl
 
 	cmd := rule.Command().
 		Text(`ANDROID_LOG_TAGS="*:e"`).
-		Tool(globalSoong.Dex2oat).
+		Tool(global.SoongConfig.Dex2oat).
 		Flag("--avoid-storing-invocation").
 		FlagWithOutput("--write-invocation-to=", invocationPath).ImplicitOutput(invocationPath).
 		Flag("--runtime-arg").FlagWithArg("-Xms", global.Dex2oatXms).
@@ -410,7 +409,7 @@ func dexpreoptCommand(ctx android.PathContext, globalSoong GlobalSoongConfig, gl
 		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(globalSoong.SoongZip).
+		rule.Command().Tool(global.SoongConfig.SoongZip).
 			FlagWithArg("-L", "9").
 			FlagWithOutput("-o", dmPath).
 			Flag("-j").

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 Soong config %q: %s\n", *globalSoongConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error reading global config %q: %s\n", *globalSoongConfigPath, err)
 		os.Exit(2)
 	}
 
-	globalSoongConfig, err := dexpreopt.ParseGlobalSoongConfig(ctx, globalSoongConfigData)
+	globalSoongConfig, err := dexpreopt.LoadGlobalSoongConfig(ctx, globalSoongConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error parsing global Soong config %q: %s\n", *globalSoongConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error loading global config %q: %s\n", *globalSoongConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -96,9 +96,9 @@ func main() {
 		os.Exit(2)
 	}
 
-	globalConfig, err := dexpreopt.ParseGlobalConfig(ctx, globalConfigData)
+	globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, globalConfigData, globalSoongConfig)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error parsing global config %q: %s\n", *globalConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error parse global config %q: %s\n", *globalConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -108,9 +108,9 @@ func main() {
 		os.Exit(2)
 	}
 
-	moduleConfig, err := dexpreopt.ParseModuleConfig(ctx, moduleConfigData)
+	moduleConfig, err := dexpreopt.LoadModuleConfig(ctx, moduleConfigData)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "error parsing module config %q: %s\n", *moduleConfigPath, err)
+		fmt.Fprintf(os.Stderr, "error loading module config %q: %s\n", *moduleConfigPath, err)
 		os.Exit(2)
 	}
 
@@ -130,12 +130,12 @@ func main() {
 		}
 	}()
 
-	writeScripts(ctx, globalSoongConfig, globalConfig, moduleConfig, *dexpreoptScriptPath)
+	writeScripts(ctx, globalConfig, moduleConfig, *dexpreoptScriptPath)
 }
 
-func writeScripts(ctx android.PathContext, globalSoong dexpreopt.GlobalSoongConfig,
+func writeScripts(ctx android.PathContext, global dexpreopt.GlobalConfig, module dexpreopt.ModuleConfig,
-	global dexpreopt.GlobalConfig, module dexpreopt.ModuleConfig, dexpreoptScriptPath string) {
+	dexpreoptScriptPath string) {
-	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, globalSoong, global, module)
+	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, global, module)
 	if err != nil {
 		panic(err)
 	}
@@ -150,7 +150,7 @@ func writeScripts(ctx android.PathContext, globalSoong dexpreopt.GlobalSoongConf
 		dexpreoptRule.Command().Text("mkdir -p").Flag(filepath.Dir(installPath.String()))
 		dexpreoptRule.Command().Text("cp -f").Input(install.From).Output(installPath)
 	}
-	dexpreoptRule.Command().Tool(globalSoong.SoongZip).
+	dexpreoptRule.Command().Tool(global.SoongConfig.SoongZip).
 		FlagWithArg("-o ", "$2").
 		FlagWithArg("-C ", installDir.String()).
 		FlagWithArg("-D ", installDir.String())

dexpreopt/dexpreopt_test.go

@@ -61,13 +61,10 @@ func testModuleConfig(ctx android.PathContext, name, partition string) ModuleCon
 }
 
 func TestDexPreopt(t *testing.T) {
-	config := android.TestConfig("out", nil, "", nil)
+	ctx := android.PathContextForTesting(android.TestConfig("out", nil, "", nil))
-	ctx := android.PathContextForTesting(config)
+	global, module := GlobalConfigForTests(ctx), testSystemModuleConfig(ctx, "test")
-	globalSoong := GlobalSoongConfigForTests(config)
-	global := GlobalConfigForTests(ctx)
-	module := testSystemModuleConfig(ctx, "test")
 
-	rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module)
+	rule, err := GenerateDexpreoptRule(ctx, global, module)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -83,9 +80,7 @@ func TestDexPreopt(t *testing.T) {
 }
 
 func TestDexPreoptSystemOther(t *testing.T) {
-	config := android.TestConfig("out", nil, "", nil)
+	ctx := android.PathContextForTesting(android.TestConfig("out", nil, "", nil))
-	ctx := android.PathContextForTesting(config)
-	globalSoong := GlobalSoongConfigForTests(config)
 	global := GlobalConfigForTests(ctx)
 	systemModule := testSystemModuleConfig(ctx, "Stest")
 	systemProductModule := testSystemProductModuleConfig(ctx, "SPtest")
@@ -123,7 +118,7 @@ func TestDexPreoptSystemOther(t *testing.T) {
 	for _, test := range tests {
 		global.PatternsOnSystemOther = test.patterns
 		for _, mt := range test.moduleTests {
-			rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, mt.module)
+			rule, err := GenerateDexpreoptRule(ctx, global, mt.module)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -143,15 +138,12 @@ func TestDexPreoptSystemOther(t *testing.T) {
 }
 
 func TestDexPreoptProfile(t *testing.T) {
-	config := android.TestConfig("out", nil, "", nil)
+	ctx := android.PathContextForTesting(android.TestConfig("out", nil, "", nil))
-	ctx := android.PathContextForTesting(config)
+	global, module := GlobalConfigForTests(ctx), testSystemModuleConfig(ctx, "test")
-	globalSoong := GlobalSoongConfigForTests(config)
-	global := GlobalConfigForTests(ctx)
-	module := testSystemModuleConfig(ctx, "test")
 
 	module.ProfileClassListing = android.OptionalPathForPath(android.PathForTesting("profile"))
 
-	rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module)
+	rule, err := GenerateDexpreoptRule(ctx, global, module)
 	if err != nil {
 		t.Fatal(err)
 	}

dexpreopt/testing.go

@@ -1,47 +0,0 @@
-// 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",
-		}
-	`
-}

java/app.go

@@ -27,7 +27,6 @@ import (
 
 	"android/soong/android"
 	"android/soong/cc"
-	"android/soong/dexpreopt"
 	"android/soong/tradefed"
 )
 
@@ -851,7 +850,6 @@ type AndroidAppImport struct {
 	android.ModuleBase
 	android.DefaultableModuleBase
 	prebuilt android.Prebuilt
-	dexpreopt.DexPreoptModule
 
 	properties   AndroidAppImportProperties
 	dpiVariants  interface{}

java/dexpreopt.go

@@ -59,7 +59,7 @@ type DexpreoptProperties struct {
 }
 
 func (d *dexpreopter) dexpreoptDisabled(ctx android.ModuleContext) bool {
-	global := dexpreopt.GetGlobalConfig(ctx)
+	global := dexpreoptGlobalConfig(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), dexpreopt.GetGlobalConfig(ctx))
+	return dexpreopt.OdexOnSystemOtherByName(ctx.ModuleName(), android.InstallPathToOnDevicePath(ctx, installPath), dexpreoptGlobalConfig(ctx))
 }
 
 func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.ModuleOutPath) android.ModuleOutPath {
@@ -104,8 +104,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 		return dexJarFile
 	}
 
-	globalSoong := dexpreopt.GetGlobalSoongConfig(ctx)
+	global := dexpreoptGlobalConfig(ctx)
-	global := dexpreopt.GetGlobalConfig(ctx)
 	bootImage := defaultBootImageConfig(ctx)
 	if global.UseApexImage {
 		bootImage = frameworkJZBootImageConfig(ctx)
@@ -190,7 +189,7 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, dexJarFile android.Mo
 		PresignedPrebuilt: d.isPresignedPrebuilt,
 	}
 
-	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, globalSoong, global, dexpreoptConfig)
+	dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule(ctx, global, dexpreoptConfig)
 	if err != nil {
 		ctx.ModuleErrorf("error generating dexpreopt rule: %s", err.Error())
 		return dexJarFile

java/dexpreopt_bootjars.go

@@ -162,7 +162,7 @@ func dexpreoptBootJarsFactory() android.Singleton {
 }
 
 func skipDexpreoptBootJars(ctx android.PathContext) bool {
-	if dexpreopt.GetGlobalConfig(ctx).DisablePreopt {
+	if dexpreoptGlobalConfig(ctx).DisablePreopt {
 		return true
 	}
 
@@ -195,7 +195,7 @@ func DexpreoptedArtApexJars(ctx android.BuilderContext) map[android.ArchType]and
 	files := artBootImageConfig(ctx).imagesDeps
 
 	// For JIT-zygote config, also include dexpreopt files for the primary JIT-zygote image.
-	if dexpreopt.GetGlobalConfig(ctx).UseApexImage {
+	if dexpreoptGlobalConfig(ctx).UseApexImage {
 		for arch, paths := range artJZBootImageConfig(ctx).imagesDeps {
 			files[arch] = append(files[arch], paths...)
 		}
@@ -213,7 +213,7 @@ func (d *dexpreoptBootJars) GenerateBuildActions(ctx android.SingletonContext) {
 	d.dexpreoptConfigForMake = android.PathForOutput(ctx, ctx.Config().DeviceName(), "dexpreopt.config")
 	writeGlobalConfigForMake(ctx, d.dexpreoptConfigForMake)
 
-	global := dexpreopt.GetGlobalConfig(ctx)
+	global := dexpreoptGlobalConfig(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.
@@ -304,8 +304,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)
-	global := dexpreopt.GetGlobalConfig(ctx)
 
 	symbolsDir := image.symbolsDir.Join(ctx, image.installSubdir, arch.String())
 	symbolsFile := symbolsDir.Join(ctx, image.stem+".oat")
@@ -340,7 +339,7 @@ func buildBootImageRuleForArch(ctx android.SingletonContext, image *bootImage,
 
 	invocationPath := outputPath.ReplaceExtension(ctx, "invocation")
 
-	cmd.Tool(globalSoong.Dex2oat).
+	cmd.Tool(global.SoongConfig.Dex2oat).
 		Flag("--avoid-storing-invocation").
 		FlagWithOutput("--write-invocation-to=", invocationPath).ImplicitOutput(invocationPath).
 		Flag("--runtime-arg").FlagWithArg("-Xms", global.Dex2oatImageXms).
@@ -443,8 +442,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)
-	global := dexpreopt.GetGlobalConfig(ctx)
 
 	if global.DisableGenerateProfile || ctx.Config().IsPdkBuild() || ctx.Config().UnbundledBuild() {
 		return nil
@@ -475,7 +473,7 @@ func bootImageProfileRule(ctx android.SingletonContext, image *bootImage, missin
 
 		rule.Command().
 			Text(`ANDROID_LOG_TAGS="*:e"`).
-			Tool(globalSoong.Profman).
+			Tool(global.SoongConfig.Profman).
 			FlagWithInput("--create-profile-from=", bootImageProfile).
 			FlagForEachInput("--apk=", image.dexPathsDeps.Paths()).
 			FlagForEachArg("--dex-location=", image.dexLocationsDeps).
@@ -498,8 +496,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)
-	global := dexpreopt.GetGlobalConfig(ctx)
 
 	if global.DisableGenerateProfile || ctx.Config().IsPdkBuild() || ctx.Config().UnbundledBuild() {
 		return nil
@@ -525,7 +522,7 @@ func bootFrameworkProfileRule(ctx android.SingletonContext, image *bootImage, mi
 
 		rule.Command().
 			Text(`ANDROID_LOG_TAGS="*:e"`).
-			Tool(globalSoong.Profman).
+			Tool(global.SoongConfig.Profman).
 			Flag("--generate-boot-profile").
 			FlagWithInput("--create-profile-from=", bootFrameworkProfile).
 			FlagForEachInput("--apk=", image.dexPathsDeps.Paths()).
@@ -587,7 +584,7 @@ func dumpOatRules(ctx android.SingletonContext, image *bootImage) {
 }
 
 func writeGlobalConfigForMake(ctx android.SingletonContext, path android.WritablePath) {
-	data := dexpreopt.GetGlobalConfigRawData(ctx)
+	data := dexpreoptGlobalConfigRaw(ctx).data
 
 	ctx.Build(pctx, android.BuildParams{
 		Rule:   android.WriteFile,

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"}
-	dexpreopt.SetTestGlobalConfig(config, dexpreoptConfig)
+	setDexpreoptTestGlobalConfig(config, dexpreoptConfig)
 
 	ctx := testContext()
 

java/dexpreopt_config.go

@@ -22,12 +22,57 @@ 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 {
+			soongConfig := dexpreopt.CreateGlobalSoongConfig(ctx)
+			globalConfig, err := dexpreopt.LoadGlobalConfig(ctx, data, soongConfig)
+			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.PathContext) []string {
 	return ctx.Config().OnceStringSlice(systemServerClasspathKey, func() []string {
-		global := dexpreopt.GetGlobalConfig(ctx)
+		global := dexpreoptGlobalConfig(ctx)
 
 		var systemServerClasspathLocations []string
 		for _, m := range global.SystemServerJars {
@@ -88,7 +133,7 @@ var (
 func genBootImageConfigs(ctx android.PathContext) map[string]*bootImageConfig {
 	return ctx.Config().Once(bootImageConfigKey, func() interface{} {
 
-		global := dexpreopt.GetGlobalConfig(ctx)
+		global := dexpreoptGlobalConfig(ctx)
 		targets := dexpreoptTargets(ctx)
 		deviceDir := android.PathForOutput(ctx, ctx.Config().DeviceName())
 
@@ -229,7 +274,7 @@ func frameworkJZBootImageConfig(ctx android.PathContext) bootImageConfig {
 
 func defaultBootclasspath(ctx android.PathContext) []string {
 	return ctx.Config().OnceStringSlice(defaultBootclasspathKey, func() []string {
-		global := dexpreopt.GetGlobalConfig(ctx)
+		global := dexpreoptGlobalConfig(ctx)
 		image := defaultBootImageConfig(ctx)
 
 		updatableBootclasspath := make([]string, len(global.UpdatableBootJars))

java/java.go

@@ -29,7 +29,6 @@ import (
 	"github.com/google/blueprint/proptools"
 
 	"android/soong/android"
-	"android/soong/dexpreopt"
 	"android/soong/java/config"
 	"android/soong/tradefed"
 )
@@ -80,8 +79,6 @@ 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)
 }
@@ -338,7 +335,6 @@ type Module struct {
 	android.DefaultableModuleBase
 	android.ApexModuleBase
 	android.SdkBase
-	dexpreopt.DexPreoptModule
 
 	properties       CompilerProperties
 	protoProperties  android.ProtoProperties
@@ -1529,16 +1525,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)
@@ -2289,7 +2275,6 @@ type Import struct {
 	android.ApexModuleBase
 	prebuilt android.Prebuilt
 	android.SdkBase
-	dexpreopt.DexPreoptModule
 
 	properties ImportProperties
 
@@ -2492,7 +2477,6 @@ type DexImport struct {
 	android.DefaultableModuleBase
 	android.ApexModuleBase
 	prebuilt android.Prebuilt
-	dexpreopt.DexPreoptModule
 
 	properties DexImportProperties
 

java/java_test.go

@@ -57,15 +57,7 @@ func TestMain(m *testing.M) {
 }
 
 func testConfig(env map[string]string, bp string, fs map[string][]byte) android.Config {
-	bp += dexpreopt.BpToolModulesForTest()
+	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 {
@@ -94,8 +86,6 @@ func testContext() *android.TestContext {
 	cc.RegisterRequiredBuildComponentsForTest(ctx)
 	ctx.RegisterModuleType("ndk_prebuilt_shared_stl", cc.NdkPrebuiltSharedStlFactory)
 
-	dexpreopt.RegisterToolModulesForTest(ctx)
-
 	return ctx
 }
 
@@ -103,7 +93,7 @@ func run(t *testing.T, ctx *android.TestContext, config android.Config) {
 	t.Helper()
 
 	pathCtx := android.PathContextForTesting(config)
-	dexpreopt.SetTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
+	setDexpreoptTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
 
 	ctx.Register(config)
 	_, errs := ctx.ParseBlueprintsFiles("Android.bp")
@@ -122,7 +112,7 @@ func testJavaErrorWithConfig(t *testing.T, pattern string, config android.Config
 	ctx := testContext()
 
 	pathCtx := android.PathContextForTesting(config)
-	dexpreopt.SetTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
+	setDexpreoptTestGlobalConfig(config, dexpreopt.GlobalConfigForTests(pathCtx))
 
 	ctx.Register(config)
 	_, errs := ctx.ParseBlueprintsFiles("Android.bp")