Tighten bootclasspath_fragment property validation

Previously, due to legacy reasons, the property validation did not
require a contents property and allowed the image_name to be either
"art" or "boot". Those reasons no longer apply and so this change
requires a contents property and only allows the image_name to be set
to "art" if specified.

Bug: 177892522
Test: m nothing
Merged-In: I8855d6e5365ef0b55490e90e7b6c0081cf070ee5
Change-Id: I8855d6e5365ef0b55490e90e7b6c0081cf070ee5
(cherry picked from commit 8018e50ddb141dea7891918c4178a7a394a788d8)

3 files changed, 87 insertions, 76 deletions

diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go
index fa51b13f6..188d36225 100644
--- a/java/bootclasspath_fragment.go
+++ b/java/bootclasspath_fragment.go
@@ -168,61 +168,70 @@ func bootclasspathFragmentFactory() android.Module {
 // necessary.
 func bootclasspathFragmentInitContentsFromImage(ctx android.EarlyModuleContext, m *BootclasspathFragmentModule) {
 	contents := m.properties.Contents
-	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 len(contents) == 0 {
+		ctx.PropertyErrorf("contents", "required property is missing")
+		return
+	}
+
+	if m.properties.Image_name == nil {
+		// Nothing to do.
+		return
 	}
 
 	imageName := proptools.String(m.properties.Image_name)
-	if imageName == "art" {
-		// TODO(b/177892522): Prebuilts (versioned or not) should not use the image_name property.
-		if android.IsModuleInVersionedSdk(m) {
-			// 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
-		}
+	if imageName != "art" {
+		ctx.PropertyErrorf("image_name", `unknown image name %q, expected "art"`, imageName)
+		return
+	}
 
-		// Get the configuration for the art apex jars. Do not use getImageConfig(ctx) here as this is
-		// too early in the Soong processing for that to work.
-		global := dexpreopt.GetGlobalConfig(ctx)
-		modules := global.ArtApexJars
-
-		// Make sure that the apex specified in the configuration is consistent and is one for which
-		// this boot image is available.
-		commonApex := ""
-		for i := 0; i < modules.Len(); i++ {
-			apex := modules.Apex(i)
-			jar := modules.Jar(i)
-			if apex == "platform" {
-				ctx.ModuleErrorf("ArtApexJars is invalid as it requests a platform variant of %q", jar)
-				continue
-			}
-			if !m.AvailableFor(apex) {
-				ctx.ModuleErrorf("ArtApexJars configuration incompatible with this module, ArtApexJars expects this to be in apex %q but this is only in apexes %q",
-					apex, m.ApexAvailable())
-				continue
-			}
-			if commonApex == "" {
-				commonApex = apex
-			} else if commonApex != apex {
-				ctx.ModuleErrorf("ArtApexJars configuration is inconsistent, expected all jars to be in the same apex but it specifies apex %q and %q",
-					commonApex, apex)
-			}
-		}
+	// TODO(b/177892522): Prebuilts (versioned or not) should not use the image_name property.
+	if android.IsModuleInVersionedSdk(m) {
+		// 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
+	}
 
-		if len(contents) != 0 {
-			// Nothing to do.
-			return
+	// Get the configuration for the art apex jars. Do not use getImageConfig(ctx) here as this is
+	// too early in the Soong processing for that to work.
+	global := dexpreopt.GetGlobalConfig(ctx)
+	modules := global.ArtApexJars
+
+	// Make sure that the apex specified in the configuration is consistent and is one for which
+	// this boot image is available.
+	commonApex := ""
+	for i := 0; i < modules.Len(); i++ {
+		apex := modules.Apex(i)
+		jar := modules.Jar(i)
+		if apex == "platform" {
+			ctx.ModuleErrorf("ArtApexJars is invalid as it requests a platform variant of %q", jar)
+			continue
+		}
+		if !m.AvailableFor(apex) {
+			ctx.ModuleErrorf("ArtApexJars configuration incompatible with this module, ArtApexJars expects this to be in apex %q but this is only in apexes %q",
+				apex, m.ApexAvailable())
+			continue
 		}
+		if commonApex == "" {
+			commonApex = apex
+		} else if commonApex != apex {
+			ctx.ModuleErrorf("ArtApexJars configuration is inconsistent, expected all jars to be in the same apex but it specifies apex %q and %q",
+				commonApex, apex)
+		}
+	}
 
-		// Store the jars in the Contents property so that they can be used to add dependencies.
-		m.properties.Contents = modules.CopyOfJars()
+	if len(contents) != 0 {
+		// Nothing to do.
+		return
 	}
+
+	// Store the jars in the Contents property so that they can be used to add dependencies.
+	m.properties.Contents = modules.CopyOfJars()
 }
 
 // bootclasspathImageNameContentsConsistencyCheck checks that the configuration that applies to this
diff --git a/java/bootclasspath_fragment_test.go b/java/bootclasspath_fragment_test.go
index 581625d8c..fba7d1a71 100644
--- a/java/bootclasspath_fragment_test.go
+++ b/java/bootclasspath_fragment_test.go
@@ -29,38 +29,28 @@ var prepareForTestWithBootclasspathFragment = android.GroupFixturePreparers(
 	dexpreopt.PrepareForTestByEnablingDexpreopt,
 )
 
-func TestUnknownBootclasspathFragment(t *testing.T) {
+func TestBootclasspathFragment_UnknownImageName(t *testing.T) {
 	prepareForTestWithBootclasspathFragment.
 		ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(
-			`\Qimage_name: Unknown image name "unknown", expected one of art, boot\E`)).
+			`\Qimage_name: unknown image name "unknown", expected "art"\E`)).
 		RunTestWithBp(t, `
 			bootclasspath_fragment {
 				name: "unknown-bootclasspath-fragment",
 				image_name: "unknown",
+				contents: ["foo"],
 			}
 		`)
 }
 
-func TestUnknownBootclasspathFragmentImageName(t *testing.T) {
+func TestPrebuiltBootclasspathFragment_UnknownImageName(t *testing.T) {
 	prepareForTestWithBootclasspathFragment.
 		ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(
-			`\Qimage_name: Unknown image name "unknown", expected one of art, boot\E`)).
-		RunTestWithBp(t, `
-			bootclasspath_fragment {
-				name: "unknown-bootclasspath-fragment",
-				image_name: "unknown",
-			}
-		`)
-}
-
-func TestUnknownPrebuiltBootclasspathFragment(t *testing.T) {
-	prepareForTestWithBootclasspathFragment.
-		ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(
-			`\Qimage_name: Unknown image name "unknown", expected one of art, boot\E`)).
+			`\Qimage_name: unknown image name "unknown", expected "art"\E`)).
 		RunTestWithBp(t, `
 			prebuilt_bootclasspath_fragment {
 				name: "unknown-bootclasspath-fragment",
 				image_name: "unknown",
+				contents: ["foo"],
 			}
 		`)
 }
