Install sdk variants in unbundled builds and package uninstallable variants

This effectively undoes both If6c3ee82d588e2742c85cef7244c090c93f38b8e
and I682e4f1f477f3024f7719dfaa67006ef335e0640.  SDK variants are now
installed again, which will fix unbundled builds of cc_test modules.
The platform variants used by com.android.virt are now packagable
even though they are not installable.

Fix the original problem in b/194403710 by adding a flag to platform
variants of modules in apexes that are not platform available, and
using that to prevent install and packaging dependencies.  That
allows the HideFromMake flag to go back to being used for preventing
install dependencies but not packaging dependencies.

Test: TestPackagingWithSkipInstallDeps
Test: TestFileSystemShouldInstallCoreVariantIfTargetBuildAppsIsSet
Test: TestFileSystemShouldSkipApexLibraries
Bug: 194403710
Bug: 268582372
Fixes: 274443025
Bug: 274028926
Change-Id: If5418df3ddbb940bd631caebdf38daa81e71f40e
Merged-In: If5418df3ddbb940bd631caebdf38daa81e71f40e
(cherry picked from commit bd3a16b5e7)

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()
 	}

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

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() {
+		if isInstallDepNeeded(dep, ctx.OtherModuleDependencyTag(dep)) {
-			installDeps = append(installDeps, dep.base().installFilesDepSet)
+			// 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
 }

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"})
 }

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")
+}

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)
-
+					// Add the unsuffixed name to SOONG_SDK_VARIANT_MODULES so that Make can rewrite
-					if c.Properties.SdkAndPlatformVariantVisibleToMake {
+					// dependencies to the .sdk suffix when building a module that uses the SDK.
-						// Add the unsuffixed name to SOONG_SDK_VARIANT_MODULES so that Make can rewrite
+					entries.SetString("SOONG_SDK_VARIANT_MODULES",
-						// dependencies to the .sdk suffix when building a module that uses the SDK.
+						"$(SOONG_SDK_VARIANT_MODULES) $(patsubst %.sdk,%,$(LOCAL_MODULE))")
-						entries.SetString("SOONG_SDK_VARIANT_MODULES",
-							"$(SOONG_SDK_VARIANT_MODULES) $(patsubst %.sdk,%,$(LOCAL_MODULE))")
-					}
 				}
 			},
 		},

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.

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