Merge changes I5645ddb9,Ib3d50f15,Ib4c5815a,If3b63706 into main

* changes:
  Revert "Revert^2 "Always embed jni libs and store uncompressed""
  Revert "Revert "Revert "Collect transitve deps of jni libs only for bund...""
  Revert "Add SkipToTransitiveDepsTag interface for dependency tags"
  Revert "Install transitive deps of jni libs, but not the jni libs themselves"

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,28 +1474,16 @@ 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.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.transitive...)
-			} else {
 			packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet)
 		}
-		}
 	})
 
 	return installDeps, packagingSpecs

android/packaging_test.go

@@ -26,7 +26,6 @@ type componentTestModule struct {
 	ModuleBase
 	props struct {
 		Deps         []string
-		Build_only_deps []string
 		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):