Merge changes I5645ddb9,Ib3d50f15,Ib4c5815a,If3b63706 into main am: e8cb9178ec

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

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

android/deptag.go

@@ -44,21 +44,6 @@ func IsInstallDepNeededTag(tag blueprint.DependencyTag) bool {
 	return false
 }
 
-// Dependency tags can implement this interface and return true from SkipToTransitiveDeps to
-// annotate that this dependency isn't installed, but its transitive dependencies are. This is
-// useful when a module is built into another module (ex: static linking) but the module still has
-// runtime dependencies.
-type SkipToTransitiveDepsTag interface {
-	SkipToTransitiveDeps() bool
-}
-
-func IsSkipToTransitiveDepsTag(tag blueprint.DependencyTag) bool {
-	if i, ok := tag.(SkipToTransitiveDepsTag); ok {
-		return i.SkipToTransitiveDeps()
-	}
-	return false
-}
-
 type PropagateAconfigValidationDependencyTag interface {
 	PropagateAconfigValidation() bool
 }

android/module.go

@@ -1474,27 +1474,15 @@ func (m *ModuleBase) computeInstallDeps(ctx ModuleContext) ([]*DepSet[InstallPat
 	var installDeps []*DepSet[InstallPath]
 	var packagingSpecs []*DepSet[PackagingSpec]
 	ctx.VisitDirectDeps(func(dep Module) {
-		depTag := ctx.OtherModuleDependencyTag(dep)
+		if isInstallDepNeeded(dep, ctx.OtherModuleDependencyTag(dep)) {
-		// If this is true, the direct outputs from the module is not gathered, but its
-		// transitive deps are still gathered.
-		skipToTransitive := IsSkipToTransitiveDepsTag(depTag)
-		if isInstallDepNeeded(dep, depTag) || skipToTransitive {
 			// Installation is still handled by Make, so anything hidden from Make is not
 			// installable.
 			if !dep.IsHideFromMake() && !dep.IsSkipInstall() {
-				if skipToTransitive {
+				installDeps = append(installDeps, dep.base().installFilesDepSet)
-					installDeps = append(installDeps, dep.base().installFilesDepSet.transitive...)
-				} else {
-					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.
-			if skipToTransitive {
+			packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet)
-				packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet.transitive...)
-			} else {
-				packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet)
-			}
 		}
 	})
 

android/packaging_test.go

@@ -25,9 +25,8 @@ import (
 type componentTestModule struct {
 	ModuleBase
 	props struct {
-		Deps            []string
+		Deps         []string
-		Build_only_deps []string
+		Skip_install *bool
-		Skip_install    *bool
 	}
 }
 
@@ -37,18 +36,6 @@ type installDepTag struct {
 	InstallAlwaysNeededDependencyTag
 }
 
-// dep tag for build_only_deps
-type buildOnlyDepTag struct {
-	blueprint.BaseDependencyTag
-	InstallAlwaysNeededDependencyTag
-}
-
-var _ SkipToTransitiveDepsTag = (*buildOnlyDepTag)(nil)
-
-func (tag buildOnlyDepTag) SkipToTransitiveDeps() bool {
-	return true
-}
-
 func componentTestModuleFactory() Module {
 	m := &componentTestModule{}
 	m.AddProperties(&m.props)
@@ -58,7 +45,6 @@ func componentTestModuleFactory() Module {
 
 func (m *componentTestModule) DepsMutator(ctx BottomUpMutatorContext) {
 	ctx.AddDependency(ctx.Module(), installDepTag{}, m.props.Deps...)
-	ctx.AddDependency(ctx.Module(), buildOnlyDepTag{}, m.props.Build_only_deps...)
 }
 
 func (m *componentTestModule) GenerateAndroidBuildActions(ctx ModuleContext) {
@@ -412,30 +398,3 @@ func TestPackagingWithSkipInstallDeps(t *testing.T) {
 		}
 		`, []string{"lib64/foo", "lib64/bar", "lib64/baz"})
 }
-
-func TestPackagingWithSkipToTransitvDeps(t *testing.T) {
-	// packag -[deps]-> foo -[build_only_deps]-> bar -[deps]-> baz
-	// bar isn't installed, but it brings baz to its parent.
-	multiTarget := false
-	runPackagingTest(t, multiTarget,
-		`
-		component {
-			name: "foo",
-			build_only_deps: ["bar"],
-		}
-
-		component {
-			name: "bar",
-			deps: ["baz"],
-		}
-
-		component {
-			name: "baz",
-		}
-
-		package_module {
-			name: "package",
-			deps: ["foo"],
-		}
-		`, []string{"lib64/foo", "lib64/baz"})
-}

java/androidmk.go

@@ -17,6 +17,7 @@ package java
 import (
 	"fmt"
 	"io"
+	"strings"
 
 	"android/soong/android"
 
@@ -412,6 +413,23 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries {
 				if app.embeddedJniLibs {
 					jniSymbols := app.JNISymbolsInstalls(app.installPathForJNISymbols.String())
 					entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", jniSymbols.String())
+				} else {
+					for _, jniLib := range app.jniLibs {
+						entries.AddStrings("LOCAL_SOONG_JNI_LIBS_"+jniLib.target.Arch.ArchType.String(), jniLib.name)
+						var partitionTag string
+
+						// Mimic the creation of partition_tag in build/make,
+						// which defaults to an empty string when the partition is system.
+						// Otherwise, capitalize with a leading _
+						if jniLib.partition == "system" {
+							partitionTag = ""
+						} else {
+							split := strings.Split(jniLib.partition, "/")
+							partitionTag = "_" + strings.ToUpper(split[len(split)-1])
+						}
+						entries.AddStrings("LOCAL_SOONG_JNI_LIBS_PARTITION_"+jniLib.target.Arch.ArchType.String(),
+							jniLib.name+":"+partitionTag)
+					}
 				}
 
 				if len(app.jniCoverageOutputs) > 0 {

java/androidmk_test.go

@@ -19,6 +19,9 @@ import (
 	"testing"
 
 	"android/soong/android"
+	"android/soong/cc"
+
+	"github.com/google/blueprint/proptools"
 )
 
 func TestRequired(t *testing.T) {
@@ -252,3 +255,149 @@ func TestGetOverriddenPackages(t *testing.T) {
 		android.AssertDeepEquals(t, "overrides property", expected.overrides, actual)
 	}
 }
+
+func TestJniPartition(t *testing.T) {
+	bp := `
+		cc_library {
+			name: "libjni_system",
+			system_shared_libs: [],
+			sdk_version: "current",
+			stl: "none",
+		}
+
+		cc_library {
+			name: "libjni_system_ext",
+			system_shared_libs: [],
+			sdk_version: "current",
+			stl: "none",
+			system_ext_specific: true,
+		}
+
+		cc_library {
+			name: "libjni_odm",
+			system_shared_libs: [],
+			sdk_version: "current",
+			stl: "none",
+			device_specific: true,
+		}
+
+		cc_library {
+			name: "libjni_product",
+			system_shared_libs: [],
+			sdk_version: "current",
+			stl: "none",
+			product_specific: true,
+		}
+
+		cc_library {
+			name: "libjni_vendor",
+			system_shared_libs: [],
+			sdk_version: "current",
+			stl: "none",
+			soc_specific: true,
+		}
+
+		android_app {
+			name: "test_app_system_jni_system",
+			privileged: true,
+			platform_apis: true,
+			certificate: "platform",
+			jni_libs: ["libjni_system"],
+		}
+
+		android_app {
+			name: "test_app_system_jni_system_ext",
+			privileged: true,
+			platform_apis: true,
+			certificate: "platform",
+			jni_libs: ["libjni_system_ext"],
+		}
+
+		android_app {
+			name: "test_app_system_ext_jni_system",
+			privileged: true,
+			platform_apis: true,
+			certificate: "platform",
+			jni_libs: ["libjni_system"],
+			system_ext_specific: true
+		}
+
+		android_app {
+			name: "test_app_system_ext_jni_system_ext",
+			sdk_version: "core_platform",
+			jni_libs: ["libjni_system_ext"],
+			system_ext_specific: true
+		}
+
+		android_app {
+			name: "test_app_product_jni_product",
+			sdk_version: "core_platform",
+			jni_libs: ["libjni_product"],
+			product_specific: true
+		}
+
+		android_app {
+			name: "test_app_vendor_jni_odm",
+			sdk_version: "core_platform",
+			jni_libs: ["libjni_odm"],
+			soc_specific: true
+		}
+
+		android_app {
+			name: "test_app_odm_jni_vendor",
+			sdk_version: "core_platform",
+			jni_libs: ["libjni_vendor"],
+			device_specific: true
+		}
+		android_app {
+			name: "test_app_system_jni_multiple",
+			privileged: true,
+			platform_apis: true,
+			certificate: "platform",
+			jni_libs: ["libjni_system", "libjni_system_ext"],
+		}
+		android_app {
+			name: "test_app_vendor_jni_multiple",
+			sdk_version: "core_platform",
+			jni_libs: ["libjni_odm", "libjni_vendor"],
+			soc_specific: true
+		}
+		`
+	arch := "arm64"
+	ctx := android.GroupFixturePreparers(
+		PrepareForTestWithJavaDefaultModules,
+		cc.PrepareForTestWithCcDefaultModules,
+		android.PrepareForTestWithAndroidMk,
+		android.FixtureModifyConfig(func(config android.Config) {
+			config.TestProductVariables.DeviceArch = proptools.StringPtr(arch)
+		}),
+	).
+		RunTestWithBp(t, bp)
+	testCases := []struct {
+		name           string
+		partitionNames []string
+		partitionTags  []string
+	}{
+		{"test_app_system_jni_system", []string{"libjni_system"}, []string{""}},
+		{"test_app_system_jni_system_ext", []string{"libjni_system_ext"}, []string{"_SYSTEM_EXT"}},
+		{"test_app_system_ext_jni_system", []string{"libjni_system"}, []string{""}},
+		{"test_app_system_ext_jni_system_ext", []string{"libjni_system_ext"}, []string{"_SYSTEM_EXT"}},
+		{"test_app_product_jni_product", []string{"libjni_product"}, []string{"_PRODUCT"}},
+		{"test_app_vendor_jni_odm", []string{"libjni_odm"}, []string{"_ODM"}},
+		{"test_app_odm_jni_vendor", []string{"libjni_vendor"}, []string{"_VENDOR"}},
+		{"test_app_system_jni_multiple", []string{"libjni_system", "libjni_system_ext"}, []string{"", "_SYSTEM_EXT"}},
+		{"test_app_vendor_jni_multiple", []string{"libjni_odm", "libjni_vendor"}, []string{"_ODM", "_VENDOR"}},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.name, func(t *testing.T) {
+			mod := ctx.ModuleForTests(test.name, "android_common").Module()
+			entry := android.AndroidMkEntriesForTest(t, ctx.TestContext, mod)[0]
+			for i := range test.partitionNames {
+				actual := entry.EntryMap["LOCAL_SOONG_JNI_LIBS_PARTITION_"+arch][i]
+				expected := test.partitionNames[i] + ":" + test.partitionTags[i]
+				android.AssertStringEquals(t, "Expected and actual differ", expected, actual)
+			}
+		})
+	}
+}

java/app.go

@@ -90,17 +90,20 @@ type appProperties struct {
 	Stl *string `android:"arch_variant"`
 
 	// Store native libraries uncompressed in the APK and set the android:extractNativeLibs="false" manifest
-	// flag so that they are used from inside the APK at runtime. This property is respected only for
+	// flag so that they are used from inside the APK at runtime.  Defaults to true for android_test modules unless
-	// APKs built using android_test or android_test_helper_app. For other APKs, this property is ignored
+	// sdk_version or min_sdk_version is set to a version that doesn't support it (<23), defaults to true for
-	// and native libraries are always embedded compressed.
+	// android_app modules that are embedded to APEXes, defaults to false for other module types where the native
+	// libraries are generally preinstalled outside the APK.
 	Use_embedded_native_libs *bool
 
 	// Store dex files uncompressed in the APK and set the android:useEmbeddedDex="true" manifest attribute so that
 	// they are used from inside the APK at runtime.
 	Use_embedded_dex *bool
 
-	// Allows compressing of embedded native libs. Only for android_test and android_test_helper_app.
+	// Forces native libraries to always be packaged into the APK,
-	AllowCompressingNativeLibs bool `blueprint:"mutated"`
+	// Use_embedded_native_libs still selects whether they are stored uncompressed and aligned or compressed.
+	// True for android_test* modules.
+	AlwaysPackageNativeLibs bool `blueprint:"mutated"`
 
 	// If set, find and merge all NOTICE files that this module and its dependencies have and store
 	// it in the APK as an asset.
@@ -400,20 +403,14 @@ func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVer
 // Returns true if the native libraries should be stored in the APK uncompressed and the
 // extractNativeLibs application flag should be set to false in the manifest.
 func (a *AndroidApp) useEmbeddedNativeLibs(ctx android.ModuleContext) bool {
-	var useEmbedded bool
-	if a.appProperties.AllowCompressingNativeLibs {
-		useEmbedded = BoolDefault(a.appProperties.Use_embedded_native_libs, true)
-	} else {
-		useEmbedded = true // always uncompress for non-test apps
-	}
-
 	minSdkVersion, err := a.MinSdkVersion(ctx).EffectiveVersion(ctx)
 	if err != nil {
 		ctx.PropertyErrorf("min_sdk_version", "invalid value %q: %s", a.MinSdkVersion(ctx), err)
 	}
-	supported := minSdkVersion.FinalOrFutureInt() >= 23
 
-	return useEmbedded && supported
+	apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider)
+	return (minSdkVersion.FinalOrFutureInt() >= 23 && Bool(a.appProperties.Use_embedded_native_libs)) ||
+		!apexInfo.IsForPlatform()
 }
 
 // Returns whether this module should have the dex file stored uncompressed in the APK.
@@ -436,23 +433,9 @@ func (a *AndroidApp) shouldUncompressDex(ctx android.ModuleContext) bool {
 }
 
 func (a *AndroidApp) shouldEmbedJnis(ctx android.BaseModuleContext) bool {
-	// Always!
-	return true
-}
-
-func (a *AndroidApp) shouldCollectRecursiveNativeDeps(ctx android.ModuleContext) bool {
-	// JNI libs are always embedded, but whether to embed their transitive dependencies as well
-	// or not is determined here. For most of the apps built here (using the platform build
-	// system), we don't need to collect the transitive deps because they will anyway be
-	// available in the partition image where the app will be installed to.
-	//
-	// Collecting transitive dependencies is required only for unbundled apps.
 	apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider)
-	apkInApex := !apexInfo.IsForPlatform()
+	return ctx.Config().UnbundledBuild() || Bool(a.appProperties.Use_embedded_native_libs) ||
-	testApp := a.appProperties.AllowCompressingNativeLibs
+		!apexInfo.IsForPlatform() || a.appProperties.AlwaysPackageNativeLibs
-	unbundledApp := ctx.Config().UnbundledBuild() || apkInApex || testApp
-
-	return a.shouldEmbedJnis(ctx) && unbundledApp
 }
 
 func generateAaptRenamePackageFlags(packageName string, renameResourcesPackage bool) []string {
@@ -846,7 +829,7 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) {
 
 	dexJarFile, packageResources := a.dexBuildActions(ctx)
 
-	jniLibs, prebuiltJniPackages, certificates := collectAppDeps(ctx, a, a.shouldCollectRecursiveNativeDeps(ctx), !Bool(a.appProperties.Jni_uses_platform_apis))
+	jniLibs, prebuiltJniPackages, certificates := collectAppDeps(ctx, a, a.shouldEmbedJnis(ctx), !Bool(a.appProperties.Jni_uses_platform_apis))
 	jniJarFile := a.jniBuildActions(jniLibs, prebuiltJniPackages, ctx)
 
 	if ctx.Failed() {
@@ -1417,7 +1400,8 @@ func AndroidTestFactory() android.Module {
 	module.Module.properties.Instrument = true
 	module.Module.properties.Supports_static_instrumentation = true
 	module.Module.properties.Installable = proptools.BoolPtr(true)
-	module.appProperties.AllowCompressingNativeLibs = true
+	module.appProperties.Use_embedded_native_libs = proptools.BoolPtr(true)
+	module.appProperties.AlwaysPackageNativeLibs = true
 	module.Module.dexpreopter.isTest = true
 	module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true)
 
@@ -1472,7 +1456,8 @@ func AndroidTestHelperAppFactory() android.Module {
 	module.Module.dexProperties.Optimize.EnabledByDefault = true
 
 	module.Module.properties.Installable = proptools.BoolPtr(true)
-	module.appProperties.AllowCompressingNativeLibs = true
+	module.appProperties.Use_embedded_native_libs = proptools.BoolPtr(true)
+	module.appProperties.AlwaysPackageNativeLibs = true
 	module.Module.dexpreopter.isTest = true
 	module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true)
 

java/app_test.go

@@ -2013,8 +2013,8 @@ func TestJNIPackaging(t *testing.T) {
 		packaged   bool
 		compressed bool
 	}{
-		{"app", true, false},
+		{"app", false, false},
-		{"app_noembed", true, false},
+		{"app_noembed", false, false},
 		{"app_embed", true, false},
 		{"test", true, false},
 		{"test_noembed", true, true},
@@ -2043,44 +2043,6 @@ func TestJNIPackaging(t *testing.T) {
 	}
 }
 
-func TestJNITranstiveDepsInstallation(t *testing.T) {
-	ctx, _ := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+`
-		android_app {
-			name: "app",
-			jni_libs: ["libjni"],
-			platform_apis: true,
-		}
-
-		cc_library {
-			name: "libjni",
-			shared_libs: ["libplatform"],
-			system_shared_libs: [],
-			stl: "none",
-			required: ["librequired"],
-		}
-
-		cc_library {
-			name: "libplatform",
-			system_shared_libs: [],
-			stl: "none",
-		}
-
-		cc_library {
-			name: "librequired",
-			system_shared_libs: [],
-			stl: "none",
-		}
-
-		`)
-
-	app := ctx.ModuleForTests("app", "android_common")
-	jniLibZip := app.Output("jnilibs.zip")
-	android.AssertPathsEndWith(t, "embedd jni lib mismatch", []string{"libjni.so"}, jniLibZip.Implicits)
-
-	install := app.Rule("Cp")
-	android.AssertPathsEndWith(t, "install dep mismatch", []string{"libplatform.so", "librequired.so"}, install.OrderOnly)
-}
-
 func TestJNISDK(t *testing.T) {
 	ctx, _ := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+`
 		cc_library {
@@ -3357,7 +3319,8 @@ func TestUsesLibraries(t *testing.T) {
 	// These also include explicit `uses_libs`/`optional_uses_libs` entries, as they may be
 	// propagated from dependencies.
 	actualManifestFixerArgs := app.Output("manifest_fixer/AndroidManifest.xml").Args["args"]
-	expectManifestFixerArgs := `--uses-library foo ` +
+	expectManifestFixerArgs := `--extract-native-libs=true ` +
+		`--uses-library foo ` +
 		`--uses-library com.non.sdk.lib ` +
 		`--uses-library qux ` +
 		`--uses-library quuz ` +
@@ -4147,7 +4110,7 @@ func TestAppIncludesJniPackages(t *testing.T) {
 		},
 		{
 			name:       "aary-no-use-embedded",
-			hasPackage: true,
+			hasPackage: false,
 		},
 	}
 

java/java.go

@@ -368,17 +368,6 @@ type dependencyTag struct {
 	static bool
 }
 
-var _ android.SkipToTransitiveDepsTag = (*dependencyTag)(nil)
-
-func (depTag dependencyTag) SkipToTransitiveDeps() bool {
-	// jni_libs are not installed because they are always embedded into the app. However,
-	// transitive deps of jni_libs themselves should be installed along with the app.
-	if IsJniDepTag(depTag) {
-		return true
-	}
-	return false
-}
-
 // installDependencyTag is a dependency tag that is annotated to cause the installed files of the
 // dependency to be installed when the parent module is installed.
 type installDependencyTag struct {

scripts/manifest_fixer.py

@@ -62,8 +62,8 @@ def parse_args():
                             'in the manifest.'))
   parser.add_argument('--extract-native-libs', dest='extract_native_libs',
                       default=None, type=lambda x: (str(x).lower() == 'true'),
-                      help=('specify if the app wants to use embedded native libraries. Must not '
+                      help=('specify if the app wants to use embedded native libraries. Must not conflict '
-                            'be true if manifest says false.'))
+                            'if already declared in the manifest.'))
   parser.add_argument('--has-no-code', dest='has_no_code', action='store_true',
                       help=('adds hasCode="false" attribute to application. Ignored if application elem '
                             'already has a hasCode attribute.'))
@@ -299,16 +299,7 @@ def add_extract_native_libs(doc, extract_native_libs):
     attr = doc.createAttributeNS(android_ns, 'android:extractNativeLibs')
     attr.value = value
     application.setAttributeNode(attr)
-  elif attr.value == "false" and value == "true":
+  elif attr.value != value:
-    # Note that we don't disallow the case of extractNativeLibs="true" in manifest and
-    # --extract-native-libs="false". This is fine because --extract-native-libs="false" means that
-    # the build system didn't compress the JNI libs, which is a fine choice for built-in apps. At
-    # runtime the JNI libs will be extracted to outside of the APK, but everything will still work
-    # okay.
-    #
-    # The opposite (extractNativeLibs="false" && --extract-native-libs="true") should however be
-    # disallowed because otherwise that would make an ill-formed APK; JNI libs are stored compressed
-    # but they won't be extracted. There's no way to execute the JNI libs.
     raise RuntimeError('existing attribute extractNativeLibs="%s" conflicts with --extract-native-libs="%s"' %
                        (attr.value, value))
 

scripts/manifest_fixer_test.py

@@ -479,8 +479,8 @@ class AddExtractNativeLibsTest(unittest.TestCase):
     self.assert_xml_equal(output, expected)
 
   def test_conflict(self):
-    manifest_input = self.manifest_tmpl % self.extract_native_libs('false')
+    manifest_input = self.manifest_tmpl % self.extract_native_libs('true')
-    self.assertRaises(RuntimeError, self.run_test, manifest_input, True)
+    self.assertRaises(RuntimeError, self.run_test, manifest_input, False)
 
 
 class AddNoCodeApplicationTest(unittest.TestCase):