From 6f907ad3ddad9eb0e928ff7ad7965fa17a8a6946 Mon Sep 17 00:00:00 2001 From: Peter Collingbourne Date: Mon, 16 Dec 2019 13:40:14 -0800 Subject: [PATCH] Also package recursive jni_libs deps of android_apps as well as direct deps. Previously, android_app targets for which a.shouldEmbedJnis(ctx) = true (e.g. CtsSelinuxTargetSdk25TestCases) would need to specify all of their recursive library dependencies, including for example libc++ when depending on the platform libc++. This means unnecessary churn when we add a new dependency to libc++ (e.g. libunwind [1]). To avoid the churn and allow jni_libs clauses to be simplified, make the build system search for the recursive dependencies and automatically include them. This change allows us to remove code that was previously adding NDK libc++ as a special case, as it is now covered by the generic code. Also fix some improper quoting that was exposed as a result of this change causing more files to be packaged than before. [1] https://android-review.googlesource.com/q/topic:%22libunwind-so%22 Bug: 144430859 Change-Id: I3d6fbcce75bc108a982eb7483992a4b202056339 --- cc/cc.go | 8 ++++---- cc/sabi.go | 2 +- java/app.go | 42 ++++++++++++++++++++++++++---------------- java/app_builder.go | 6 +++--- java/app_test.go | 38 ++++++++++++++++++++++++++++++++++++++ java/java.go | 1 - 6 files changed, 72 insertions(+), 25 deletions(-) diff --git a/cc/cc.go b/cc/cc.go index 512fe8eb4..7aa02fce5 100644 --- a/cc/cc.go +++ b/cc/cc.go @@ -759,7 +759,7 @@ func (c *Module) isNdk() bool { return inList(c.Name(), ndkMigratedLibs) } -func (c *Module) isLlndk(config android.Config) bool { +func (c *Module) IsLlndk(config android.Config) bool { // Returns true for both LLNDK (public) and LLNDK-private libs. return isLlndkLibrary(c.BaseModuleName(), config) } @@ -999,7 +999,7 @@ func (ctx *moduleContextImpl) isNdk() bool { } func (ctx *moduleContextImpl) isLlndk(config android.Config) bool { - return ctx.mod.isLlndk(config) + return ctx.mod.IsLlndk(config) } func (ctx *moduleContextImpl) isLlndkPublic(config android.Config) bool { @@ -1880,7 +1880,7 @@ func checkDoubleLoadableLibraries(ctx android.TopDownMutatorContext) { return true } - if to.isVndkSp() || to.isLlndk(ctx.Config()) || Bool(to.VendorProperties.Double_loadable) { + if to.isVndkSp() || to.IsLlndk(ctx.Config()) || Bool(to.VendorProperties.Double_loadable) { return false } @@ -1895,7 +1895,7 @@ func checkDoubleLoadableLibraries(ctx android.TopDownMutatorContext) { } if module, ok := ctx.Module().(*Module); ok { if lib, ok := module.linker.(*libraryDecorator); ok && lib.shared() { - if module.isLlndk(ctx.Config()) || Bool(module.VendorProperties.Double_loadable) { + if module.IsLlndk(ctx.Config()) || Bool(module.VendorProperties.Double_loadable) { ctx.WalkDeps(check) } } diff --git a/cc/sabi.go b/cc/sabi.go index 8cef1700c..7c9c651b9 100644 --- a/cc/sabi.go +++ b/cc/sabi.go @@ -80,7 +80,7 @@ func (sabimod *sabi) flags(ctx ModuleContext, flags Flags) Flags { func sabiDepsMutator(mctx android.TopDownMutatorContext) { if c, ok := mctx.Module().(*Module); ok && - ((c.IsVndk() && c.UseVndk()) || c.isLlndk(mctx.Config()) || + ((c.IsVndk() && c.UseVndk()) || c.IsLlndk(mctx.Config()) || (c.sabi != nil && c.sabi.Properties.CreateSAbiDumps)) { mctx.VisitDirectDeps(func(m android.Module) { tag := mctx.OtherModuleDependencyTag(m) diff --git a/java/app.go b/java/app.go index 7595e36a6..aa9c78c0f 100755 --- a/java/app.go +++ b/java/app.go @@ -167,18 +167,11 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { a.aapt.deps(ctx, sdkDep) } + tag := &jniDependencyTag{} for _, jniTarget := range ctx.MultiTargets() { variation := append(jniTarget.Variations(), blueprint.Variation{Mutator: "link", Variation: "shared"}) - tag := &jniDependencyTag{ - target: jniTarget, - } ctx.AddFarVariationDependencies(variation, tag, a.appProperties.Jni_libs...) - if String(a.appProperties.Stl) == "c++_shared" { - if a.shouldEmbedJnis(ctx) { - ctx.AddFarVariationDependencies(variation, tag, "ndk_libc++_shared") - } - } } a.usesLibrary.deps(ctx, sdkDep.hasFrameworkLibs()) @@ -471,7 +464,7 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { dexJarFile := a.dexBuildActions(ctx) - jniLibs, certificateDeps := collectAppDeps(ctx) + jniLibs, certificateDeps := collectAppDeps(ctx, a.shouldEmbedJnis(ctx)) jniJarFile := a.jniBuildActions(jniLibs, ctx) if ctx.Failed() { @@ -507,22 +500,33 @@ func (a *AndroidApp) generateAndroidBuildActions(ctx android.ModuleContext) { } } -func collectAppDeps(ctx android.ModuleContext) ([]jniLib, []Certificate) { +func collectAppDeps(ctx android.ModuleContext, shouldCollectRecursiveNativeDeps bool) ([]jniLib, []Certificate) { var jniLibs []jniLib var certificates []Certificate + seenModulePaths := make(map[string]bool) - ctx.VisitDirectDeps(func(module android.Module) { + ctx.WalkDeps(func(module android.Module, parent android.Module) bool { otherName := ctx.OtherModuleName(module) tag := ctx.OtherModuleDependencyTag(module) - if jniTag, ok := tag.(*jniDependencyTag); ok { + if IsJniDepTag(tag) || tag == cc.SharedDepTag { if dep, ok := module.(*cc.Module); ok { + if dep.IsLlndk(ctx.Config()) || dep.IsStubs() { + return false + } + lib := dep.OutputFile() + path := lib.Path() + if seenModulePaths[path.String()] { + return false + } + seenModulePaths[path.String()] = true + if lib.Valid() { jniLibs = append(jniLibs, jniLib{ name: ctx.OtherModuleName(module), - path: lib.Path(), - target: jniTag.target, + path: path, + target: module.Target(), }) } else { ctx.ModuleErrorf("dependency %q missing output file", otherName) @@ -530,13 +534,19 @@ func collectAppDeps(ctx android.ModuleContext) ([]jniLib, []Certificate) { } else { ctx.ModuleErrorf("jni_libs dependency %q must be a cc library", otherName) } - } else if tag == certificateTag { + + return shouldCollectRecursiveNativeDeps + } + + if tag == certificateTag { if dep, ok := module.(*AndroidAppCertificate); ok { certificates = append(certificates, dep.Certificate) } else { ctx.ModuleErrorf("certificate dependency %q must be an android_app_certificate module", otherName) } } + + return false }) return jniLibs, certificates @@ -968,7 +978,7 @@ func (a *AndroidAppImport) generateAndroidBuildActions(ctx android.ModuleContext ctx.ModuleErrorf("One and only one of certficate, presigned, and default_dev_cert properties must be set") } - _, certificates := collectAppDeps(ctx) + _, certificates := collectAppDeps(ctx, false) // TODO: LOCAL_EXTRACT_APK/LOCAL_EXTRACT_DPI_APK // TODO: LOCAL_PACKAGE_SPLITS diff --git a/java/app_builder.go b/java/app_builder.go index ec2f6dafe..5e7fbe6de 100644 --- a/java/app_builder.go +++ b/java/app_builder.go @@ -200,14 +200,14 @@ func TransformJniLibsToJar(ctx android.ModuleContext, outputFile android.Writabl } if uncompressJNI { - jarArgs = append(jarArgs, "-L 0") + jarArgs = append(jarArgs, "-L", "0") } for _, j := range jniLibs { deps = append(deps, j.path) jarArgs = append(jarArgs, - "-P "+targetToJniDir(j.target), - "-f "+j.path.String()) + "-P", targetToJniDir(j.target), + "-f", j.path.String()) } ctx.Build(pctx, android.BuildParams{ diff --git a/java/app_test.go b/java/app_test.go index 1800bb73b..5cf0ba78d 100644 --- a/java/app_test.go +++ b/java/app_test.go @@ -1630,8 +1630,46 @@ func TestAndroidTestImport(t *testing.T) { func TestStl(t *testing.T) { ctx, _ := testJava(t, cc.GatherRequiredDepsForTest(android.Android)+` + cc_library { + name: "ndk_libunwind", + sdk_version: "current", + stl: "none", + system_shared_libs: [], + } + + cc_library { + name: "libc.ndk.current", + sdk_version: "current", + stl: "none", + system_shared_libs: [], + } + + cc_library { + name: "libm.ndk.current", + sdk_version: "current", + stl: "none", + system_shared_libs: [], + } + + cc_library { + name: "libdl.ndk.current", + sdk_version: "current", + stl: "none", + system_shared_libs: [], + } + + cc_object { + name: "ndk_crtbegin_so.27", + } + + cc_object { + name: "ndk_crtend_so.27", + } + cc_library { name: "libjni", + sdk_version: "current", + stl: "c++_shared", } android_test { diff --git a/java/java.go b/java/java.go index d8db5f8a4..cdae282bb 100644 --- a/java/java.go +++ b/java/java.go @@ -466,7 +466,6 @@ type dependencyTag struct { type jniDependencyTag struct { blueprint.BaseDependencyTag - target android.Target } func IsJniDepTag(depTag blueprint.DependencyTag) bool {