From 54f780505dadcd3f6760699ca6475c6798cd740c Mon Sep 17 00:00:00 2001 From: Jooyung Han Date: Mon, 20 Feb 2023 18:17:47 +0900 Subject: [PATCH] Prevent sdk variants from install Platform variants should be available even when unbundled_build so that unbundled build of com.android.virt apex can correctly have microdroid image with system variants. Bug: 268582372 Test: m (soong test) Test: banchan com.android.virt aosp_arm64 UNBUNDLED_BUILD_SDKS_FROM_SOURCE=true m apps_only dist (microdroid should have the same contents) Change-Id: I682e4f1f477f3024f7719dfaa67006ef335e0640 --- cc/androidmk.go | 13 +++-- cc/sdk.go | 11 +++-- cc/sdk_test.go | 92 +++++++++++++++++++++++++++++++++++ filesystem/filesystem_test.go | 37 ++++++++++++++ 4 files changed, 144 insertions(+), 9 deletions(-) diff --git a/cc/androidmk.go b/cc/androidmk.go index ce35b5c44..980dd0762 100644 --- a/cc/androidmk.go +++ b/cc/androidmk.go @@ -124,14 +124,17 @@ func (c *Module) AndroidMkEntries() []android.AndroidMkEntries { } } } - if c.Properties.IsSdkVariant && c.Properties.SdkAndPlatformVariantVisibleToMake { + if c.Properties.IsSdkVariant { // Make the SDK variant uninstallable so that there are not two rules to install // to the same location. entries.SetBool("LOCAL_UNINSTALLABLE_MODULE", true) - // Add the unsuffixed name to SOONG_SDK_VARIANT_MODULES so that Make can rewrite - // dependencies to the .sdk suffix when building a module that uses the SDK. - entries.SetString("SOONG_SDK_VARIANT_MODULES", - "$(SOONG_SDK_VARIANT_MODULES) $(patsubst %.sdk,%,$(LOCAL_MODULE))") + + if c.Properties.SdkAndPlatformVariantVisibleToMake { + // Add the unsuffixed name to SOONG_SDK_VARIANT_MODULES so that Make can rewrite + // dependencies to the .sdk suffix when building a module that uses the SDK. + entries.SetString("SOONG_SDK_VARIANT_MODULES", + "$(SOONG_SDK_VARIANT_MODULES) $(patsubst %.sdk,%,$(LOCAL_MODULE))") + } } }, }, diff --git a/cc/sdk.go b/cc/sdk.go index 3e50c9f4b..23dd0cb2d 100644 --- a/cc/sdk.go +++ b/cc/sdk.go @@ -47,16 +47,16 @@ func sdkMutator(ctx android.BottomUpMutatorContext) { // Mark the SDK variant. modules[1].(*Module).Properties.IsSdkVariant = true + // SDK variant is not supposed to be installed + modules[1].(*Module).Properties.PreventInstall = true if ctx.Config().UnbundledBuildApps() { // For an unbundled apps build, hide the platform variant from Make. modules[0].(*Module).Properties.HideFromMake = true - modules[0].(*Module).Properties.PreventInstall = true } else { // For a platform build, mark the SDK variant so that it gets a ".sdk" suffix when // exposed to Make. modules[1].(*Module).Properties.SdkAndPlatformVariantVisibleToMake = true - modules[1].(*Module).Properties.PreventInstall = true } ctx.AliasVariation("") } else if isCcModule && ccModule.isImportedApiLibrary() { @@ -64,16 +64,19 @@ func sdkMutator(ctx android.BottomUpMutatorContext) { if apiLibrary.hasNDKStubs() && ccModule.canUseSdk() { // Handle cc_api_library module with NDK stubs and variants only which can use SDK modules := ctx.CreateVariations("", "sdk") + + // Mark the SDK variant. modules[1].(*Module).Properties.IsSdkVariant = true + // SDK variant is not supposed to be installed + modules[1].(*Module).Properties.PreventInstall = true + if ctx.Config().UnbundledBuildApps() { // For an unbundled apps build, hide the platform variant from Make. modules[0].(*Module).Properties.HideFromMake = true - modules[0].(*Module).Properties.PreventInstall = true } else { // For a platform build, mark the SDK variant so that it gets a ".sdk" suffix when // exposed to Make. modules[1].(*Module).Properties.SdkAndPlatformVariantVisibleToMake = true - modules[1].(*Module).Properties.PreventInstall = true } } else { ccModule.Properties.Sdk_version = nil diff --git a/cc/sdk_test.go b/cc/sdk_test.go index 61925e30c..790440cb9 100644 --- a/cc/sdk_test.go +++ b/cc/sdk_test.go @@ -101,3 +101,95 @@ func TestSdkMutator(t *testing.T) { assertDep(t, libsdkNDK, libcxxNDK) assertDep(t, libsdkPlatform, libcxxPlatform) } + +func TestMakeModuleNameForSdkVariant(t *testing.T) { + bp := ` + cc_library { + name: "libfoo", + srcs: ["main_test.cpp"], + sdk_version: "current", + stl: "none", + } + ` + platformVariant := "android_arm64_armv8-a_shared" + sdkVariant := "android_arm64_armv8-a_sdk_shared" + testCases := []struct { + name string + unbundledApps []string + variant string + skipInstall bool // soong skips install + hideFromMake bool // no make entry + makeUninstallable bool // make skips install + makeModuleName string + }{ + { + name: "platform variant in normal builds", + unbundledApps: nil, + variant: platformVariant, + // installable in soong + skipInstall: false, + // visiable in Make as "libfoo" + hideFromMake: false, + makeModuleName: "libfoo", + // installable in Make + makeUninstallable: false, + }, + { + name: "sdk variant in normal builds", + unbundledApps: nil, + variant: sdkVariant, + // soong doesn't install + skipInstall: true, + // visible in Make as "libfoo.sdk" + hideFromMake: false, + makeModuleName: "libfoo.sdk", + // but not installed + makeUninstallable: true, + }, + { + name: "platform variant in unbunded builds", + unbundledApps: []string{"bar"}, + variant: platformVariant, + // installable in soong + skipInstall: false, + // hidden from make + hideFromMake: true, + }, + { + name: "sdk variant in unbunded builds", + unbundledApps: []string{"bar"}, + variant: sdkVariant, + // soong doesn't install + skipInstall: true, + // visible in Make as "libfoo" + hideFromMake: false, + makeModuleName: "libfoo", + // but not installed + makeUninstallable: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + fixture := android.GroupFixturePreparers(prepareForCcTest, + android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.Unbundled_build_apps = tc.unbundledApps + }), + ) + ctx := fixture.RunTestWithBp(t, bp).TestContext + module := ctx.ModuleForTests("libfoo", tc.variant).Module().(*Module) + android.AssertBoolEquals(t, "IsSkipInstall", tc.skipInstall, module.IsSkipInstall()) + android.AssertBoolEquals(t, "HideFromMake", tc.hideFromMake, module.HiddenFromMake()) + if !tc.hideFromMake { + entries := android.AndroidMkEntriesForTest(t, ctx, module)[0] + android.AssertStringEquals(t, "LOCAL_MODULE", + tc.makeModuleName, entries.EntryMap["LOCAL_MODULE"][0]) + actualUninstallable := false + if actual, ok := entries.EntryMap["LOCAL_UNINSTALLABLE_MODULE"]; ok { + actualUninstallable = "true" == actual[0] + } + android.AssertBoolEquals(t, "LOCAL_UNINSTALLABLE_MODULE", + tc.makeUninstallable, actualUninstallable) + } + }) + } +} diff --git a/filesystem/filesystem_test.go b/filesystem/filesystem_test.go index 444ffd0aa..a65d9dd5f 100644 --- a/filesystem/filesystem_test.go +++ b/filesystem/filesystem_test.go @@ -188,3 +188,40 @@ func TestAvbAddHashFooter(t *testing.T) { android.AssertStringDoesContain(t, "Can't find --include_descriptors_from_image", cmd, "--include_descriptors_from_image ") } + +func TestFileSystemShouldInstallCoreVariantIfTargetBuildAppsIsSet(t *testing.T) { + context := android.GroupFixturePreparers( + fixture, + android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { + variables.Unbundled_build_apps = []string{"bar"} + }), + ) + result := context.RunTestWithBp(t, ` + android_system_image { + name: "myfilesystem", + deps: [ + "libfoo", + ], + linker_config_src: "linker.config.json", + } + + cc_library { + name: "libfoo", + shared_libs: [ + "libbar", + ], + stl: "none", + } + + cc_library { + name: "libbar", + sdk_version: "9", + stl: "none", + } + `) + + inputs := result.ModuleForTests("myfilesystem", "android_common").Output("deps.zip").Implicits + android.AssertStringListContains(t, "filesystem should have libbar even for unbundled build", + inputs.Strings(), + "out/soong/.intermediates/libbar/android_arm64_armv8-a_shared/libbar.so") +}