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

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.
+		android.FixtureWithRootAndroidBp(`
-		java.FixtureConfigureBootJars("com.android.art:foo", "com.android.art:bar"),
-	).RunTestWithBp(t, `
 		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.
+	contentsInsert := func(contents []string) string {
-		prebuilt_bootclasspath_fragment {
+		insert := ""
-			name: "mybootclasspathfragment",
+		if contents != nil {
-			image_name: "art",
+			insert = fmt.Sprintf(`contents: ["%s"],`, strings.Join(contents, `", "`))
-			prefer: true,
-			apex_available: [
-				"com.android.art",
-			],
 		}
-	`)
+		return insert
+	}
 
-	ensureExactContents(t, result.TestContext, "com.android.art", "android_common_com.android.art_image", []string{
+	addSource := func(contents ...string) android.FixturePreparer {
-		"javalib/arm/boot.art",
+		text := fmt.Sprintf(`
-		"javalib/arm/boot.oat",
+			bootclasspath_fragment {
-		"javalib/arm/boot.vdex",
+				name: "mybootclasspathfragment",
-		"javalib/arm/boot-bar.art",
+				image_name: "art",
-		"javalib/arm/boot-bar.oat",
+				%s
-		"javalib/arm/boot-bar.vdex",
+				apex_available: [
-		"javalib/arm64/boot.art",
+					"com.android.art",
-		"javalib/arm64/boot.oat",
+				],
-		"javalib/arm64/boot.vdex",
+			}
-		"javalib/arm64/boot-bar.art",
+		`, contentsInsert(contents))
-		"javalib/arm64/boot-bar.oat",
+
-		"javalib/arm64/boot-bar.vdex",
+		return android.FixtureAddTextFile("art/build/boot/Android.bp", text)
-		"javalib/bar.jar",
+	}
-		"javalib/foo.jar",
+
+	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{
+	t.Run("source with inconsistency between config and contents", func(t *testing.T) {
-		`bar`,
+		android.GroupFixturePreparers(
-		`com.android.art.key`,
+			commonPreparer,
-		`mybootclasspathfragment`,
+
+			// 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)
 	})
 }
 

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)
 

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",