From 5ae65ee6e060815dc5f3c11b505a313a3c37317c Mon Sep 17 00:00:00 2001 From: Spandan Das Date: Tue, 16 Apr 2024 22:03:26 +0000 Subject: [PATCH] Allow dexpreopt of source sdklib in prebuilt apex builds aosp/2984037 disabled dexperopt of the source apex system server jars when prebuilts are selected. This was done to prevent duplicate installation and dex2oat deps. AOSP art has some additional variants like com.android.art.debug. In source builds, this apex should contain service-art.jar and service-art.jar.prof (a dexpeopt artifact). We have a test to check this (`art_apex_test.py`). If we disable dexpreopt of source sdlib when prebuilts are selected, this test needs to be disabled. This is the behavior at ToT. This CL changes the behavior to enable running this test even when com.google.android.art prebuilt is active in a specific release configuraiton. To prevent collisions that prompted aosp/2984037, this CL special-cases the installation and dex2oat rules instead of disabling dexpreopt of the source sdklib altogether. b/331665856 tracks the principled solution to prevent duplicate dexpreopt rules. Implementation: Add a new copyApexSystemServerJarDex arg to GenerateDexpreoptRule API. If true, the dexjar file will be copied to out/soong/system_server_jars/. For soong modules, the value of this will be the inverse of disableSourceApexVariant. Since none of the apex system server jars are in make, this will be a noop in dexpreopt_gen Bug: 331665856 Test: modified trunk_staging.scl locally to select art.google.contributions.prebuilt; mmma art; (with the sibling CL in topic) Change-Id: Idb59e424f83d126cdc8b1671dde358745979bd8d --- dexpreopt/dexpreopt.go | 8 +++--- dexpreopt/dexpreopt_gen/dexpreopt_gen.go | 3 ++- dexpreopt/dexpreopt_test.go | 31 ++++++++++++++++++------ java/dexpreopt.go | 10 ++++---- java/java.go | 6 +++++ java/sdk_library.go | 6 +++++ 6 files changed, 47 insertions(+), 17 deletions(-) diff --git a/dexpreopt/dexpreopt.go b/dexpreopt/dexpreopt.go index 04bc61dad..93351f1fc 100644 --- a/dexpreopt/dexpreopt.go +++ b/dexpreopt/dexpreopt.go @@ -52,7 +52,7 @@ var DexpreoptRunningInSoong = false // 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.BuilderContext, globalSoong *GlobalSoongConfig, - global *GlobalConfig, module *ModuleConfig, productPackages android.Path) ( + global *GlobalConfig, module *ModuleConfig, productPackages android.Path, copyApexSystemServerJarDex bool) ( rule *android.RuleBuilder, err error) { defer func() { @@ -94,7 +94,7 @@ func GenerateDexpreoptRule(ctx android.BuilderContext, globalSoong *GlobalSoongC for archIdx, _ := range module.Archs { dexpreoptCommand(ctx, globalSoong, global, module, rule, archIdx, profile, appImage, - generateDM, productPackages) + generateDM, productPackages, copyApexSystemServerJarDex) } } } @@ -231,7 +231,7 @@ func ToOdexPath(path string, arch android.ArchType) string { func dexpreoptCommand(ctx android.BuilderContext, globalSoong *GlobalSoongConfig, global *GlobalConfig, module *ModuleConfig, rule *android.RuleBuilder, archIdx int, - profile android.WritablePath, appImage bool, generateDM bool, productPackages android.Path) { + profile android.WritablePath, appImage bool, generateDM bool, productPackages android.Path, copyApexSystemServerJarDex bool) { arch := module.Archs[archIdx] @@ -277,7 +277,7 @@ func dexpreoptCommand(ctx android.BuilderContext, globalSoong *GlobalSoongConfig clcTarget = append(clcTarget, GetSystemServerDexLocation(ctx, global, lib)) } - if DexpreoptRunningInSoong { + if DexpreoptRunningInSoong && copyApexSystemServerJarDex { // Copy the system server jar to a predefined location where dex2oat will find it. dexPathHost := SystemServerDexJarHostPath(ctx, module.Name) rule.Command().Text("mkdir -p").Flag(filepath.Dir(dexPathHost.String())) diff --git a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go index 8033b48e5..75120052e 100644 --- a/dexpreopt/dexpreopt_gen/dexpreopt_gen.go +++ b/dexpreopt/dexpreopt_gen/dexpreopt_gen.go @@ -205,8 +205,9 @@ func writeScripts(ctx android.BuilderContext, globalSoong *dexpreopt.GlobalSoong panic(err) } } + cpApexSscpServerJar := false // dexpreopt_gen operates on make modules, and since sscp libraries are in soong, this should be a noop dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule( - ctx, globalSoong, global, module, android.PathForTesting(productPackagesPath)) + ctx, globalSoong, global, module, android.PathForTesting(productPackagesPath), cpApexSscpServerJar) if err != nil { panic(err) } diff --git a/dexpreopt/dexpreopt_test.go b/dexpreopt/dexpreopt_test.go index 7071f3e9e..eff2416e5 100644 --- a/dexpreopt/dexpreopt_test.go +++ b/dexpreopt/dexpreopt_test.go @@ -101,7 +101,7 @@ func TestDexPreopt(t *testing.T) { module := testSystemModuleConfig(ctx, "test") productPackages := android.PathForTesting("product_packages.txt") - rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, true) if err != nil { t.Fatal(err) } @@ -161,7 +161,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, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, mt.module, productPackages, true) if err != nil { t.Fatal(err) } @@ -181,6 +181,11 @@ func TestDexPreoptSystemOther(t *testing.T) { } func TestDexPreoptApexSystemServerJars(t *testing.T) { + // modify the global variable for test + var oldDexpreoptRunningInSoong = DexpreoptRunningInSoong + DexpreoptRunningInSoong = true + + // test begin config := android.TestConfig("out", nil, "", nil) ctx := android.BuilderContextForTesting(config) globalSoong := globalSoongConfigForTests(ctx) @@ -191,7 +196,7 @@ func TestDexPreoptApexSystemServerJars(t *testing.T) { global.ApexSystemServerJars = android.CreateTestConfiguredJarList( []string{"com.android.apex1:service-A"}) - rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, true) if err != nil { t.Fatal(err) } @@ -202,6 +207,18 @@ func TestDexPreoptApexSystemServerJars(t *testing.T) { } android.AssertStringEquals(t, "installs", wantInstalls.String(), rule.Installs().String()) + + android.AssertStringListContains(t, "apex sscp jar copy", rule.Outputs().Strings(), "out/soong/system_server_dexjars/service-A.jar") + + // rule with apex sscp cp as false + rule, err = GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, false) + if err != nil { + t.Fatal(err) + } + android.AssertStringListDoesNotContain(t, "apex sscp jar copy", rule.Outputs().Strings(), "out/soong/system_server_dexjars/service-A.jar") + + // cleanup the global variable for test + DexpreoptRunningInSoong = oldDexpreoptRunningInSoong } func TestDexPreoptStandaloneSystemServerJars(t *testing.T) { @@ -215,7 +232,7 @@ func TestDexPreoptStandaloneSystemServerJars(t *testing.T) { global.StandaloneSystemServerJars = android.CreateTestConfiguredJarList( []string{"platform:service-A"}) - rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, true) if err != nil { t.Fatal(err) } @@ -239,7 +256,7 @@ func TestDexPreoptSystemExtSystemServerJars(t *testing.T) { global.StandaloneSystemServerJars = android.CreateTestConfiguredJarList( []string{"system_ext:service-A"}) - rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, true) if err != nil { t.Fatal(err) } @@ -263,7 +280,7 @@ func TestDexPreoptApexStandaloneSystemServerJars(t *testing.T) { global.ApexStandaloneSystemServerJars = android.CreateTestConfiguredJarList( []string{"com.android.apex1:service-A"}) - rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, true) if err != nil { t.Fatal(err) } @@ -286,7 +303,7 @@ func TestDexPreoptProfile(t *testing.T) { module.ProfileClassListing = android.OptionalPathForPath(android.PathForTesting("profile")) - rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages) + rule, err := GenerateDexpreoptRule(ctx, globalSoong, global, module, productPackages, true) if err != nil { t.Fatal(err) } diff --git a/java/dexpreopt.go b/java/dexpreopt.go index 38ed856ee..25e95db14 100644 --- a/java/dexpreopt.go +++ b/java/dexpreopt.go @@ -243,10 +243,6 @@ func (d *dexpreopter) dexpreoptDisabled(ctx android.BaseModuleContext, libName s return true } - if disableSourceApexVariant(ctx) { - return true - } - if _, isApex := android.ModuleProvider(ctx, android.ApexBundleInfoProvider); isApex { // dexpreopt rules for system server jars can be generated in the ModuleCtx of prebuilt apexes return false @@ -501,8 +497,12 @@ func (d *dexpreopter) dexpreopt(ctx android.ModuleContext, libName string, dexJa Output(appProductPackages) productPackagesRule.Restat().Build("product_packages."+dexJarStem, "dexpreopt product_packages") + // Prebuilts are active, do not copy the dexpreopt'd source javalib to out/soong/system_server_dexjars + // The javalib from the deapexed prebuilt will be copied to this location. + // TODO (b/331665856): Implement a principled solution for this. + copyApexSystemServerJarDex := !disableSourceApexVariant(ctx) dexpreoptRule, err := dexpreopt.GenerateDexpreoptRule( - ctx, globalSoong, global, dexpreoptConfig, appProductPackages) + ctx, globalSoong, global, dexpreoptConfig, appProductPackages, copyApexSystemServerJarDex) if err != nil { ctx.ModuleErrorf("error generating dexpreopt rule: %s", err.Error()) return diff --git a/java/java.go b/java/java.go index fb5bb1cae..188723060 100644 --- a/java/java.go +++ b/java/java.go @@ -886,6 +886,12 @@ func init() { } func (j *Library) GenerateAndroidBuildActions(ctx android.ModuleContext) { + if disableSourceApexVariant(ctx) { + // Prebuilts are active, do not create the installation rules for the source javalib. + // Even though the source javalib is not used, we need to hide it to prevent duplicate installation rules. + // TODO (b/331665856): Implement a principled solution for this. + j.HideFromMake() + } j.provideHiddenAPIPropertyInfo(ctx) j.sdkVersion = j.SdkVersion(ctx) diff --git a/java/sdk_library.go b/java/sdk_library.go index e7e53a2a8..56b1d399d 100644 --- a/java/sdk_library.go +++ b/java/sdk_library.go @@ -1562,6 +1562,12 @@ func (module *SdkLibrary) OutputFiles(tag string) (android.Paths, error) { } func (module *SdkLibrary) GenerateAndroidBuildActions(ctx android.ModuleContext) { + if disableSourceApexVariant(ctx) { + // Prebuilts are active, do not create the installation rules for the source javalib. + // Even though the source javalib is not used, we need to hide it to prevent duplicate installation rules. + // TODO (b/331665856): Implement a principled solution for this. + module.HideFromMake() + } if proptools.String(module.deviceProperties.Min_sdk_version) != "" { module.CheckMinSdkVersion(ctx) }