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)
diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go
index fa51b13..188d362 100644
--- a/java/bootclasspath_fragment.go
+++ b/java/bootclasspath_fragment.go
@@ -168,61 +168,70 @@
// 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
- }
-
- // 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)
- }
- }
-
- 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()
+ if imageName != "art" {
+ ctx.PropertyErrorf("image_name", `unknown image name %q, expected "art"`, imageName)
+ return
}
+
+ // 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
+ }
+
+ // 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)
+ }
+ }
+
+ 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 581625d..fba7d1a 100644
--- a/java/bootclasspath_fragment_test.go
+++ b/java/bootclasspath_fragment_test.go
@@ -29,38 +29,28 @@
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 @@
bootclasspath_fragment {
name: "bootclasspath-fragment",
image_name: "art",
+ contents: ["foo", "bar"],
apex_available: [
"apex",
],
@@ -94,6 +85,7 @@
bootclasspath_fragment {
name: "bootclasspath-fragment",
image_name: "art",
+ contents: ["foo", "bar"],
apex_available: [
"apex1",
"apex2",
@@ -102,17 +94,6 @@
`)
}
-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 bd69f06..d9fe281 100644
--- a/sdk/bootclasspath_fragment_sdk_test.go
+++ b/sdk/bootclasspath_fragment_sdk_test.go
@@ -424,6 +424,7 @@
android.GroupFixturePreparers(
prepareForSdkTestWithApex,
prepareForSdkTestWithJava,
+ android.FixtureAddFile("java/mybootlib.jar", nil),
android.FixtureWithRootAndroidBp(`
sdk {
name: "mysdk",
@@ -433,16 +434,27 @@
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 @@
"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)