diff --git a/apex/bootclasspath_fragment_test.go b/apex/bootclasspath_fragment_test.go index feda482a1..3d39d341c 100644 --- a/apex/bootclasspath_fragment_test.go +++ b/apex/bootclasspath_fragment_test.go @@ -15,6 +15,7 @@ package apex import ( + "fmt" "strings" "testing" @@ -162,13 +163,11 @@ func checkBootclasspathFragment(t *testing.T, result *android.TestResult, module } func TestBootclasspathFragmentInArtApex(t *testing.T) { - result := android.GroupFixturePreparers( + commonPreparer := android.GroupFixturePreparers( prepareForTestWithBootclasspathFragment, prepareForTestWithArtApex, - // Configure some libraries in the art bootclasspath_fragment. - java.FixtureConfigureBootJars("com.android.art:foo", "com.android.art:bar"), - ).RunTestWithBp(t, ` + android.FixtureWithRootAndroidBp(` apex { name: "com.android.art", key: "com.android.art.key", @@ -208,14 +207,6 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) { ], } - bootclasspath_fragment { - name: "mybootclasspathfragment", - image_name: "art", - apex_available: [ - "com.android.art", - ], - } - java_import { name: "foo", jars: ["foo.jar"], @@ -231,39 +222,141 @@ func TestBootclasspathFragmentInArtApex(t *testing.T) { "com.android.art", ], } + `), + ) - // Make sure that a preferred prebuilt doesn't affect the apex. - prebuilt_bootclasspath_fragment { - name: "mybootclasspathfragment", - image_name: "art", - prefer: true, - apex_available: [ - "com.android.art", - ], + contentsInsert := func(contents []string) string { + insert := "" + if contents != nil { + insert = fmt.Sprintf(`contents: ["%s"],`, strings.Join(contents, `", "`)) } - `) + return insert + } - ensureExactContents(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{ - "javalib/arm/boot.art", - "javalib/arm/boot.oat", - "javalib/arm/boot.vdex", - "javalib/arm/boot-bar.art", - "javalib/arm/boot-bar.oat", - "javalib/arm/boot-bar.vdex", - "javalib/arm64/boot.art", - "javalib/arm64/boot.oat", - "javalib/arm64/boot.vdex", - "javalib/arm64/boot-bar.art", - "javalib/arm64/boot-bar.oat", - "javalib/arm64/boot-bar.vdex", - "javalib/bar.jar", - "javalib/foo.jar", + addSource := func(contents ...string) android.FixturePreparer { + text := fmt.Sprintf(` + bootclasspath_fragment { + name: "mybootclasspathfragment", + image_name: "art", + %s + apex_available: [ + "com.android.art", + ], + } + `, contentsInsert(contents)) + + return android.FixtureAddTextFile("art/build/boot/Android.bp", text) + } + + addPrebuilt := func(prefer bool, contents ...string) android.FixturePreparer { + text := fmt.Sprintf(` + prebuilt_bootclasspath_fragment { + name: "mybootclasspathfragment", + image_name: "art", + %s + prefer: %t, + apex_available: [ + "com.android.art", + ], + } + `, contentsInsert(contents), prefer) + return android.FixtureAddTextFile("prebuilts/module_sdk/art/Android.bp", text) + } + + t.Run("boot image files", 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"), + + // Make sure that a preferred prebuilt with consistent contents doesn't affect the apex. + addPrebuilt(true, "foo", "bar"), + ).RunTest(t) + + ensureExactContents(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{ + "javalib/arm/boot.art", + "javalib/arm/boot.oat", + "javalib/arm/boot.vdex", + "javalib/arm/boot-bar.art", + "javalib/arm/boot-bar.oat", + "javalib/arm/boot-bar.vdex", + "javalib/arm64/boot.art", + "javalib/arm64/boot.oat", + "javalib/arm64/boot.vdex", + "javalib/arm64/boot-bar.art", + "javalib/arm64/boot-bar.oat", + "javalib/arm64/boot-bar.vdex", + "javalib/bar.jar", + "javalib/foo.jar", + }) + + java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{ + `bar`, + `com.android.art.key`, + `mybootclasspathfragment`, + }) }) - java.CheckModuleDependencies(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{ - `bar`, - `com.android.art.key`, - `mybootclasspathfragment`, + t.Run("source with inconsistency between config and contents", func(t *testing.T) { + android.GroupFixturePreparers( + commonPreparer, + + // Create an inconsistency between the ArtApexJars configuration and the art source + // bootclasspath_fragment module's contents property. + java.FixtureConfigureBootJars("com.android.art:foo"), + addSource("foo", "bar"), + ). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(`\QArtApexJars configuration specifies []string{"foo"}, contents property specifies []string{"foo", "bar"}\E`)). + RunTest(t) + }) + + t.Run("prebuilt with inconsistency between config and contents", func(t *testing.T) { + android.GroupFixturePreparers( + commonPreparer, + + // Create an inconsistency between the ArtApexJars configuration and the art + // prebuilt_bootclasspath_fragment module's contents property. + java.FixtureConfigureBootJars("com.android.art:foo"), + addPrebuilt(false, "foo", "bar"), + ). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(`\QArtApexJars configuration specifies []string{"foo"}, contents property specifies []string{"foo", "bar"}\E`)). + RunTest(t) + }) + + t.Run("preferred prebuilt with inconsistency between config and contents", func(t *testing.T) { + android.GroupFixturePreparers( + commonPreparer, + + // Create an inconsistency between the ArtApexJars configuration and the art + // prebuilt_bootclasspath_fragment module's contents property. + java.FixtureConfigureBootJars("com.android.art:foo"), + addPrebuilt(true, "foo", "bar"), + + // Source contents property is consistent with the config. + addSource("foo"), + ). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(`\QArtApexJars configuration specifies []string{"foo"}, contents property specifies []string{"foo", "bar"}\E`)). + RunTest(t) + }) + + t.Run("source preferred and prebuilt with inconsistency between config and contents", func(t *testing.T) { + android.GroupFixturePreparers( + commonPreparer, + + // Create an inconsistency between the ArtApexJars configuration and the art + // prebuilt_bootclasspath_fragment module's contents property. + java.FixtureConfigureBootJars("com.android.art:foo"), + addPrebuilt(false, "foo", "bar"), + + // Source contents property is consistent with the config. + addSource("foo"), + + // This should pass because while the prebuilt is inconsistent with the configuration it is + // not actually used. + ).RunTest(t) }) } diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go index 8bb5cb18e..d0570b413 100644 --- a/java/bootclasspath_fragment.go +++ b/java/bootclasspath_fragment.go @@ -17,6 +17,7 @@ package java import ( "fmt" "path/filepath" + "reflect" "strings" "android/soong/android" @@ -128,8 +129,10 @@ func bootclasspathFragmentInitContentsFromImage(ctx android.EarlyModuleContext, if m.properties.Image_name == nil && len(contents) == 0 { ctx.ModuleErrorf(`neither of the "image_name" and "contents" properties have been supplied, please supply exactly one`) } - if m.properties.Image_name != nil && len(contents) != 0 { - ctx.ModuleErrorf(`both of the "image_name" and "contents" properties have been supplied, please supply exactly one`) + + if len(contents) != 0 { + // Nothing to do. + return } imageName := proptools.String(m.properties.Image_name) @@ -154,7 +157,6 @@ func bootclasspathFragmentInitContentsFromImage(ctx android.EarlyModuleContext, // Make sure that the apex specified in the configuration is consistent and is one for which // this boot image is available. - jars := []string{} commonApex := "" for i := 0; i < modules.Len(); i++ { apex := modules.Apex(i) @@ -174,11 +176,50 @@ func bootclasspathFragmentInitContentsFromImage(ctx android.EarlyModuleContext, ctx.ModuleErrorf("ArtApexJars configuration is inconsistent, expected all jars to be in the same apex but it specifies apex %q and %q", commonApex, apex) } - jars = append(jars, jar) } // Store the jars in the Contents property so that they can be used to add dependencies. - m.properties.Contents = jars + m.properties.Contents = modules.CopyOfJars() + } +} + +// bootclasspathImageNameContentsConsistencyCheck checks that the configuration that applies to this +// module (if any) matches the contents. +// +// This should be a noop as if image_name="art" then the contents will be set from the ArtApexJars +// config by bootclasspathFragmentInitContentsFromImage so it will be guaranteed to match. However, +// in future this will not be the case. +func (b *BootclasspathFragmentModule) bootclasspathImageNameContentsConsistencyCheck(ctx android.BaseModuleContext) { + imageName := proptools.String(b.properties.Image_name) + if imageName == "art" { + // TODO(b/177892522): Prebuilts (versioned or not) should not use the image_name property. + if b.MemberName() != "" { + // The module is a versioned prebuilt so ignore it. This is done for a couple of reasons: + // 1. There is no way to use this at the moment so ignoring it is safe. + // 2. Attempting to initialize the contents property from the configuration will end up having + // the versioned prebuilt depending on the unversioned prebuilt. That will cause problems + // as the unversioned prebuilt could end up with an APEX variant created for the source + // APEX which will prevent it from having an APEX variant for the prebuilt APEX which in + // turn will prevent it from accessing the dex implementation jar from that which will + // break hidden API processing, amongst others. + return + } + + // Get the configuration for the art apex jars. + modules := b.getImageConfig(ctx).modules + configuredJars := modules.CopyOfJars() + + // Skip the check if the configured jars list is empty as that is a common configuration when + // building targets that do not result in a system image. + if len(configuredJars) == 0 { + return + } + + contents := b.properties.Contents + if !reflect.DeepEqual(configuredJars, contents) { + ctx.ModuleErrorf("inconsistency in specification of contents. ArtApexJars configuration specifies %#v, contents property specifies %#v", + configuredJars, contents) + } } } @@ -274,6 +315,13 @@ func (b *BootclasspathFragmentModule) DepsMutator(ctx android.BottomUpMutatorCon } func (b *BootclasspathFragmentModule) GenerateAndroidBuildActions(ctx android.ModuleContext) { + // Only perform a consistency check if this module is the active module. That will prevent an + // unused prebuilt that was created without instrumentation from breaking an instrumentation + // build. + if isActiveModule(ctx.Module()) { + b.bootclasspathImageNameContentsConsistencyCheck(ctx) + } + // Perform hidden API processing. b.generateHiddenAPIBuildActions(ctx) diff --git a/java/bootclasspath_fragment_test.go b/java/bootclasspath_fragment_test.go index 0db93619c..0419a469b 100644 --- a/java/bootclasspath_fragment_test.go +++ b/java/bootclasspath_fragment_test.go @@ -113,19 +113,6 @@ func TestBootclasspathFragmentWithoutImageNameOrContents(t *testing.T) { `) } -func TestBootclasspathFragmentWithImageNameAndContents(t *testing.T) { - prepareForTestWithBootclasspathFragment. - ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( - `\Qboth of the "image_name" and "contents" properties\E`)). - RunTestWithBp(t, ` - bootclasspath_fragment { - name: "bootclasspath-fragment", - image_name: "boot", - contents: ["other"], - } - `) -} - func TestBootclasspathFragment_Coverage(t *testing.T) { prepareForTestWithFrameworkCoverage := android.FixtureMergeEnv(map[string]string{ "EMMA_INSTRUMENT": "true",