From 7b2e6f3ed7c8b871195f344d0efeecb59a202e8f Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Sat, 25 Jan 2020 01:44:30 +0000 Subject: [PATCH] Revert "Get the dex2oat host tool path from module dependency on..." Revert submission 1211982-dex2oat-soong-dep Reason for revert: Build failures. See b/148312086. Reverted Changes: Ibc427a9a8: Make dex2oat(d) visible for use as implicit dexpre... I71df11c1e: Move the Once cache for dexpreopt.GlobalConfig int... I38317f2d5: Get the dex2oat host tool path from module depende... I440a09dba: Separate dexpreopt.GlobalSoongConfig to allow inde... Bug: 148312086 Bug: 145934348 Exempt-From-Owner-Approval: Plain revert Change-Id: I0cf6fef04434fd3c0b7957ee9abdcaabeb20b9d8 --- dexpreopt/Android.bp | 1 - dexpreopt/config.go | 105 +++++-------------------------------------- dexpreopt/testing.go | 47 ------------------- java/app.go | 2 - java/java.go | 16 ------- java/java_test.go | 4 -- 6 files changed, 10 insertions(+), 165 deletions(-) delete mode 100644 dexpreopt/testing.go diff --git a/dexpreopt/Android.bp b/dexpreopt/Android.bp index b8f7ea682..c5f24e273 100644 --- a/dexpreopt/Android.bp +++ b/dexpreopt/Android.bp @@ -4,7 +4,6 @@ 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 72c01d0b7..cd74ec847 100644 --- a/dexpreopt/config.go +++ b/dexpreopt/config.go @@ -16,11 +16,8 @@ package dexpreopt import ( "encoding/json" - "fmt" "strings" - "github.com/google/blueprint" - "android/soong/android" ) @@ -301,71 +298,6 @@ 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 { @@ -376,9 +308,18 @@ 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: 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,16 +328,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, @@ -405,14 +336,6 @@ 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 } @@ -460,10 +383,6 @@ 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(), @@ -491,10 +410,6 @@ 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 deleted file mode 100644 index b572eb351..000000000 --- a/dexpreopt/testing.go +++ /dev/null @@ -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", - } - ` -} diff --git a/java/app.go b/java/app.go index 4f060878c..c59047df5 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" ) @@ -851,7 +850,6 @@ 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 320cb7b55..4c6a5a5ac 100644 --- a/java/java.go +++ b/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 diff --git a/java/java_test.go b/java/java_test.go index b724b4de2..f047486e6 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -57,8 +57,6 @@ 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 @@ -94,8 +92,6 @@ func testContext() *android.TestContext { cc.RegisterRequiredBuildComponentsForTest(ctx) ctx.RegisterModuleType("ndk_prebuilt_shared_stl", cc.NdkPrebuiltSharedStlFactory) - dexpreopt.RegisterToolModulesForTest(ctx) - return ctx }