@@ -76,6 +66,7 @@ func TestBootclasspathFragmentInconsistentArtConfiguration_Platform(t *testing.T
 			bootclasspath_fragment {
 				name: "bootclasspath-fragment",
 				image_name: "art",
+				contents: ["foo", "bar"],
 				apex_available: [
 					"apex",
 				],
@@ -94,6 +85,7 @@ func TestBootclasspathFragmentInconsistentArtConfiguration_ApexMixture(t *testin
 			bootclasspath_fragment {
 				name: "bootclasspath-fragment",
 				image_name: "art",
+				contents: ["foo", "bar"],
 				apex_available: [
 					"apex1",
 					"apex2",
@@ -102,17 +94,6 @@ func TestBootclasspathFragmentInconsistentArtConfiguration_ApexMixture(t *testin
 		`)
 }
 
-func TestBootclasspathFragmentWithoutImageNameOrContents(t *testing.T) {
-	prepareForTestWithBootclasspathFragment.
-		ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(
-			`\Qneither of the "image_name" and "contents" properties\E`)).
-		RunTestWithBp(t, `
-			bootclasspath_fragment {
-				name: "bootclasspath-fragment",
-			}
-		`)
-}
-
 func TestBootclasspathFragment_Coverage(t *testing.T) {
 	prepareForTestWithFrameworkCoverage := android.FixtureMergeEnv(map[string]string{
 		"EMMA_INSTRUMENT":           "true",
diff --git a/sdk/bootclasspath_fragment_sdk_test.go b/sdk/bootclasspath_fragment_sdk_test.go
index bd69f06f2..d9fe2816f 100644
--- a/sdk/bootclasspath_fragment_sdk_test.go
+++ b/sdk/bootclasspath_fragment_sdk_test.go
@@ -424,6 +424,7 @@ func TestBasicSdkWithBootclasspathFragment(t *testing.T) {
 	android.GroupFixturePreparers(
 		prepareForSdkTestWithApex,
 		prepareForSdkTestWithJava,
+		android.FixtureAddFile("java/mybootlib.jar", nil),
 		android.FixtureWithRootAndroidBp(`
 		sdk {
 			name: "mysdk",
@@ -433,16 +434,27 @@ func TestBasicSdkWithBootclasspathFragment(t *testing.T) {
 		bootclasspath_fragment {
 			name: "mybootclasspathfragment",
 			image_name: "art",
+			contents: ["mybootlib"],
 			apex_available: ["myapex"],
 		}
 
+		java_library {
+			name: "mybootlib",
+			apex_available: ["myapex"],
+			srcs: ["Test.java"],
+			system_modules: "none",
+			sdk_version: "none",
+			min_sdk_version: "1",
+			compile_dex: true,
+		}
+
 		sdk_snapshot {
 			name: "mysdk@1",
-			bootclasspath_fragments: ["mybootclasspathfragment_mysdk_1"],
+			bootclasspath_fragments: ["mysdk_mybootclasspathfragment@1"],
 		}
 
 		prebuilt_bootclasspath_fragment {
-			name: "mybootclasspathfragment_mysdk_1",
+			name: "mysdk_mybootclasspathfragment@1",
 			sdk_member_name: "mybootclasspathfragment",
 			prefer: false,
 			visibility: ["//visibility:public"],
@@ -450,6 +462,15 @@ func TestBasicSdkWithBootclasspathFragment(t *testing.T) {
 				"myapex",
 			],
 			image_name: "art",
+			contents: ["mysdk_mybootlib@1"],
+		}
+
+		java_import {
+			name: "mysdk_mybootlib@1",
+			sdk_member_name: "mybootlib",
+			visibility: ["//visibility:public"],
+			apex_available: ["com.android.art"],
+			jars: ["java/mybootlib.jar"],
 		}
 	`),
 	).RunTest(t)