From 0780372ae82137817b7f352e066d65bd988fcc01 Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Tue, 18 May 2021 17:39:56 +0100 Subject: [PATCH 1/2] Use oatdump rather than oatdumpd for boot jar boot.*.oatdump.txt files. oatdumpd isn't available as a prebuilt. Test: m SOONG_CONFIG_art_module_source_build=false droid Bug: 172480615 Change-Id: I70317eb8f253272d629f23063cbe265d556caad3 --- java/dexpreopt_bootjars.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go index be202c0db..e9f7c44d2 100644 --- a/java/dexpreopt_bootjars.go +++ b/java/dexpreopt_bootjars.go @@ -803,8 +803,7 @@ func dumpOatRules(ctx android.ModuleContext, image *bootImageConfig) { rule := android.NewRuleBuilder(pctx, ctx) imageLocationsOnHost, _ := image.imageLocations() rule.Command(). - // TODO: for now, use the debug version for better error reporting - BuiltTool("oatdumpd"). + BuiltTool("oatdump"). FlagWithInputList("--runtime-arg -Xbootclasspath:", image.dexPathsDeps.Paths(), ":"). FlagWithList("--runtime-arg -Xbootclasspath-locations:", image.dexLocationsDeps, ":"). FlagWithArg("--image=", strings.Join(imageLocationsOnHost, ":")).Implicits(image.imagesDeps.Paths()). From 0e4ccebb1681ba464d4ae816347fbd86159d354c Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Thu, 13 May 2021 02:38:35 +0100 Subject: [PATCH 2/2] Don't fail if the target module is disabled in dex2oat tool dependencies. dexpreopt.RegisterToolDeps runs late after prebuilt dependencies have been resolved, and there's special code in dex2oatPathFromDep to resolve the prebuilt from the source module. However, if the source module is disabled then the dependencies check in validateAndroidModule will complain, so we need to disable that check in this particular situation. Also add a comment to explain why dexpreopt.RegisterToolDeps needs to run so late. Test: m nothing Bug: 145934348 Bug: 172480615 Change-Id: Ibc673303d0336768fa23261a2068e91a08f46a30 --- android/module.go | 31 ++++++++++++++++----------- dexpreopt/config.go | 10 +++++++++ java/dexpreopt_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++ java/java.go | 4 ++++ java/testing.go | 11 ++++++++-- 5 files changed, 90 insertions(+), 14 deletions(-) diff --git a/android/module.go b/android/module.go index c9a1662bb..69572dd23 100644 --- a/android/module.go +++ b/android/module.go @@ -1809,8 +1809,8 @@ func (m *ModuleBase) GenerateBuildActions(blueprintCtx blueprint.ModuleContext) if m.Enabled() { // ensure all direct android.Module deps are enabled ctx.VisitDirectDepsBlueprint(func(bm blueprint.Module) { - if _, ok := bm.(Module); ok { - ctx.validateAndroidModule(bm, ctx.baseModuleContext.strictVisitDeps) + if m, ok := bm.(Module); ok { + ctx.validateAndroidModule(bm, ctx.OtherModuleDependencyTag(m), ctx.baseModuleContext.strictVisitDeps) } }) @@ -2234,7 +2234,12 @@ func (b *baseModuleContext) AddMissingDependencies(deps []string) { } } -func (b *baseModuleContext) validateAndroidModule(module blueprint.Module, strict bool) Module { +type AllowDisabledModuleDependency interface { + blueprint.DependencyTag + AllowDisabledModuleDependency(target Module) bool +} + +func (b *baseModuleContext) validateAndroidModule(module blueprint.Module, tag blueprint.DependencyTag, strict bool) Module { aModule, _ := module.(Module) if !strict { @@ -2247,10 +2252,12 @@ func (b *baseModuleContext) validateAndroidModule(module blueprint.Module, stric } if !aModule.Enabled() { - if b.Config().AllowMissingDependencies() { - b.AddMissingDependencies([]string{b.OtherModuleName(aModule)}) - } else { - b.ModuleErrorf("depends on disabled module %q", b.OtherModuleName(aModule)) + if t, ok := tag.(AllowDisabledModuleDependency); !ok || !t.AllowDisabledModuleDependency(aModule) { + if b.Config().AllowMissingDependencies() { + b.AddMissingDependencies([]string{b.OtherModuleName(aModule)}) + } else { + b.ModuleErrorf("depends on disabled module %q", b.OtherModuleName(aModule)) + } } return nil } @@ -2343,7 +2350,7 @@ func (b *baseModuleContext) VisitDirectDepsBlueprint(visit func(blueprint.Module func (b *baseModuleContext) VisitDirectDeps(visit func(Module)) { b.bp.VisitDirectDeps(func(module blueprint.Module) { - if aModule := b.validateAndroidModule(module, b.strictVisitDeps); aModule != nil { + if aModule := b.validateAndroidModule(module, b.bp.OtherModuleDependencyTag(module), b.strictVisitDeps); aModule != nil { visit(aModule) } }) @@ -2351,7 +2358,7 @@ func (b *baseModuleContext) VisitDirectDeps(visit func(Module)) { func (b *baseModuleContext) VisitDirectDepsWithTag(tag blueprint.DependencyTag, visit func(Module)) { b.bp.VisitDirectDeps(func(module blueprint.Module) { - if aModule := b.validateAndroidModule(module, b.strictVisitDeps); aModule != nil { + if aModule := b.validateAndroidModule(module, b.bp.OtherModuleDependencyTag(module), b.strictVisitDeps); aModule != nil { if b.bp.OtherModuleDependencyTag(aModule) == tag { visit(aModule) } @@ -2363,7 +2370,7 @@ func (b *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func b.bp.VisitDirectDepsIf( // pred func(module blueprint.Module) bool { - if aModule := b.validateAndroidModule(module, b.strictVisitDeps); aModule != nil { + if aModule := b.validateAndroidModule(module, b.bp.OtherModuleDependencyTag(module), b.strictVisitDeps); aModule != nil { return pred(aModule) } else { return false @@ -2377,7 +2384,7 @@ func (b *baseModuleContext) VisitDirectDepsIf(pred func(Module) bool, visit func func (b *baseModuleContext) VisitDepsDepthFirst(visit func(Module)) { b.bp.VisitDepsDepthFirst(func(module blueprint.Module) { - if aModule := b.validateAndroidModule(module, b.strictVisitDeps); aModule != nil { + if aModule := b.validateAndroidModule(module, b.bp.OtherModuleDependencyTag(module), b.strictVisitDeps); aModule != nil { visit(aModule) } }) @@ -2387,7 +2394,7 @@ func (b *baseModuleContext) VisitDepsDepthFirstIf(pred func(Module) bool, visit b.bp.VisitDepsDepthFirstIf( // pred func(module blueprint.Module) bool { - if aModule := b.validateAndroidModule(module, b.strictVisitDeps); aModule != nil { + if aModule := b.validateAndroidModule(module, b.bp.OtherModuleDependencyTag(module), b.strictVisitDeps); aModule != nil { return pred(aModule) } else { return false diff --git a/dexpreopt/config.go b/dexpreopt/config.go index 7e73bf71d..0bcec17b5 100644 --- a/dexpreopt/config.go +++ b/dexpreopt/config.go @@ -372,6 +372,15 @@ func (d dex2oatDependencyTag) ExcludeFromVisibilityEnforcement() { func (d dex2oatDependencyTag) ExcludeFromApexContents() { } +func (d dex2oatDependencyTag) AllowDisabledModuleDependency(target android.Module) bool { + // RegisterToolDeps may run after the prebuilt mutators and hence register a + // dependency on the source module even when the prebuilt is to be used. + // dex2oatPathFromDep takes that into account when it retrieves the path to + // the binary, but we also need to disable the check for dependencies on + // disabled modules. + return target.IsReplacedByPrebuilt() +} + // Dex2oatDepTag represents the dependency onto the dex2oatd module. It is added to any module that // needs dexpreopting and so it makes no sense for it to be checked for visibility or included in // the apex. @@ -379,6 +388,7 @@ var Dex2oatDepTag = dex2oatDependencyTag{} var _ android.ExcludeFromVisibilityEnforcementTag = Dex2oatDepTag var _ android.ExcludeFromApexContentsTag = Dex2oatDepTag +var _ android.AllowDisabledModuleDependency = Dex2oatDepTag // RegisterToolDeps adds the necessary dependencies to binary modules for tools // that are required later when Get(Cached)GlobalSoongConfig is called. It diff --git a/java/dexpreopt_test.go b/java/dexpreopt_test.go index a9e0773b7..f065534d5 100644 --- a/java/dexpreopt_test.go +++ b/java/dexpreopt_test.go @@ -15,7 +15,12 @@ package java import ( + "fmt" "testing" + + "android/soong/android" + "android/soong/cc" + "android/soong/dexpreopt" ) func TestDexpreoptEnabled(t *testing.T) { @@ -166,3 +171,46 @@ func enabledString(enabled bool) string { return "disabled" } } + +func TestDex2oatToolDeps(t *testing.T) { + preparers := android.GroupFixturePreparers( + cc.PrepareForTestWithCcDefaultModules, + PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd, + dexpreopt.PrepareForTestByEnablingDexpreopt) + + testDex2oatToolDep := func(sourceEnabled, prebuiltEnabled, prebuiltPreferred bool, + expectedDex2oatPath string) { + name := fmt.Sprintf("sourceEnabled:%t,prebuiltEnabled:%t,prebuiltPreferred:%t", + sourceEnabled, prebuiltEnabled, prebuiltPreferred) + t.Run(name, func(t *testing.T) { + result := preparers.RunTestWithBp(t, fmt.Sprintf(` + cc_binary { + name: "dex2oatd", + enabled: %t, + host_supported: true, + } + cc_prebuilt_binary { + name: "dex2oatd", + enabled: %t, + prefer: %t, + host_supported: true, + srcs: ["x86_64/bin/dex2oatd"], + } + java_library { + name: "myjavalib", + } + `, sourceEnabled, prebuiltEnabled, prebuiltPreferred)) + pathContext := android.PathContextForTesting(result.Config) + dex2oatPath := dexpreopt.GetCachedGlobalSoongConfig(pathContext).Dex2oat + android.AssertStringEquals(t, "Testing "+name, expectedDex2oatPath, android.NormalizePathForTesting(dex2oatPath)) + }) + } + + sourceDex2oatPath := "host/linux-x86/bin/dex2oatd" + prebuiltDex2oatPath := ".intermediates/prebuilt_dex2oatd/linux_glibc_x86_64/dex2oatd" + + testDex2oatToolDep(true, false, false, sourceDex2oatPath) + testDex2oatToolDep(true, true, false, sourceDex2oatPath) + testDex2oatToolDep(true, true, true, prebuiltDex2oatPath) + testDex2oatToolDep(false, true, false, prebuiltDex2oatPath) +} diff --git a/java/java.go b/java/java.go index 45eb69339..2bbb5b102 100644 --- a/java/java.go +++ b/java/java.go @@ -57,6 +57,10 @@ func registerJavaBuildComponents(ctx android.RegistrationContext) { ctx.RegisterModuleType("java_host_for_device", HostForDeviceFactory) ctx.RegisterModuleType("dex_import", DexImportFactory) + // This mutator registers dependencies on dex2oat for modules that should be + // dexpreopted. This is done late when the final variants have been + // established, to not get the dependencies split into the wrong variants and + // to support the checks in dexpreoptDisabled(). ctx.FinalDepsMutators(func(ctx android.RegisterMutatorsContext) { ctx.BottomUp("dexpreopt_tool_deps", dexpreoptToolDepsMutator).Parallel() }) diff --git a/java/testing.go b/java/testing.go index 387d59573..51a1c621d 100644 --- a/java/testing.go +++ b/java/testing.go @@ -24,6 +24,7 @@ import ( "android/soong/android" "android/soong/cc" "android/soong/dexpreopt" + "github.com/google/blueprint" ) @@ -55,8 +56,9 @@ var PrepareForTestWithJavaBuildComponents = android.GroupFixturePreparers( }.AddToFixture(), ) -// Test fixture preparer that will define default java modules, e.g. standard prebuilt modules. -var PrepareForTestWithJavaDefaultModules = android.GroupFixturePreparers( +// Test fixture preparer that will define all default java modules except the +// fake_tool_binary for dex2oatd. +var PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd = android.GroupFixturePreparers( // Make sure that all the module types used in the defaults are registered. PrepareForTestWithJavaBuildComponents, // Additional files needed when test disallows non-existent source. @@ -72,6 +74,11 @@ var PrepareForTestWithJavaDefaultModules = android.GroupFixturePreparers( android.FixtureAddTextFile(defaultJavaDir+"/Android.bp", gatherRequiredDepsForTest()), // Add dexpreopt compat libs (android.test.base, etc.) and a fake dex2oatd module. dexpreopt.PrepareForTestWithDexpreoptCompatLibs, +) + +// Test fixture preparer that will define default java modules, e.g. standard prebuilt modules. +var PrepareForTestWithJavaDefaultModules = android.GroupFixturePreparers( + PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd, dexpreopt.PrepareForTestWithFakeDex2oatd, )