From d34eb0c4a6e176ca129f66f0449bd0ba19fdff5e Mon Sep 17 00:00:00 2001 From: satayev Date: Fri, 6 Aug 2021 13:20:28 +0100 Subject: [PATCH] Make sure that classpath fragment contents appear in make vars. The source of truth for what jars are in BOOTCLASSPATH and SYSTEMSERVERCLASSPATH is make; adding a classpath fragment in soong with a new jar in the contents without having it declared in make is wrong (excluding test apexes). Bug: 191369843 Test: m nothing Change-Id: Ifd313776ee7ad206031244534ed3870126e4f835 --- android/config.go | 7 +- apex/classpath_element_test.go | 15 -- apex/platform_bootclasspath_test.go | 137 +++++++++++++++++++ apex/systemserver_classpath_fragment_test.go | 51 +++++++ java/bootclasspath_fragment.go | 8 +- java/systemserver_classpath_fragment.go | 11 +- 6 files changed, 209 insertions(+), 20 deletions(-) diff --git a/android/config.go b/android/config.go index e3d05a67e..29784db85 100644 --- a/android/config.go +++ b/android/config.go @@ -1671,8 +1671,9 @@ func (l *ConfiguredJarList) RemoveList(list ConfiguredJarList) ConfiguredJarList return ConfiguredJarList{apexes, jars} } -// Filter keeps the entries if a jar appears in the given list of jars to keep; returns a new list. -func (l *ConfiguredJarList) Filter(jarsToKeep []string) ConfiguredJarList { +// Filter keeps the entries if a jar appears in the given list of jars to keep. Returns a new list +// and any remaining jars that are not on this list. +func (l *ConfiguredJarList) Filter(jarsToKeep []string) (ConfiguredJarList, []string) { var apexes []string var jars []string @@ -1683,7 +1684,7 @@ func (l *ConfiguredJarList) Filter(jarsToKeep []string) ConfiguredJarList { } } - return ConfiguredJarList{apexes, jars} + return ConfiguredJarList{apexes, jars}, RemoveListFromList(jarsToKeep, jars) } // CopyOfJars returns a copy of the list of strings containing jar module name diff --git a/apex/classpath_element_test.go b/apex/classpath_element_test.go index e2d846559..60f18bd57 100644 --- a/apex/classpath_element_test.go +++ b/apex/classpath_element_test.go @@ -159,11 +159,6 @@ func TestCreateClasspathElements(t *testing.T) { ], } - bootclasspath_fragment { - name: "non-apex-fragment", - contents: ["othersdklibrary"], - } - apex { name: "otherapex", key: "otherapex.key", @@ -213,7 +208,6 @@ func TestCreateClasspathElements(t *testing.T) { myFragment := result.Module("mybootclasspath-fragment", "android_common_apex10000") myBar := result.Module("bar", "android_common_apex10000") - nonApexFragment := result.Module("non-apex-fragment", "android_common") other := result.Module("othersdklibrary", "android_common_apex10000") otherApexLibrary := result.Module("otherapexlibrary", "android_common_apex10000") @@ -253,15 +247,6 @@ func TestCreateClasspathElements(t *testing.T) { assertElementsEquals(t, "elements", expectedElements, elements) }) - // Verify that CreateClasspathElements detects when a fragment does not have an associated apex. - t.Run("non apex fragment", func(t *testing.T) { - ctx := newCtx() - elements := java.CreateClasspathElements(ctx, []android.Module{}, []android.Module{nonApexFragment}) - android.FailIfNoMatchingErrors(t, "fragment non-apex-fragment{.*} is not part of an apex", ctx.errs) - expectedElements := java.ClasspathElements{} - assertElementsEquals(t, "elements", expectedElements, elements) - }) - // Verify that CreateClasspathElements detects when an apex has multiple fragments. t.Run("multiple fragments for same apex", func(t *testing.T) { ctx := newCtx() diff --git a/apex/platform_bootclasspath_test.go b/apex/platform_bootclasspath_test.go index e0421f63d..513ddc07f 100644 --- a/apex/platform_bootclasspath_test.go +++ b/apex/platform_bootclasspath_test.go @@ -543,3 +543,140 @@ func TestPlatformBootclasspath_IncludesRemainingApexJars(t *testing.T) { "out/soong/target/product/test_device/system/etc/classpaths", ) } + +func TestBootJarNotInApex(t *testing.T) { + android.GroupFixturePreparers( + prepareForTestWithPlatformBootclasspath, + PrepareForTestWithApexBuildComponents, + prepareForTestWithMyapex, + java.FixtureConfigureApexBootJars("myapex:foo"), + ).ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( + `dependency "foo" of "myplatform-bootclasspath" missing variant`)). + RunTestWithBp(t, ` + apex { + name: "myapex", + key: "myapex.key", + updatable: false, + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + java_library { + name: "foo", + srcs: ["b.java"], + installable: true, + apex_available: [ + "myapex", + ], + } + + bootclasspath_fragment { + name: "not-in-apex-fragment", + contents: [ + "foo", + ], + } + + platform_bootclasspath { + name: "myplatform-bootclasspath", + } + `) +} + +func TestBootFragmentNotInApex(t *testing.T) { + android.GroupFixturePreparers( + prepareForTestWithPlatformBootclasspath, + PrepareForTestWithApexBuildComponents, + prepareForTestWithMyapex, + java.FixtureConfigureApexBootJars("myapex:foo"), + ).ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( + `library foo.*have no corresponding fragment.*`)).RunTestWithBp(t, ` + apex { + name: "myapex", + key: "myapex.key", + java_libs: ["foo"], + updatable: false, + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + java_library { + name: "foo", + srcs: ["b.java"], + installable: true, + apex_available: ["myapex"], + permitted_packages: ["foo"], + } + + bootclasspath_fragment { + name: "not-in-apex-fragment", + contents: ["foo"], + } + + platform_bootclasspath { + name: "myplatform-bootclasspath", + } + `) +} + +func TestNonBootJarInFragment(t *testing.T) { + android.GroupFixturePreparers( + prepareForTestWithPlatformBootclasspath, + PrepareForTestWithApexBuildComponents, + prepareForTestWithMyapex, + java.FixtureConfigureApexBootJars("myapex:foo"), + ).ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( + `in contents must also be declared in PRODUCT_APEX_BOOT_JARS`)). + RunTestWithBp(t, ` + apex { + name: "myapex", + key: "myapex.key", + bootclasspath_fragments: ["apex-fragment"], + updatable: false, + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + java_library { + name: "foo", + srcs: ["b.java"], + installable: true, + apex_available: ["myapex"], + permitted_packages: ["foo"], + } + + java_library { + name: "bar", + srcs: ["b.java"], + installable: true, + apex_available: ["myapex"], + permitted_packages: ["bar"], + } + + bootclasspath_fragment { + name: "apex-fragment", + contents: ["foo", "bar"], + apex_available:[ "myapex" ], + } + + platform_bootclasspath { + name: "myplatform-bootclasspath", + fragments: [{ + apex: "myapex", + module:"apex-fragment", + }], + } + `) +} diff --git a/apex/systemserver_classpath_fragment_test.go b/apex/systemserver_classpath_fragment_test.go index a64c6f4a2..a8d5931db 100644 --- a/apex/systemserver_classpath_fragment_test.go +++ b/apex/systemserver_classpath_fragment_test.go @@ -130,3 +130,54 @@ func TestSystemserverclasspathFragmentNoGeneratedProto(t *testing.T) { `mysystemserverclasspathfragment`, }) } + +func TestSystemServerClasspathFragmentWithContentNotInMake(t *testing.T) { + android.GroupFixturePreparers( + prepareForTestWithSystemserverclasspathFragment, + prepareForTestWithMyapex, + dexpreopt.FixtureSetApexSystemServerJars("myapex:foo"), + ). + ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern( + `in contents must also be declared in PRODUCT_UPDATABLE_SYSTEM_SERVER_JARS`)). + RunTestWithBp(t, ` + apex { + name: "myapex", + key: "myapex.key", + systemserverclasspath_fragments: [ + "mysystemserverclasspathfragment", + ], + updatable: false, + } + + apex_key { + name: "myapex.key", + public_key: "testkey.avbpubkey", + private_key: "testkey.pem", + } + + java_library { + name: "foo", + srcs: ["b.java"], + installable: true, + apex_available: ["myapex"], + } + + java_library { + name: "bar", + srcs: ["b.java"], + installable: true, + apex_available: ["myapex"], + } + + systemserverclasspath_fragment { + name: "mysystemserverclasspathfragment", + contents: [ + "foo", + "bar", + ], + apex_available: [ + "myapex", + ], + } + `) +} diff --git a/java/bootclasspath_fragment.go b/java/bootclasspath_fragment.go index bb542c49c..757731632 100644 --- a/java/bootclasspath_fragment.go +++ b/java/bootclasspath_fragment.go @@ -538,7 +538,7 @@ func (b *BootclasspathFragmentModule) configuredJars(ctx android.ModuleContext) global := dexpreopt.GetGlobalConfig(ctx) possibleUpdatableModules := gatherPossibleApexModuleNamesAndStems(ctx, b.properties.Contents, bootclasspathFragmentContentDepTag) - jars := global.ApexBootJars.Filter(possibleUpdatableModules) + jars, unknown := global.ApexBootJars.Filter(possibleUpdatableModules) // TODO(satayev): for apex_test we want to include all contents unconditionally to classpaths // config. However, any test specific jars would not be present in ApexBootJars. Instead, @@ -546,6 +546,12 @@ func (b *BootclasspathFragmentModule) configuredJars(ctx android.ModuleContext) // This is an exception to support end-to-end test for SdkExtensions, until such support exists. if android.InList("test_framework-sdkextensions", possibleUpdatableModules) { jars = jars.Append("com.android.sdkext", "test_framework-sdkextensions") + } else if global.ApexBootJars.Len() != 0 && !android.IsModuleInVersionedSdk(ctx.Module()) { + unknown = android.RemoveListFromList(unknown, b.properties.Coverage.Contents) + _, unknown = android.RemoveFromList("core-icu4j", unknown) + if len(unknown) > 0 { + ctx.ModuleErrorf("%s in contents must also be declared in PRODUCT_APEX_BOOT_JARS", unknown) + } } return jars } diff --git a/java/systemserver_classpath_fragment.go b/java/systemserver_classpath_fragment.go index 6c2a5b58b..5311f62c1 100644 --- a/java/systemserver_classpath_fragment.go +++ b/java/systemserver_classpath_fragment.go @@ -107,7 +107,16 @@ func (s *SystemServerClasspathModule) configuredJars(ctx android.ModuleContext) global := dexpreopt.GetGlobalConfig(ctx) possibleUpdatableModules := gatherPossibleApexModuleNamesAndStems(ctx, s.properties.Contents, systemServerClasspathFragmentContentDepTag) - return global.ApexSystemServerJars.Filter(possibleUpdatableModules) + jars, unknown := global.ApexSystemServerJars.Filter(possibleUpdatableModules) + // TODO(satayev): remove geotz ssc_fragment, since geotz is not part of SSCP anymore. + _, unknown = android.RemoveFromList("geotz", unknown) + + // For non test apexes, make sure that all contents are actually declared in make. + if global.ApexSystemServerJars.Len() > 0 && len(unknown) > 0 { + ctx.ModuleErrorf("%s in contents must also be declared in PRODUCT_UPDATABLE_SYSTEM_SERVER_JARS", unknown) + } + + return jars } type systemServerClasspathFragmentContentDependencyTag struct {