diff --git a/android/deptag.go b/android/deptag.go index c7ba4d36f..77b9d6114 100644 --- a/android/deptag.go +++ b/android/deptag.go @@ -44,6 +44,21 @@ 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 } diff --git a/android/module.go b/android/module.go index 738f5435b..d7f0537f7 100644 --- a/android/module.go +++ b/android/module.go @@ -1470,15 +1470,27 @@ func (m *ModuleBase) computeInstallDeps(ctx ModuleContext) ([]*DepSet[InstallPat var installDeps []*DepSet[InstallPath] var packagingSpecs []*DepSet[PackagingSpec] ctx.VisitDirectDeps(func(dep Module) { - if isInstallDepNeeded(dep, ctx.OtherModuleDependencyTag(dep)) { + depTag := 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() { - installDeps = append(installDeps, dep.base().installFilesDepSet) + 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. - packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet) + if skipToTransitive { + packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet.transitive...) + } else { + packagingSpecs = append(packagingSpecs, dep.base().packagingSpecsDepSet) + } } }) diff --git a/android/packaging_test.go b/android/packaging_test.go index 383343723..4b72c24f9 100644 --- a/android/packaging_test.go +++ b/android/packaging_test.go @@ -25,8 +25,9 @@ import ( type componentTestModule struct { ModuleBase props struct { - Deps []string - Skip_install *bool + Deps []string + Build_only_deps []string + Skip_install *bool } } @@ -36,6 +37,18 @@ 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) @@ -45,6 +58,7 @@ 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) { @@ -398,3 +412,30 @@ 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"}) +} diff --git a/java/androidmk.go b/java/androidmk.go index 4f740b231..43160741b 100644 --- a/java/androidmk.go +++ b/java/androidmk.go @@ -17,7 +17,6 @@ package java import ( "fmt" "io" - "strings" "android/soong/android" @@ -413,23 +412,6 @@ 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 2978a40aa..875e06f11 100644 --- a/java/androidmk_test.go +++ b/java/androidmk_test.go @@ -19,9 +19,6 @@ import ( "testing" "android/soong/android" - "android/soong/cc" - - "github.com/google/blueprint/proptools" ) func TestRequired(t *testing.T) { @@ -255,149 +252,3 @@ 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 50d1a2f43..254fbf4fd 100644 --- a/java/app.go +++ b/java/app.go @@ -90,20 +90,17 @@ 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. 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. + // 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. 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 - // 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"` + // Allows compressing of embedded native libs. Only for android_test and android_test_helper_app. + AllowCompressingNativeLibs 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. @@ -403,14 +400,20 @@ 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 - apexInfo, _ := android.ModuleProvider(ctx, android.ApexInfoProvider) - return (minSdkVersion.FinalOrFutureInt() >= 23 && Bool(a.appProperties.Use_embedded_native_libs)) || - !apexInfo.IsForPlatform() + return useEmbedded && supported } // Returns whether this module should have the dex file stored uncompressed in the APK. @@ -433,9 +436,23 @@ 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) - return ctx.Config().UnbundledBuild() || Bool(a.appProperties.Use_embedded_native_libs) || - !apexInfo.IsForPlatform() || a.appProperties.AlwaysPackageNativeLibs + apkInApex := !apexInfo.IsForPlatform() + testApp := a.appProperties.AllowCompressingNativeLibs + unbundledApp := ctx.Config().UnbundledBuild() || apkInApex || testApp + + return a.shouldEmbedJnis(ctx) && unbundledApp } func generateAaptRenamePackageFlags(packageName string, renameResourcesPackage bool) []string { @@ -829,7 +846,7 @@ 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)) + jniLibs, prebuiltJniPackages, certificates := collectAppDeps(ctx, a, a.shouldCollectRecursiveNativeDeps(ctx), !Bool(a.appProperties.Jni_uses_platform_apis)) jniJarFile := a.jniBuildActions(jniLibs, prebuiltJniPackages, ctx) if ctx.Failed() { @@ -1400,8 +1417,7 @@ 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.Use_embedded_native_libs = proptools.BoolPtr(true) - module.appProperties.AlwaysPackageNativeLibs = true + module.appProperties.AllowCompressingNativeLibs = true module.Module.dexpreopter.isTest = true module.Module.linter.properties.Lint.Test = proptools.BoolPtr(true) @@ -1456,8 +1472,7 @@ func AndroidTestHelperAppFactory() android.Module { module.Module.dexProperties.Optimize.EnabledByDefault = true module.Module.properties.Installable = proptools.BoolPtr(true) - module.appProperties.Use_embedded_native_libs = proptools.BoolPtr(true) - module.appProperties.AlwaysPackageNativeLibs = true + module.appProperties.AllowCompressingNativeLibs = 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 a7c48a1ed..d6ba0f1dd 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", false, false}, - {"app_noembed", false, false}, + {"app", true, false}, + {"app_noembed", true, false}, {"app_embed", true, false}, {"test", true, false}, {"test_noembed", true, true}, @@ -2043,6 +2043,44 @@ 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 { @@ -3319,8 +3357,7 @@ 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 := `--extract-native-libs=true ` + - `--uses-library foo ` + + expectManifestFixerArgs := `--uses-library foo ` + `--uses-library com.non.sdk.lib ` + `--uses-library qux ` + `--uses-library quuz ` + @@ -4110,7 +4147,7 @@ func TestAppIncludesJniPackages(t *testing.T) { }, { name: "aary-no-use-embedded", - hasPackage: false, + hasPackage: true, }, } diff --git a/java/java.go b/java/java.go index 0df96a3a5..fc7e5c5a3 100644 --- a/java/java.go +++ b/java/java.go @@ -368,6 +368,17 @@ 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 { diff --git a/scripts/manifest_fixer.py b/scripts/manifest_fixer.py index 58079aa5d..35d2a1c81 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 conflict ' - 'if already declared in the manifest.')) + help=('specify if the app wants to use embedded native libraries. Must not ' + 'be true if manifest says false.')) 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,7 +299,16 @@ 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 != value: + 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. 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 0a62b10a4..9fce6b9b8 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('true') - self.assertRaises(RuntimeError, self.run_test, manifest_input, False) + manifest_input = self.manifest_tmpl % self.extract_native_libs('false') + self.assertRaises(RuntimeError, self.run_test, manifest_input, True) class AddNoCodeApplicationTest(unittest.TestCase):