From 25b9222a43a599cccbce856bb72f4190059a30c8 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 17 May 2024 22:58:54 +0000 Subject: [PATCH 1/4] Revert^2 "Install jni symlinks in Soong" b7646e4d4ff20002c658ab667b4c1892bc3f0f1d This is a relanding of I0930cb1ebb8ca8a6efd64b1ce2cdfd1c47fe19ef plus some forward fix described below: Export non-embedded JNI lib names via LOCAL_REQUIRED_MODULES The non-embedded JNI libs are installed as the dependencies of the APK. However, that dependency is not revealed to the Make world and as a result, the JNI libs are dropped from the file_list.txt file which Make uses to filter files to include in the image file. Adding the lib names to LOCAL_REQUIRED_MODULES fixes it. Bug: 341335305 Bug: 330276359 Test: m out/target/product/vsoc_x86_64_only/obj/PACKAGING/system_intermediates/file_list.txt and check if libcarservicejni.so is there Test: go test ./... under soong/java Change-Id: If915a05909129c92fab7a6cbbd0c4c55f5ced598 --- java/androidmk.go | 18 +---- java/androidmk_test.go | 176 +++++++++-------------------------------- java/app.go | 21 +++++ java/java.go | 1 + 4 files changed, 63 insertions(+), 153 deletions(-) diff --git a/java/androidmk.go b/java/androidmk.go index 4f740b231..a1bc90494 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -17,7 +17,6 @@ package java import ( "fmt" "io" - "strings" "android/soong/android" @@ -414,22 +413,11 @@ func (app *AndroidApp) AndroidMkEntries() []android.AndroidMkEntries { jniSymbols := app.JNISymbolsInstalls(app.installPathForJNISymbols.String()) entries.SetString("LOCAL_SOONG_JNI_LIBS_SYMBOLS", jniSymbols.String()) } else { + var names []string 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) + names = append(names, jniLib.name) } + entries.AddStrings("LOCAL_REQUIRED_MODULES", names...) } if len(app.jniCoverageOutputs) > 0 { diff --git a/java/androidmk_test.go b/java/androidmk_test.go index 2978a40aa..243a2791e 100644 --- a/java/androidmk_test.go +++ b/java/androidmk_test.go @@ -20,8 +20,6 @@ import ( "android/soong/android" "android/soong/cc" - - "github.com/google/blueprint/proptools" ) func TestRequired(t *testing.T) { @@ -256,148 +254,50 @@ func TestGetOverriddenPackages(t *testing.T) { } } -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" +func TestJniAsRequiredDeps(t *testing.T) { 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 + ).RunTestWithBp(t, ` + android_app { + name: "app", + jni_libs: ["libjni"], + platform_apis: true, + } + + android_app { + name: "app_embedded", + jni_libs: ["libjni"], + platform_apis: true, + use_embedded_native_libs: true, + } + + cc_library { + name: "libjni", + system_shared_libs: [], + stl: "none", + } + `) + + testcases := []struct { + name string + expected []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"}}, + { + name: "app", + expected: []string{"libjni"}, + }, + { + name: "app_embedded", + expected: nil, + }, } - 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) - } - }) + for _, tc := range testcases { + mod := ctx.ModuleForTests(tc.name, "android_common").Module() + entries := android.AndroidMkEntriesForTest(t, ctx.TestContext, mod)[0] + required := entries.EntryMap["LOCAL_REQUIRED_MODULES"] + android.AssertDeepEquals(t, "unexpected required deps", tc.expected, required) } } diff --git a/java/app.go b/java/app.go index f05b8a7cd..22958e5c8 100644 --- a/java/app.go +++ b/java/app.go @@ -911,6 +911,26 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { installed := ctx.InstallFile(a.installDir, extra.Base(), extra) extraInstalledPaths = append(extraInstalledPaths, installed) } + // If we don't embed jni libs, make sure that those are installed along with the + // app, and also place symlinks to the installed paths under the lib/ + // directory of the app installation directory. ex: + // /system/app/MyApp/lib/arm64/libfoo.so -> /system/lib64/libfoo.so + if !a.embeddedJniLibs { + for _, jniLib := range jniLibs { + archStr := jniLib.target.Arch.ArchType.String() + symlinkDir := a.installDir.Join(ctx, "lib", archStr) + for _, installedLib := range jniLib.installPaths { + // install the symlink target along with the app + extraInstalledPaths = append(extraInstalledPaths, installedLib) + ctx.PackageFile(installedLib, "", jniLib.path) + + // install the symlink itself + symlinkName := installedLib.Base() + symlinkTarget := android.InstallPathToOnDevicePath(ctx, installedLib) + ctx.InstallAbsoluteSymlink(symlinkDir, symlinkName, symlinkTarget) + } + } + } ctx.InstallFile(a.installDir, a.outputFile.Base(), a.outputFile, extraInstalledPaths...) } @@ -998,6 +1018,7 @@ func collectJniDeps(ctx android.ModuleContext, coverageFile: dep.CoverageOutputFile(), unstrippedFile: dep.UnstrippedOutputFile(), partition: dep.Partition(), + installPaths: dep.FilesToInstall(), }) } else if ctx.Config().AllowMissingDependencies() { ctx.AddMissingDependencies([]string{otherName}) diff --git a/java/java.go b/java/java.go index 0df96a3a5..249480428 100644 --- a/java/java.go +++ b/java/java.go @@ -491,6 +491,7 @@ type jniLib struct { coverageFile android.OptionalPath unstrippedFile android.Path partition string + installPaths android.InstallPaths } func sdkDeps(ctx android.BottomUpMutatorContext, sdkContext android.SdkContext, d dexer) { From 970c524ab5adef198382c833f873e8b573e7003f Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 17 May 2024 22:58:54 +0000 Subject: [PATCH 2/4] Revert "Revert "APK-in-APEX should set use_embedded_native_libs:..." Revert submission 3094658-revert-3088867-QEHDREAABY Reason for revert: not a regression Reverted changes: /q/submissionid:3094658-revert-3088867-QEHDREAABY Change-Id: Ie1e4a196f181643a3e64e573b132d52d7455461b --- apex/apex_test.go | 1 + java/app.go | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/apex/apex_test.go b/apex/apex_test.go index b14600e80..f2c0ab6ea 100644 --- a/apex/apex_test.go +++ b/apex/apex_test.go @@ -5922,6 +5922,7 @@ func TestApexWithApps(t *testing.T) { srcs: ["foo/bar/MyClass.java"], sdk_version: "current", system_modules: "none", + use_embedded_native_libs: true, jni_libs: ["libjni"], stl: "none", apex_available: [ "myapex" ], diff --git a/java/app.go b/java/app.go index 22958e5c8..441091b17 100644 --- a/java/app.go +++ b/java/app.go @@ -334,6 +334,7 @@ func (a *AndroidTestHelperApp) GenerateAndroidBuildActions(ctx android.ModuleCon func (a *AndroidApp) GenerateAndroidBuildActions(ctx android.ModuleContext) { a.checkAppSdkVersions(ctx) + a.checkEmbedJnis(ctx) a.generateAndroidBuildActions(ctx) a.generateJavaUsedByApex(ctx) } @@ -378,6 +379,17 @@ func (a *AndroidApp) checkAppSdkVersions(ctx android.ModuleContext) { a.checkSdkVersions(ctx) } +// Ensures that use_embedded_native_libs are set for apk-in-apex +func (a *AndroidApp) checkEmbedJnis(ctx android.BaseModuleContext) { + apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider) + apkInApex := !apexInfo.IsForPlatform() + hasJnis := len(a.appProperties.Jni_libs) > 0 + + if apkInApex && hasJnis && !Bool(a.appProperties.Use_embedded_native_libs) { + ctx.ModuleErrorf("APK in APEX should have use_embedded_native_libs: true") + } +} + // If an updatable APK sets min_sdk_version, min_sdk_vesion of JNI libs should match with it. // This check is enforced for "updatable" APKs (including APK-in-APEX). func (a *AndroidApp) checkJniLibsSdkVersion(ctx android.ModuleContext, minSdkVersion android.ApiLevel) { @@ -433,9 +445,9 @@ func (a *AndroidApp) shouldUncompressDex(ctx android.ModuleContext) bool { } func (a *AndroidApp) shouldEmbedJnis(ctx android.BaseModuleContext) bool { - apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider) return ctx.Config().UnbundledBuild() || Bool(a.appProperties.Use_embedded_native_libs) || - !apexInfo.IsForPlatform() || a.appProperties.AlwaysPackageNativeLibs + Bool(a.appProperties.Updatable) || + a.appProperties.AlwaysPackageNativeLibs } func generateAaptRenamePackageFlags(packageName string, renameResourcesPackage bool) []string { From c1e5b1848a3c3c2434093e6ddc0e7efd40b1b3a1 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 17 May 2024 22:58:54 +0000 Subject: [PATCH 3/4] Revert^2 "Add make java.dependencyTag installable or not" f14b5ba86388b97a482ab02d7c18a07f22e51c5d Change-Id: I0638d4997cb8e5178f869ea0f469a1a5c141611e --- java/java.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/java/java.go b/java/java.go index 249480428..05ef5d04f 100644 --- a/java/java.go +++ b/java/java.go @@ -366,14 +366,14 @@ type dependencyTag struct { toolchain bool static bool + + installable bool } -// 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 { - blueprint.BaseDependencyTag - android.InstallAlwaysNeededDependencyTag - name string +var _ android.InstallNeededDependencyTag = (*dependencyTag)(nil) + +func (d dependencyTag) InstallDepNeeded() bool { + return d.installable } func (d dependencyTag) LicenseAnnotations() []android.LicenseAnnotation { @@ -405,7 +405,7 @@ func makeUsesLibraryDependencyTag(sdkVersion int, optional bool) usesLibraryDepe } func IsJniDepTag(depTag blueprint.DependencyTag) bool { - return depTag == jniLibTag + return depTag == jniLibTag || depTag == jniInstallTag } var ( @@ -434,8 +434,8 @@ var ( javaApiContributionTag = dependencyTag{name: "java-api-contribution"} depApiSrcsTag = dependencyTag{name: "dep-api-srcs"} aconfigDeclarationTag = dependencyTag{name: "aconfig-declaration"} - jniInstallTag = installDependencyTag{name: "jni install"} - binaryInstallTag = installDependencyTag{name: "binary install"} + jniInstallTag = dependencyTag{name: "jni install", runtimeLinked: true, installable: true} + binaryInstallTag = dependencyTag{name: "binary install", runtimeLinked: true, installable: true} usesLibReqTag = makeUsesLibraryDependencyTag(dexpreopt.AnySdkVersion, false) usesLibOptTag = makeUsesLibraryDependencyTag(dexpreopt.AnySdkVersion, true) usesLibCompat28OptTag = makeUsesLibraryDependencyTag(28, true) From 92d8404bc5d91f6567c5c0a2c4c01497736f175a Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 17 May 2024 22:58:54 +0000 Subject: [PATCH 4/4] Revert^2 "Refactor how jni_libs dependencies are added" 562d9054e52fb776ff6dde92fc280acc344e6f31 Change-Id: Ib9e6efc736bd0ba20637afb0a8d05b9b0266586f --- java/app.go | 45 ++++++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/java/app.go b/java/app.go index 441091b17..377851e39 100644 --- a/java/app.go +++ b/java/app.go @@ -274,16 +274,37 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { variation := append(jniTarget.Variations(), blueprint.Variation{Mutator: "link", Variation: "shared"}) - // If the app builds against an Android SDK use the SDK variant of JNI dependencies - // unless jni_uses_platform_apis is set. - // Don't require the SDK variant for apps that are shipped on vendor, etc., as they already - // have stable APIs through the VNDK. - if (usesSDK && !a.RequiresStableAPIs(ctx) && - !Bool(a.appProperties.Jni_uses_platform_apis)) || - Bool(a.appProperties.Jni_uses_sdk_apis) { + // Test whether to use the SDK variant or the non-SDK variant of JNI dependencies. + // Many factors are considered here. + // 1. Basically, the selection follows whether the app has sdk_version set or not. + jniUsesSdkVariant := usesSDK + // 2. However, jni_uses_platform_apis and jni_uses_sdk_apis can override it + if Bool(a.appProperties.Jni_uses_sdk_apis) { + jniUsesSdkVariant = true + } + if Bool(a.appProperties.Jni_uses_platform_apis) { + jniUsesSdkVariant = false + } + // 3. Then the use of SDK variant is again prohibited for the following cases: + // 3.1. the app is shipped on unbundled partitions like vendor. Since the entire + // partition (not only the app) is considered unbudled, there's no need to use the + // SDK variant. + // 3.2. the app doesn't support embedding the JNI libs + if a.RequiresStableAPIs(ctx) || !a.shouldEmbedJnis(ctx) { + jniUsesSdkVariant = false + } + if jniUsesSdkVariant { variation = append(variation, blueprint.Variation{Mutator: "sdk", Variation: "sdk"}) } - ctx.AddFarVariationDependencies(variation, jniLibTag, a.appProperties.Jni_libs...) + + // Use the installable dep tag when the JNIs are not embedded + var tag dependencyTag + if a.shouldEmbedJnis(ctx) { + tag = jniLibTag + } else { + tag = jniInstallTag + } + ctx.AddFarVariationDependencies(variation, tag, a.appProperties.Jni_libs...) } for _, aconfig_declaration := range a.aaptProperties.Flags_packages { ctx.AddDependency(ctx.Module(), aconfigDeclarationTag, aconfig_declaration) @@ -841,7 +862,9 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { dexJarFile, packageResources := a.dexBuildActions(ctx) - jniLibs, prebuiltJniPackages, certificates := collectAppDeps(ctx, a, a.shouldEmbedJnis(ctx), !Bool(a.appProperties.Jni_uses_platform_apis)) + // No need to check the SDK version of the JNI deps unless we embed them + checkNativeSdkVersion := a.shouldEmbedJnis(ctx) && !Bool(a.appProperties.Jni_uses_platform_apis) + jniLibs, prebuiltJniPackages, certificates := collectAppDeps(ctx, a, a.shouldEmbedJnis(ctx), checkNativeSdkVersion) jniJarFile := a.jniBuildActions(jniLibs, prebuiltJniPackages, ctx) if ctx.Failed() { @@ -932,10 +955,6 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { archStr := jniLib.target.Arch.ArchType.String() symlinkDir := a.installDir.Join(ctx, "lib", archStr) for _, installedLib := range jniLib.installPaths { - // install the symlink target along with the app - extraInstalledPaths = append(extraInstalledPaths, installedLib) - ctx.PackageFile(installedLib, "", jniLib.path) - // install the symlink itself symlinkName := installedLib.Base() symlinkTarget := android.InstallPathToOnDevicePath(ctx, installedLib)