From 43c44b00d63aef2d84aee2e2e7b9bce14dceb91b Mon Sep 17 00:00:00 2001 From: Martin Stjernholm Date: Wed, 30 Jun 2021 16:35:07 +0100 Subject: [PATCH] Do not allow duplicate deapexer dependencies. Without these errors, the last encountered deapexer would silently be used. However with PRODUCT_INSTALL_APEXES there will only be deapexer for installable APEXes, and we can assume only a single installable APEX exists for each APEX variant, so make it an error if it isn't the case. Corresponding to http://ag/15156931. Test: m nothing SOONG_CONFIG_art_module_source_build=true Test: m nothing SOONG_CONFIG_art_module_source_build=false Test: m nothing SOONG_CONFIG_art_module_source_build=false with installable:true for the com.android.art.debug prebuilt APEX - check that it fails with an "ambiguous duplicate deapexer" error Bug: 192006406 Bug: 192542393 Change-Id: I44566fd26b12f82a8a67fe4a69e56303460756d0 --- android/deapexer.go | 8 +++- apex/apex_test.go | 69 +++++++++++++++++++++++++++++ apex/bootclasspath_fragment_test.go | 53 ++++++++++++++++------ 3 files changed, 114 insertions(+), 16 deletions(-) diff --git a/android/deapexer.go b/android/deapexer.go index bed657464..265f5312b 100644 --- a/android/deapexer.go +++ b/android/deapexer.go @@ -143,12 +143,16 @@ type RequiresFilesFromPrebuiltApexTag interface { } // FindDeapexerProviderForModule searches through the direct dependencies of the current context -// module for a DeapexerTag dependency and returns its DeapexerInfo. If there is an error then it is -// reported with ctx.ModuleErrorf and nil is returned. +// module for a DeapexerTag dependency and returns its DeapexerInfo. If a single nonambiguous +// deapexer module isn't found then errors are reported with ctx.ModuleErrorf and nil is returned. func FindDeapexerProviderForModule(ctx ModuleContext) *DeapexerInfo { var di *DeapexerInfo ctx.VisitDirectDepsWithTag(DeapexerTag, func(m Module) { p := ctx.OtherModuleProvider(m, DeapexerProvider).(DeapexerInfo) + if di != nil { + ctx.ModuleErrorf("Multiple installable prebuilt APEXes provide ambiguous deapexers: %s and %s", + di.ApexModuleName(), p.ApexModuleName()) + } di = &p }) if di != nil { diff --git a/apex/apex_test.go b/apex/apex_test.go index 2f7dce5a8..387dbc16f 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -7045,6 +7045,75 @@ func testDexpreoptWithApexes(t *testing.T, bp, errmsg string, preparer android.F return result.TestContext } +func TestDuplicateDeapexeresFromPrebuiltApexes(t *testing.T) { + preparers := android.GroupFixturePreparers( + java.PrepareForTestWithJavaDefaultModules, + PrepareForTestWithApexBuildComponents, + ). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( + "Multiple installable prebuilt APEXes provide ambiguous deapexers: com.android.myapex and com.mycompany.android.myapex")) + + bpBase := ` + apex_set { + name: "com.android.myapex", + installable: true, + exported_bootclasspath_fragments: ["my-bootclasspath-fragment"], + set: "myapex.apks", + } + + apex_set { + name: "com.mycompany.android.myapex", + apex_name: "com.android.myapex", + installable: true, + exported_bootclasspath_fragments: ["my-bootclasspath-fragment"], + set: "company-myapex.apks", + } + + prebuilt_bootclasspath_fragment { + name: "my-bootclasspath-fragment", + apex_available: ["com.android.myapex"], + %s + } + ` + + t.Run("java_import", func(t *testing.T) { + _ = preparers.RunTestWithBp(t, fmt.Sprintf(bpBase, `contents: ["libfoo"]`)+` + java_import { + name: "libfoo", + jars: ["libfoo.jar"], + apex_available: ["com.android.myapex"], + } + `) + }) + + t.Run("java_sdk_library_import", func(t *testing.T) { + _ = preparers.RunTestWithBp(t, fmt.Sprintf(bpBase, `contents: ["libfoo"]`)+` + java_sdk_library_import { + name: "libfoo", + public: { + jars: ["libbar.jar"], + }, + apex_available: ["com.android.myapex"], + } + `) + }) + + t.Run("prebuilt_bootclasspath_fragment", func(t *testing.T) { + _ = preparers.RunTestWithBp(t, fmt.Sprintf(bpBase, ` + image_name: "art", + contents: ["libfoo"], + `)+` + java_sdk_library_import { + name: "libfoo", + public: { + jars: ["libbar.jar"], + }, + apex_available: ["com.android.myapex"], + } + `) + }) +} + func TestUpdatable_should_set_min_sdk_version(t *testing.T) { testApexError(t, `"myapex" .*: updatable: updatable APEXes should set min_sdk_version`, ` apex { diff --git a/apex/bootclasspath_fragment_test.go b/apex/bootclasspath_fragment_test.go index 9e030f10c..7ecf9b20f 100644 --- a/apex/bootclasspath_fragment_test.go +++ b/apex/bootclasspath_fragment_test.go @@ -548,7 +548,7 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) { } func TestBootclasspathFragmentInPrebuiltArtApex(t *testing.T) { - result := android.GroupFixturePreparers( + preparers := android.GroupFixturePreparers( prepareForTestWithBootclasspathFragment, prepareForTestWithArtApex, @@ -559,7 +559,9 @@ func TestBootclasspathFragmentInPrebuiltArtApex(t *testing.T) { // Configure some libraries in the art bootclasspath_fragment. java.FixtureConfigureBootJars("com.android.art:foo", "com.android.art:bar"), - ).RunTestWithBp(t, ` + ) + + bp := ` prebuilt_apex { name: "com.android.art", arch: { @@ -605,22 +607,45 @@ func TestBootclasspathFragmentInPrebuiltArtApex(t *testing.T) { all_flags: "mybootclasspathfragment/all-flags.csv", }, } - `) - java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art", []string{ - `com.android.art.apex.selector`, - `prebuilt_mybootclasspathfragment`, + // A prebuilt apex with the same apex_name that shouldn't interfere when it isn't enabled. + prebuilt_apex { + name: "com.mycompany.android.art", + apex_name: "com.android.art", + %s + src: "com.mycompany.android.art.apex", + exported_bootclasspath_fragments: ["mybootclasspathfragment"], + } + ` + + t.Run("disabled alternative APEX", func(t *testing.T) { + result := preparers.RunTestWithBp(t, fmt.Sprintf(bp, "enabled: false,")) + + java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art", []string{ + `com.android.art.apex.selector`, + `prebuilt_mybootclasspathfragment`, + }) + + java.CheckModuleDependencies(t, result.TestContext, "mybootclasspathfragment", "android_common_com.android.art", []string{ + `com.android.art.deapexer`, + `dex2oatd`, + `prebuilt_bar`, + `prebuilt_foo`, + }) + + module := result.ModuleForTests("mybootclasspathfragment", "android_common_com.android.art") + checkCopiesToPredefinedLocationForArt(t, result.Config, module, "bar", "foo") + + // Check that the right deapexer module was chosen for a boot image. + param := module.Output("out/soong/test_device/dex_artjars/android/apex/art_boot_images/javalib/arm64/boot.art") + android.AssertStringDoesContain(t, "didn't find the expected deapexer in the input path", param.Input.String(), "/com.android.art.deapexer") }) - java.CheckModuleDependencies(t, result.TestContext, "mybootclasspathfragment", "android_common_com.android.art", []string{ - `com.android.art.deapexer`, - `dex2oatd`, - `prebuilt_bar`, - `prebuilt_foo`, + t.Run("enabled alternative APEX", func(t *testing.T) { + preparers.ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( + "Multiple installable prebuilt APEXes provide ambiguous deapexers: com.android.art and com.mycompany.android.art")). + RunTestWithBp(t, fmt.Sprintf(bp, "")) }) - - module := result.ModuleForTests("mybootclasspathfragment", "android_common_com.android.art") - checkCopiesToPredefinedLocationForArt(t, result.Config, module, "bar", "foo") } // checkCopiesToPredefinedLocationForArt checks that the supplied modules are copied to the