From 3d08c388b9e00cf98510cbd1408b5ffc3524e2ff Mon Sep 17 00:00:00 2001 From: Qiao Yang Date: Fri, 5 May 2023 15:03:24 +0000 Subject: [PATCH 1/2] Revert "Generate app profiles even if dexpreopt is disabled." Revert submission 2574032 Reason for revert: DroidMonitor-triggered revert due to breakage , bug Reverted changes: /q/submissionid:2574032 Change-Id: Ia9d05f3b7439604eb4a4b4100f46879fe11f5820 BUG: <280902279> --- android/testing.go | 21 ++++++--------------- dexpreopt/dexpreopt.go | 8 -------- dexpreopt/testing.go | 7 ------- java/dex_test.go | 10 +++++----- java/dexpreopt.go | 10 ++++++++-- java/dexpreopt_test.go | 25 ------------------------- 6 files changed, 19 insertions(+), 62 deletions(-) diff --git a/android/testing.go b/android/testing.go index 2a9c6584e..fc39a9c7a 100644 --- a/android/testing.go +++ b/android/testing.go @@ -813,20 +813,6 @@ func normalizePathRelativeToTop(path Path) Path { return path.RelativeToTop() } -func allOutputs(p BuildParams) []string { - outputs := append(WritablePaths(nil), p.Outputs...) - outputs = append(outputs, p.ImplicitOutputs...) - if p.Output != nil { - outputs = append(outputs, p.Output) - } - return outputs.Strings() -} - -// AllOutputs returns all 'BuildParams.Output's and 'BuildParams.Outputs's in their full path string forms. -func (p TestingBuildParams) AllOutputs() []string { - return allOutputs(p.BuildParams) -} - // baseTestingComponent provides functionality common to both TestingModule and TestingSingleton. type baseTestingComponent struct { config Config @@ -968,7 +954,12 @@ func (b baseTestingComponent) buildParamsFromOutput(file string) TestingBuildPar func (b baseTestingComponent) allOutputs() []string { var outputFullPaths []string for _, p := range b.provider.BuildParamsForTests() { - outputFullPaths = append(outputFullPaths, allOutputs(p)...) + outputs := append(WritablePaths(nil), p.Outputs...) + outputs = append(outputs, p.ImplicitOutputs...) + if p.Output != nil { + outputs = append(outputs, p.Output) + } + outputFullPaths = append(outputFullPaths, outputs.Strings()...) } return outputFullPaths } diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 2b38793ff..a590c72a5 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -100,19 +100,11 @@ func GenerateDexpreoptRule(ctx android.BuilderContext, globalSoong *GlobalSoongC return rule, nil } -// If dexpreopt is applicable to the module, returns whether dexpreopt is disabled. Otherwise, the -// behavior is undefined. -// When it returns true, dexpreopt artifacts will not be generated, but profile will still be -// generated if profile-guided compilation is requested. func dexpreoptDisabled(ctx android.PathContext, global *GlobalConfig, module *ModuleConfig) bool { if ctx.Config().UnbundledBuild() { return true } - if global.DisablePreopt { - return true - } - if contains(global.DisablePreoptModules, module.Name) { return true } diff --git a/dexpreopt/testing.go b/dexpreopt/testing.go index 6ed0736f7..b3dd3cca2 100644 --- a/dexpreopt/testing.go +++ b/dexpreopt/testing.go @@ -181,10 +181,3 @@ func FixtureDisableDexpreoptBootImages(disable bool) android.FixturePreparer { dexpreoptConfig.DisablePreoptBootImages = disable }) } - -// FixtureDisableDexpreopt sets the DisablePreopt property in the global config. -func FixtureDisableDexpreopt(disable bool) android.FixturePreparer { - return FixtureModifyGlobalConfig(func(_ android.PathContext, dexpreoptConfig *GlobalConfig) { - dexpreoptConfig.DisablePreopt = disable - }) -} diff --git a/java/dex_test.go b/java/dex_test.go index 2ba3831f4..97fc3d0dd 100644 --- a/java/dex_test.go +++ b/java/dex_test.go @@ -23,7 +23,7 @@ import ( ) func TestR8(t *testing.T) { - result := PrepareForTestWithJavaDefaultModules.RunTestWithBp(t, ` + result := PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd.RunTestWithBp(t, ` android_app { name: "app", srcs: ["foo.java"], @@ -191,7 +191,7 @@ func TestR8TransitiveDeps(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - fixturePreparer := PrepareForTestWithJavaDefaultModules + fixturePreparer := PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd if tc.unbundled { fixturePreparer = android.GroupFixturePreparers( fixturePreparer, @@ -258,7 +258,7 @@ func TestR8TransitiveDeps(t *testing.T) { } func TestR8Flags(t *testing.T) { - result := PrepareForTestWithJavaDefaultModules.RunTestWithBp(t, ` + result := PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd.RunTestWithBp(t, ` android_app { name: "app", srcs: ["foo.java"], @@ -287,7 +287,7 @@ func TestR8Flags(t *testing.T) { } func TestD8(t *testing.T) { - result := PrepareForTestWithJavaDefaultModules.RunTestWithBp(t, ` + result := PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd.RunTestWithBp(t, ` java_library { name: "foo", srcs: ["foo.java"], @@ -328,7 +328,7 @@ func TestD8(t *testing.T) { } func TestProguardFlagsInheritance(t *testing.T) { - result := PrepareForTestWithJavaDefaultModules.RunTestWithBp(t, ` + result := PrepareForTestWithJavaDefaultModulesWithoutFakeDex2oatd.RunTestWithBp(t, ` android_app { name: "app", static_libs: [ diff --git a/java/dexpreopt.go b/java/dexpreopt.go index f0bb9a32c..0ffedf6c4 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -180,8 +180,6 @@ func moduleName(ctx android.BaseModuleContext) string { return android.RemoveOptionalPrebuiltPrefix(ctx.ModuleName()) } -// Returns whether dexpreopt is applicable to the module. -// When it returns true, neither profile nor dexpreopt artifacts will be generated. func (d *dexpreopter) dexpreoptDisabled(ctx android.BaseModuleContext) bool { if !ctx.Device() { return true @@ -207,6 +205,14 @@ func (d *dexpreopter) dexpreoptDisabled(ctx android.BaseModuleContext) bool { global := dexpreopt.GetGlobalConfig(ctx) + if global.DisablePreopt { + return true + } + + if inList(moduleName(ctx), global.DisablePreoptModules) { + return true + } + isApexSystemServerJar := global.AllApexSystemServerJars(ctx).ContainsJar(moduleName(ctx)) if isApexVariant(ctx) { // Don't preopt APEX variant module unless the module is an APEX system server jar. diff --git a/java/dexpreopt_test.go b/java/dexpreopt_test.go index f91ac5cc3..3d2c5c3a1 100644 --- a/java/dexpreopt_test.go +++ b/java/dexpreopt_test.go @@ -438,28 +438,3 @@ func TestAndroidMkEntriesForApex(t *testing.T) { android.AssertIntEquals(t, "entries count", 0, len(entriesList)) } - -func TestGenerateProfileEvenIfDexpreoptIsDisabled(t *testing.T) { - preparers := android.GroupFixturePreparers( - PrepareForTestWithJavaDefaultModules, - PrepareForTestWithFakeApexMutator, - dexpreopt.FixtureDisableDexpreopt(true), - ) - - result := preparers.RunTestWithBp(t, ` - java_library { - name: "foo", - installable: true, - dex_preopt: { - profile: "art-profile", - }, - srcs: ["a.java"], - }`) - - ctx := result.TestContext - dexpreopt := ctx.ModuleForTests("foo", "android_common").MaybeRule("dexpreopt") - - expected := []string{"out/soong/.intermediates/foo/android_common/dexpreopt/profile.prof"} - - android.AssertArrayString(t, "outputs", expected, dexpreopt.AllOutputs()) -} From 8d8c660710037e87c7094a612246d64e5af74846 Mon Sep 17 00:00:00 2001 From: Qiao Yang Date: Fri, 5 May 2023 15:03:24 +0000 Subject: [PATCH 2/2] Revert "Generate boot image profiles even if dexpreopt is disabled." Revert submission 2574032 Reason for revert: DroidMonitor-triggered revert due to breakage , bug Reverted changes: /q/submissionid:2574032 Change-Id: I8e99f8231639198b149ea8d822ee7f9a5b391a89 BUG: <280902279> --- apex/apex.go | 2 +- apex/bootclasspath_fragment_test.go | 20 -------------------- dexpreopt/testing.go | 7 ------- java/bootclasspath_fragment.go | 15 ++++++++------- java/dexpreopt_bootjars.go | 7 +++---- java/platform_bootclasspath.go | 15 +++++++++------ sdk/java_sdk_test.go | 2 -- 7 files changed, 21 insertions(+), 47 deletions(-) diff --git a/apex/apex.go b/apex/apex.go index 5e4d3fc0e..baf4737d8 100644 --- a/apex/apex.go +++ b/apex/apex.go @@ -2675,7 +2675,7 @@ func apexBootclasspathFragmentFiles(ctx android.ModuleContext, module blueprint. } pathInApex := bootclasspathFragmentInfo.ProfileInstallPathInApex() - if pathInApex != "" { + if pathInApex != "" && !java.SkipDexpreoptBootJars(ctx) { pathOnHost := bootclasspathFragmentInfo.ProfilePathOnHost() tempPath := android.PathForModuleOut(ctx, "boot_image_profile", pathInApex) diff --git a/apex/bootclasspath_fragment_test.go b/apex/bootclasspath_fragment_test.go index 4a661d4ea..2ddfd0305 100644 --- a/apex/bootclasspath_fragment_test.go +++ b/apex/bootclasspath_fragment_test.go @@ -497,26 +497,6 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) { }) }) - t.Run("generate boot image profile even if dexpreopt is disabled", func(t *testing.T) { - result := android.GroupFixturePreparers( - commonPreparer, - - // Configure some libraries in the art bootclasspath_fragment that match the source - // bootclasspath_fragment's contents property. - java.FixtureConfigureBootJars("com.android.art:foo", "com.android.art:bar"), - addSource("foo", "bar"), - java.FixtureSetBootImageInstallDirOnDevice("art", "system/framework"), - dexpreopt.FixtureDisableDexpreoptBootImages(true), - ).RunTest(t) - - ensureExactContents(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{ - "etc/boot-image.prof", - "etc/classpaths/bootclasspath.pb", - "javalib/bar.jar", - "javalib/foo.jar", - }) - }) - t.Run("boot image disable generate profile", func(t *testing.T) { result := android.GroupFixturePreparers( commonPreparer, diff --git a/dexpreopt/testing.go b/dexpreopt/testing.go index b3dd3cca2..47ae494e3 100644 --- a/dexpreopt/testing.go +++ b/dexpreopt/testing.go @@ -174,10 +174,3 @@ func FixtureDisableGenerateProfile(disable bool) android.FixturePreparer { dexpreoptConfig.DisableGenerateProfile = disable }) } - -// FixtureDisableDexpreoptBootImages sets the DisablePreoptBootImages property in the global config. -func FixtureDisableDexpreoptBootImages(disable bool) android.FixturePreparer { - return FixtureModifyGlobalConfig(func(_ android.PathContext, dexpreoptConfig *GlobalConfig) { - dexpreoptConfig.DisablePreoptBootImages = disable - }) -} diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go index d1a020e4d..f69256347 100644 --- a/java/bootclasspath_fragment.go +++ b/java/bootclasspath_fragment.go @@ -506,6 +506,10 @@ func (b *BootclasspathFragmentModule) DepsMutator(ctx android.BottomUpMutatorCon } } + if SkipDexpreoptBootJars(ctx) { + return + } + // Add a dependency onto the dex2oat tool which is needed for creating the boot image. The // path is retrieved from the dependency by GetGlobalSoongConfig(ctx). dexpreopt.RegisterToolDeps(ctx) @@ -897,6 +901,10 @@ func (b *BootclasspathFragmentModule) produceHiddenAPIOutput(ctx android.ModuleC // produceBootImageFiles builds the boot image files from the source if it is required. func (b *BootclasspathFragmentModule) produceBootImageFiles(ctx android.ModuleContext, imageConfig *bootImageConfig) bootImageOutputs { + if SkipDexpreoptBootJars(ctx) { + return bootImageOutputs{} + } + // Only generate the boot image if the configuration does not skip it. return b.generateBootImageBuildActions(ctx, imageConfig) } @@ -924,13 +932,6 @@ func (b *BootclasspathFragmentModule) generateBootImageBuildActions(ctx android. // Build boot image files for the host variants. buildBootImageVariantsForBuildOs(ctx, imageConfig, profile) - // If dexpreopt of boot image jars should be skipped, generate only a profile. - if SkipDexpreoptBootJars(ctx) { - return bootImageOutputs{ - profile: profile, - } - } - // Build boot image files for the android variants. bootImageFiles := buildBootImageVariantsForAndroidOs(ctx, imageConfig, profile) diff --git a/java/dexpreopt_bootjars.go b/java/dexpreopt_bootjars.go index fed384e9d..f4827ea3a 100644 --- a/java/dexpreopt_bootjars.go +++ b/java/dexpreopt_bootjars.go @@ -500,6 +500,9 @@ func (d *dexpreoptBootJars) GenerateAndroidBuildActions(ctx android.ModuleContex // Generate build rules for boot images. func (d *dexpreoptBootJars) GenerateSingletonBuildActions(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 @@ -1011,10 +1014,6 @@ func (d *dexpreoptBootJars) MakeVars(ctx android.MakeVarsContext) { ctx.Strict("DEXPREOPT_IMAGE_PROFILE_LICENSE_METADATA", image.profileLicenseMetadataFile.String()) } - if SkipDexpreoptBootJars(ctx) { - return - } - global := dexpreopt.GetGlobalConfig(ctx) dexPaths, dexLocations := bcpForDexpreopt(ctx, global.PreoptWithUpdatableBcp) ctx.Strict("DEXPREOPT_BOOTCLASSPATH_DEX_FILES", strings.Join(dexPaths.Strings(), " ")) diff --git a/java/platform_bootclasspath.go b/java/platform_bootclasspath.go index 254c40535..0ea360979 100644 --- a/java/platform_bootclasspath.go +++ b/java/platform_bootclasspath.go @@ -103,6 +103,10 @@ func (b *platformBootclasspathModule) OutputFiles(tag string) (android.Paths, er func (b *platformBootclasspathModule) DepsMutator(ctx android.BottomUpMutatorContext) { b.hiddenAPIDepsMutator(ctx) + if SkipDexpreoptBootJars(ctx) { + return + } + // Add a dependency onto the dex2oat tool which is needed for creating the boot image. The // path is retrieved from the dependency by GetGlobalSoongConfig(ctx). dexpreopt.RegisterToolDeps(ctx) @@ -183,6 +187,11 @@ func (b *platformBootclasspathModule) GenerateAndroidBuildActions(ctx android.Mo bootDexJarByModule := b.generateHiddenAPIBuildActions(ctx, b.configuredModules, b.fragments) buildRuleForBootJarsPackageCheck(ctx, bootDexJarByModule) + // Nothing to do if skipping the dexpreopt of boot image jars. + if SkipDexpreoptBootJars(ctx) { + return + } + b.generateBootImageBuildActions(ctx, platformModules, apexModules) } @@ -420,12 +429,6 @@ func (b *platformBootclasspathModule) generateBootImage(ctx android.ModuleContex // Build a profile for the image config and then use that to build the boot image. profile := bootImageProfileRule(ctx, imageConfig) - // If dexpreopt of boot image jars should be skipped, generate only a profile. - global := dexpreopt.GetGlobalConfig(ctx) - if global.DisablePreoptBootImages { - return - } - // Build boot image files for the android variants. androidBootImageFiles := buildBootImageVariantsForAndroidOs(ctx, imageConfig, profile) diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go index 6159ea9c2..3a2ecc00c 100644 --- a/sdk/java_sdk_test.go +++ b/sdk/java_sdk_test.go @@ -19,14 +19,12 @@ import ( "testing" "android/soong/android" - "android/soong/dexpreopt" "android/soong/java" ) var prepareForSdkTestWithJava = android.GroupFixturePreparers( java.PrepareForTestWithJavaBuildComponents, PrepareForTestWithSdkBuildComponents, - dexpreopt.PrepareForTestWithFakeDex2oatd, // Ensure that all source paths are provided. This helps ensure that the snapshot generation is // consistent and all files referenced from the snapshot's Android.bp file have actually been