Install sdk variants in unbundled builds and package uninstallable variants am: 3d6f3a02f8 am: 46021831fb

Original change: https://googleplex-android-review.googlesource.com/c/platform/build/soong/+/24086298

Change-Id: I816de606ea7d06e99e8b078a1c5d98c64a8d0d1f
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

8 files changed, 92 insertions, 111 deletions

diff --git a/android/deptag.go b/android/deptag.go
index be5c35c8d..a15443b4a 100644
--- a/android/deptag.go
+++ b/android/deptag.go
@@ -34,10 +34,10 @@ func (i InstallAlwaysNeededDependencyTag) InstallDepNeeded() bool {
 
 var _ InstallNeededDependencyTag = InstallAlwaysNeededDependencyTag{}
 
-// IsInstallDepNeeded returns true if the dependency tag implements the InstallNeededDependencyTag
+// IsInstallDepNeededTag returns true if the dependency tag implements the InstallNeededDependencyTag
 // interface and the InstallDepNeeded returns true, meaning that the installed files of the parent
 // should depend on the installed files of the child.
-func IsInstallDepNeeded(tag blueprint.DependencyTag) bool {
+func IsInstallDepNeededTag(tag blueprint.DependencyTag) bool {
 	if i, ok := tag.(InstallNeededDependencyTag); ok {
 		return i.InstallDepNeeded()
 	}
diff --git a/android/license_metadata.go b/android/license_metadata.go
index 18b63d310..73000a9fa 100644
--- a/android/license_metadata.go
+++ b/android/license_metadata.go
@@ -74,7 +74,7 @@ func buildLicenseMetadata(ctx ModuleContext, licenseMetadataFile WritablePath) {
 		if ctx.OtherModuleHasProvider(dep, LicenseMetadataProvider) {
 			info := ctx.OtherModuleProvider(dep, LicenseMetadataProvider).(*LicenseMetadataInfo)
 			allDepMetadataFiles = append(allDepMetadataFiles, info.LicenseMetadataPath)
-			if isContainer || IsInstallDepNeeded(ctx.OtherModuleDependencyTag(dep)) {
+			if isContainer || isInstallDepNeeded(dep, ctx.OtherModuleDependencyTag(dep)) {
 				allDepMetadataDepSets = append(allDepMetadataDepSets, info.LicenseMetadataDepSet)
 			}
 
diff --git a/android/module.go b/android/module.go
index ba474530d..76b4e3d11 100644
--- a/android/module.go
+++ b/android/module.go
@@ -925,6 +925,12 @@ type commonProperties struct {
 	// and don't create a rule to install the file.
 	SkipInstall bool `blueprint:"mutated"`
 
+	// UninstallableApexPlatformVariant is set by MakeUninstallable called by the apex
+	// mutator.  MakeUninstallable also sets HideFromMake.  UninstallableApexPlatformVariant
+	// is used to avoid adding install or packaging dependencies into libraries provided
+	// by apexes.
+	UninstallableApexPlatformVariant bool `blueprint:"mutated"`
+
 	// Whether the module has been replaced by a prebuilt
 	ReplacedByPrebuilt bool `blueprint:"mutated"`
 
@@ -2009,6 +2015,7 @@ func (m *ModuleBase) IsSkipInstall() bool {
 // have other side effects, in particular when it adds a NOTICE file target,
 // which other install targets might depend on.
 func (m *ModuleBase) MakeUninstallable() {
+	m.commonProperties.UninstallableApexPlatformVariant = true
 	m.HideFromMake()
 }
 
@@ -2038,13 +2045,19 @@ func (m *ModuleBase) EffectiveLicenseFiles() Paths {
 }
 
 // computeInstallDeps finds the installed paths of all dependencies that have a dependency
-// tag that is annotated as needing installation via the IsInstallDepNeeded method.
+// tag that is annotated as needing installation via the isInstallDepNeeded method.
 func (m *ModuleBase) computeInstallDeps(ctx ModuleContext) ([]*installPathsDepSet, []*packagingSpecsDepSet) {
 	var installDeps []*installPathsDepSet
 	var packagingSpecs []*packagingSpecsDepSet
 	ctx.VisitDirectDeps(func(dep Module) {
-		if IsInstallDepNeeded(ctx.OtherModuleDependencyTag(dep)) && !dep.IsHideFromMake() && !dep.IsSkipInstall() {
-			installDeps = append(installDeps, dep.base().installFilesDepSet)
+		if isInstallDepNeeded(dep, ctx.OtherModuleDependencyTag(dep)) {
+			// Installation is still handled by Make, so anything hidden from Make is not
+			// installable.
+			if !dep.IsHideFromMake() && !dep.IsSkipInstall() {
+				installDeps = append(installDeps, dep.base().installFilesDepSet)
+			}
+			// Add packaging deps even when the dependency is not installed so that uninstallable
+			// modules can still be packaged.  Often the package will be installed instead.
 			packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet)
 		}
 	})
@@ -2052,6 +2065,17 @@ func (m *ModuleBase) computeInstallDeps(ctx ModuleContext) ([]*installPathsDepSe
 	return installDeps, packagingSpecs
 }
 
+// isInstallDepNeeded returns true if installing the output files of the current module
+// should also install the output files of the given dependency and dependency tag.
+func isInstallDepNeeded(dep Module, tag blueprint.DependencyTag) bool {
+	// Don't add a dependency from the platform to a library provided by an apex.
+	if dep.base().commonProperties.UninstallableApexPlatformVariant {
+		return false
+	}
+	// Only install modules if the dependency tag is an InstallDepNeeded tag.
+	return IsInstallDepNeededTag(tag)
+}
+
 func (m *ModuleBase) FilesToInstall() InstallPaths {
 	return m.installFiles
 }
diff --git a/android/packaging_test.go b/android/packaging_test.go
index 91ac1f386..383343723 100644
--- a/android/packaging_test.go
+++ b/android/packaging_test.go
@@ -373,7 +373,7 @@ func TestPackagingBaseSingleTarget(t *testing.T) {
 
 func TestPackagingWithSkipInstallDeps(t *testing.T) {
 	// package -[dep]-> foo -[dep]-> bar      -[dep]-> baz
-	//                  OK           SKIPPED
+	// Packaging should continue transitively through modules that are not installed.
 	multiTarget := false
 	runPackagingTest(t, multiTarget,
 		`
@@ -396,5 +396,5 @@ func TestPackagingWithSkipInstallDeps(t *testing.T) {
 			name: "package",
 			deps: ["foo"],
 		}
-		`, []string{"lib64/foo"})
+		`, []string{"lib64/foo", "lib64/bar", "lib64/baz"})
 }
diff --git a/apex/apex_test.go b/apex/apex_test.go
index 139b77ef6..e12f758b1 100644
--- a/apex/apex_test.go
+++ b/apex/apex_test.go
@@ -33,6 +33,7 @@ import (
 	"android/soong/cc"
 	"android/soong/dexpreopt"
 	prebuilt_etc "android/soong/etc"
+	"android/soong/filesystem"
 	"android/soong/java"
 	"android/soong/rust"
 	"android/soong/sh"
@@ -10421,3 +10422,54 @@ func TestCannedFsConfig_HasCustomConfig(t *testing.T) {
 	// Ensure that canned_fs_config has "cat my_config" at the end
 	ensureContains(t, cmd, `( echo '/ 1000 1000 0755'; echo '/apex_manifest.json 1000 1000 0644'; echo '/apex_manifest.pb 1000 1000 0644'; cat my_config ) >`)
 }
+
+func TestFileSystemShouldSkipApexLibraries(t *testing.T) {
+	context := android.GroupFixturePreparers(
+		android.PrepareForIntegrationTestWithAndroid,
+		cc.PrepareForIntegrationTestWithCc,
+		PrepareForTestWithApexBuildComponents,
+		prepareForTestWithMyapex,
+		filesystem.PrepareForTestWithFilesystemBuildComponents,
+	)
+	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",
+			stl: "none",
+			apex_available: ["myapex"],
+		}
+
+		apex {
+			name: "myapex",
+			native_shared_libs: ["libbar"],
+			key: "myapex.key",
+			updatable: false,
+		}
+
+		apex_key {
+			name: "myapex.key",
+			public_key: "testkey.avbpubkey",
+			private_key: "testkey.pem",
+		}
+	`)
+
+	inputs := result.ModuleForTests("myfilesystem", "android_common").Output("deps.zip").Implicits
+	android.AssertStringListDoesNotContain(t, "filesystem should not have libbar",
+		inputs.Strings(),
+		"out/soong/.intermediates/libbar/android_arm64_armv8-a_shared/libbar.so")
+}
diff --git a/cc/androidmk.go b/cc/androidmk.go
index 980dd0762..ce35b5c44 100644
--- a/cc/androidmk.go
+++ b/cc/androidmk.go
@@ -124,17 +124,14 @@ func (c *Module) AndroidMkEntries() []android.AndroidMkEntries {
 						}
 					}
 				}
-				if c.Properties.IsSdkVariant {
+				if c.Properties.IsSdkVariant && c.Properties.SdkAndPlatformVariantVisibleToMake {
 					// Make the SDK variant uninstallable so that there are not two rules to install
 					// to the same location.
 					entries.SetBool("LOCAL_UNINSTALLABLE_MODULE", true)
-
-					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))")
-					}
+					// 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 4f361eb16..6341926ff 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() {
@@ -74,8 +74,8 @@ func sdkMutator(ctx android.BottomUpMutatorContext) {
 					if apiLibrary.hasApexStubs() {
 						// For an unbundled apps build, hide the platform variant from Make.
 						modules[1].(*Module).Properties.HideFromMake = true
-						modules[1].(*Module).Properties.PreventInstall = true
 					}
+					modules[1].(*Module).Properties.PreventInstall = true
 				} else {
 					// For a platform build, mark the SDK variant so that it gets a ".sdk" suffix when
 					// exposed to Make.
diff --git a/cc/sdk_test.go b/cc/sdk_test.go
index 790440cb9..61925e30c 100644
--- a/cc/sdk_test.go
+++ b/cc/sdk_test.go
@@ -101,95 +101,3 @@ 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)
-			}
-		})
-	}
-}