Allow contents and image_name to be specified together
Previously, only one of the contents or image_name properties could be
specified at once which meant that there was no way to create a
prebuilt which lists its fixed contents while at the same time allowing
it to check that that the contents matched what the build configuration
required.
e.g. a prebuilt_bootclasspath_fragment that had image_name: "art",
could not list its contents and also check that those contents matched
the ART_APEX_JARS which the build configuration required.
This change allows contents and image_name to be specified together and
adds a check to make sure that the contents are consistent with the
configuration appropriate to the image_name. The check is only
performed for modules that are active so that a
prebuilt_bootclasspath_fragment which was created without coverage
enabled (the default) would not cause a build failure in a coverage
build unless it was preferred.
Bug: 177892522
Test: m nothing
Change-Id: Ie601f29f707b3f6030fa7d252afa2c4826cc9f8e
diff --git a/apex/bootclasspath_fragment_test.go b/apex/bootclasspath_fragment_test.go
index feda482..3d39d34 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 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 @@
],
}
- bootclasspath_fragment {
- name: "mybootclasspathfragment",
- image_name: "art",
- apex_available: [
- "com.android.art",
- ],
- }
-
java_import {
name: "foo",
jars: ["foo.jar"],
@@ -231,39 +222,141 @@
"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 8bb5cb1..d0570b4 100644
--- a/java/bootclasspath_fragment.go
+++ b/java/bootclasspath_fragment.go
@@ -17,6 +17,7 @@
import (
"fmt"
"path/filepath"
+ "reflect"
"strings"
"android/soong/android"
@@ -128,8 +129,10 @@
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 @@
// 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 @@
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) 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 0db9361..0419a46 100644
--- a/java/bootclasspath_fragment_test.go
+++ b/java/bootclasspath_fragment_test.go
@@ -113,19 +113,6 @@
`)
}
-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",