From 35881365b49235b2311f48bfcbaff8f873f19b88 Mon Sep 17 00:00:00 2001 From: Sam Delmerico Date: Fri, 30 Jun 2023 14:40:10 -0400 Subject: [PATCH] don't export systemserverclasspath_fragment if contents are empty If a systemserverclasspath_fragment only contains libraries that have a higher min_sdk_version than the target build release version, then we should not export the systemserverclasspath_fragment. Before this change, the fragment was exported with an empty `contents` property which caused errors after being dropped as a prebuilt. Bug: 289183551 Test: go test ./sdk Change-Id: Ifefc6880228e4dd37f5e42b2bda31a83df785375 --- sdk/bootclasspath_fragment_sdk_test.go | 55 +++++++++++++++++++ sdk/bp.go | 10 ++-- ...systemserverclasspath_fragment_sdk_test.go | 45 +++++++++++++++ sdk/update.go | 36 ++++++++++-- 4 files changed, 136 insertions(+), 10 deletions(-) diff --git a/sdk/bootclasspath_fragment_sdk_test.go b/sdk/bootclasspath_fragment_sdk_test.go index 894109a79..d830968f6 100644 --- a/sdk/bootclasspath_fragment_sdk_test.go +++ b/sdk/bootclasspath_fragment_sdk_test.go @@ -1194,3 +1194,58 @@ java_sdk_library_import { expectedSnapshot, expectedCopyRules, expectedStubFlagsInputs, "") }) } + +func TestSnapshotWithEmptyBootClasspathFragment(t *testing.T) { + result := android.GroupFixturePreparers( + prepareForSdkTestWithJava, + java.PrepareForTestWithJavaDefaultModules, + java.PrepareForTestWithJavaSdkLibraryFiles, + java.FixtureWithLastReleaseApis("mysdklibrary", "mynewsdklibrary"), + java.FixtureConfigureApexBootJars("myapex:mysdklibrary", "myapex:mynewsdklibrary"), + prepareForSdkTestWithApex, + // Add a platform_bootclasspath that depends on the fragment. + fixtureAddPlatformBootclasspathForBootclasspathFragment("myapex", "mybootclasspathfragment"), + android.FixtureMergeEnv(map[string]string{ + "SOONG_SDK_SNAPSHOT_TARGET_BUILD_RELEASE": "S", + }), + android.FixtureWithRootAndroidBp(` + sdk { + name: "mysdk", + apexes: ["myapex"], + } + apex { + name: "myapex", + key: "myapex.key", + min_sdk_version: "S", + bootclasspath_fragments: ["mybootclasspathfragment"], + } + bootclasspath_fragment { + name: "mybootclasspathfragment", + apex_available: ["myapex"], + contents: ["mysdklibrary", "mynewsdklibrary"], + hidden_api: { + split_packages: [], + }, + } + java_sdk_library { + name: "mysdklibrary", + apex_available: ["myapex"], + srcs: ["Test.java"], + shared_library: false, + public: {enabled: true}, + min_sdk_version: "Tiramisu", + } + java_sdk_library { + name: "mynewsdklibrary", + apex_available: ["myapex"], + srcs: ["Test.java"], + compile_dex: true, + public: {enabled: true}, + min_sdk_version: "Tiramisu", + permitted_packages: ["mynewsdklibrary"], + } + `), + ).RunTest(t) + + CheckSnapshot(t, result, "mysdk", "", checkAndroidBpContents(`// This is auto-generated. DO NOT EDIT.`)) +} diff --git a/sdk/bp.go b/sdk/bp.go index 7ff85a121..57eb2ca38 100644 --- a/sdk/bp.go +++ b/sdk/bp.go @@ -311,13 +311,15 @@ func (t identityTransformation) transformProperty(_ string, value interface{}, t } func (m *bpModule) deepCopy() *bpModule { - return m.transform(deepCopyTransformer) + return transformModule(m, deepCopyTransformer) } -func (m *bpModule) transform(transformer bpTransformer) *bpModule { +func transformModule(m *bpModule, transformer bpTransformer) *bpModule { transformedModule := transformer.transformModule(m) - // Copy the contents of the returned property set into the module and then transform that. - transformedModule.bpPropertySet, _ = transformPropertySet(transformer, "", transformedModule.bpPropertySet, nil) + if transformedModule != nil { + // Copy the contents of the returned property set into the module and then transform that. + transformedModule.bpPropertySet, _ = transformPropertySet(transformer, "", transformedModule.bpPropertySet, nil) + } return transformedModule } diff --git a/sdk/systemserverclasspath_fragment_sdk_test.go b/sdk/systemserverclasspath_fragment_sdk_test.go index 66c44c843..7ccc11413 100644 --- a/sdk/systemserverclasspath_fragment_sdk_test.go +++ b/sdk/systemserverclasspath_fragment_sdk_test.go @@ -86,6 +86,51 @@ func testSnapshotWithSystemServerClasspathFragment(t *testing.T, sdk string, tar ) } +func TestSnapshotWithEmptySystemServerClasspathFragment(t *testing.T) { + commonSdk := ` + apex { + name: "myapex", + key: "myapex.key", + min_sdk_version: "Tiramisu", + systemserverclasspath_fragments: ["mysystemserverclasspathfragment"], + } + systemserverclasspath_fragment { + name: "mysystemserverclasspathfragment", + apex_available: ["myapex"], + contents: ["mysdklibrary"], + } + java_sdk_library { + name: "mysdklibrary", + apex_available: ["myapex"], + srcs: ["Test.java"], + min_sdk_version: "34", // UpsideDownCake + } + sdk { + name: "mysdk", + apexes: ["myapex"], + } + ` + + result := android.GroupFixturePreparers( + prepareForSdkTestWithJava, + java.PrepareForTestWithJavaDefaultModules, + java.PrepareForTestWithJavaSdkLibraryFiles, + java.FixtureWithLastReleaseApis("mysdklibrary"), + dexpreopt.FixtureSetApexSystemServerJars("myapex:mysdklibrary"), + android.FixtureModifyEnv(func(env map[string]string) { + // targeting Tiramisu here means that we won't export mysdklibrary + env["SOONG_SDK_SNAPSHOT_TARGET_BUILD_RELEASE"] = "Tiramisu" + }), + android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.Platform_version_active_codenames = []string{"UpsideDownCake"} + }), + prepareForSdkTestWithApex, + android.FixtureWithRootAndroidBp(commonSdk), + ).RunTest(t) + + CheckSnapshot(t, result, "mysdk", "", checkAndroidBpContents(`// This is auto-generated. DO NOT EDIT.`)) +} + func TestSnapshotWithSystemServerClasspathFragment(t *testing.T) { commonSdk := ` diff --git a/sdk/update.go b/sdk/update.go index d3c59b0e4..4c39faea1 100644 --- a/sdk/update.go +++ b/sdk/update.go @@ -455,11 +455,14 @@ be unnecessary as every module in the sdk already has its own licenses property. for _, module := range builder.prebuiltOrder { // Prune any empty property sets. - module = module.transform(pruneEmptySetTransformer{}) + module = transformModule(module, pruneEmptySetTransformer{}) // Transform the module module to make it suitable for use in the snapshot. - module.transform(snapshotTransformer) - bpFile.AddModule(module) + module = transformModule(module, snapshotTransformer) + module = transformModule(module, emptyClasspathContentsTransformation{}) + if module != nil { + bpFile.AddModule(module) + } } // generate Android.bp @@ -835,9 +838,11 @@ type snapshotTransformation struct { } func (t snapshotTransformation) transformModule(module *bpModule) *bpModule { - // If the module is an internal member then use a unique name for it. - name := module.Name() - module.setProperty("name", t.builder.snapshotSdkMemberName(name, true)) + if module != nil { + // If the module is an internal member then use a unique name for it. + name := module.Name() + module.setProperty("name", t.builder.snapshotSdkMemberName(name, true)) + } return module } @@ -850,6 +855,25 @@ func (t snapshotTransformation) transformProperty(_ string, value interface{}, t } } +type emptyClasspathContentsTransformation struct { + identityTransformation +} + +func (t emptyClasspathContentsTransformation) transformModule(module *bpModule) *bpModule { + classpathModuleTypes := []string{ + "prebuilt_bootclasspath_fragment", + "prebuilt_systemserverclasspath_fragment", + } + if module != nil && android.InList(module.moduleType, classpathModuleTypes) { + if contents, ok := module.bpPropertySet.properties["contents"].([]string); ok { + if len(contents) == 0 { + return nil + } + } + } + return module +} + type pruneEmptySetTransformer struct { identityTransformation }