From d044bb40dad01816d913a0f66d4f10ee4f6616d6 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Wed, 15 May 2024 02:09:54 +0900 Subject: [PATCH] Revert "Revert^2 "Always embed jni libs and store uncompressed"" This reverts commit 20df11ef2b7a9bd5fd1c62eee5f7dffb9d560df4. Change-Id: I5645ddb9e0d2c0873916a9192aa3cfbc967fc2cc --- java/androidmk.go | 18 ++++ java/androidmk_test.go | 149 +++++++++++++++++++++++++++++++++ java/app.go | 36 ++++---- java/app_test.go | 9 +- scripts/manifest_fixer.py | 15 +--- scripts/manifest_fixer_test.py | 4 +- 6 files changed, 195 insertions(+), 36 deletions(-) diff --git a/java/androidmk.go b/java/androidmk.go index 43160741b..4f740b231 100644 --- a/java/androidmk.go +++ b/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 { diff --git a/java/androidmk_test.go b/java/androidmk_test.go index 875e06f11..2978a40aa 100644 --- a/java/androidmk_test.go +++ b/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) + } + }) + } +} diff --git a/java/app.go b/java/app.go index 0170ea186..50d1a2f43 100644 --- a/java/app.go +++ b/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 - // APKs built using android_test or android_test_helper_app. For other APKs, this property is ignored - // and native libraries are always embedded compressed. + // flag so that they are used from inside the APK at runtime. Defaults to true for android_test modules unless + // sdk_version or min_sdk_version is set to a version that doesn't support it (<23), defaults to true for + // 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. - AllowCompressingNativeLibs bool `blueprint:"mutated"` + // Forces native libraries to always be packaged into the APK, + // 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,8 +433,9 @@ func (a *AndroidApp) shouldUncompressDex(ctx android.ModuleContext) bool { } func (a *AndroidApp) shouldEmbedJnis(ctx android.BaseModuleContext) bool { - // Always! - return true + apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider) + return ctx.Config().UnbundledBuild() || Bool(a.appProperties.Use_embedded_native_libs) || + !apexInfo.IsForPlatform() || a.appProperties.AlwaysPackageNativeLibs } func generateAaptRenamePackageFlags(packageName string, renameResourcesPackage bool) []string { @@ -1402,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) @@ -1457,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) diff --git a/java/app_test.go b/java/app_test.go index 92fe2244f..a7c48a1ed 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -2013,8 +2013,8 @@ func TestJNIPackaging(t *testing.T) { packaged bool compressed bool }{ - {"app", true, false}, - {"app_noembed", true, false}, + {"app", false, false}, + {"app_noembed", false, false}, {"app_embed", true, false}, {"test", true, false}, {"test_noembed", true, true}, @@ -3319,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 ` + @@ -4109,7 +4110,7 @@ func TestAppIncludesJniPackages(t *testing.T) { }, { name: "aary-no-use-embedded", - hasPackage: true, + hasPackage: false, }, } diff --git a/scripts/manifest_fixer.py b/scripts/manifest_fixer.py index 35d2a1c81..58079aa5d 100755 --- a/scripts/manifest_fixer.py +++ b/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 ' - 'be true if manifest says false.')) + help=('specify if the app wants to use embedded native libraries. Must not conflict ' + '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": - # 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. + elif attr.value != value: raise RuntimeError('existing attribute extractNativeLibs="%s" conflicts with --extract-native-libs="%s"' % (attr.value, value)) diff --git a/scripts/manifest_fixer_test.py b/scripts/manifest_fixer_test.py index 9fce6b9b8..0a62b10a4 100755 --- a/scripts/manifest_fixer_test.py +++ b/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') - self.assertRaises(RuntimeError, self.run_test, manifest_input, True) + manifest_input = self.manifest_tmpl % self.extract_native_libs('true') + self.assertRaises(RuntimeError, self.run_test, manifest_input, False) class AddNoCodeApplicationTest(unittest.TestCase):