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)
This commit is contained in:
Paul Duffin 2021-05-21 19:28:09 +01:00
parent 0788cd6038
commit 588e22a6bc
3 changed files with 91 additions and 80 deletions

View File

@ -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
}
// 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

View File

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

View File

@ -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)