From de78d138a152f6aac7e45968ffbe30c28da16fb0 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 9 Oct 2020 18:59:49 -0700 Subject: [PATCH 1/3] Replace jniDependencyTag with a value Support GetDirectDepsWithTag on JNI deps by replacing the jniDependencyTag type with a jniLibTag value. Test: app_test.go Change-Id: I8d66a5d3f433562e131a1fbafce75891d1b094dd --- java/app.go | 3 +-- java/java.go | 10 +++------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/java/app.go b/java/app.go index c25835c41..76b3598ad 100755 --- a/java/app.go +++ b/java/app.go @@ -379,7 +379,6 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { "can only be set for modules that set sdk_version") } - tag := &jniDependencyTag{} for _, jniTarget := range ctx.MultiTargets() { variation := append(jniTarget.Variations(), blueprint.Variation{Mutator: "link", Variation: "shared"}) @@ -393,7 +392,7 @@ func (a *AndroidApp) DepsMutator(ctx android.BottomUpMutatorContext) { Bool(a.appProperties.Jni_uses_sdk_apis) { variation = append(variation, blueprint.Variation{Mutator: "sdk", Variation: "sdk"}) } - ctx.AddFarVariationDependencies(variation, tag, a.appProperties.Jni_libs...) + ctx.AddFarVariationDependencies(variation, jniLibTag, a.appProperties.Jni_libs...) } a.usesLibrary.deps(ctx, sdkDep.hasFrameworkLibs()) diff --git a/java/java.go b/java/java.go index 2553a30bd..b06e604a9 100644 --- a/java/java.go +++ b/java/java.go @@ -547,13 +547,8 @@ type dependencyTag struct { name string } -type jniDependencyTag struct { - blueprint.BaseDependencyTag -} - func IsJniDepTag(depTag blueprint.DependencyTag) bool { - _, ok := depTag.(*jniDependencyTag) - return ok + return depTag == jniLibTag } var ( @@ -573,6 +568,7 @@ var ( instrumentationForTag = dependencyTag{name: "instrumentation_for"} usesLibTag = dependencyTag{name: "uses-library"} extraLintCheckTag = dependencyTag{name: "extra-lint-check"} + jniLibTag = dependencyTag{name: "jnilib"} ) func IsLibDepTag(depTag blueprint.DependencyTag) bool { @@ -1001,7 +997,7 @@ func (j *Module) collectDeps(ctx android.ModuleContext) deps { otherName := ctx.OtherModuleName(module) tag := ctx.OtherModuleDependencyTag(module) - if _, ok := tag.(*jniDependencyTag); ok { + if IsJniDepTag(tag) { // Handled by AndroidApp.collectAppDeps return } From c179ea68129972f6c5789c5fc8344d06caebcc8b Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 9 Oct 2020 10:54:15 -0700 Subject: [PATCH 2/3] Make java_binary common variant a dependency ctx.PrimaryModule() is wrong in the case of a java_binary that supports both host and device, use an explicit dependency instead. Once the dependency exists there is no need to manually request the jar be installed, it will automatically be installed by the host installation rules for dependencies. Test: TestBinary Change-Id: Iddeea2d08bc574c79d42139020558cd70d718ca1 --- android/arch.go | 19 +++++++++++++++---- java/java.go | 9 ++++----- java/java_test.go | 5 ++--- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/android/arch.go b/android/arch.go index f4b0d66d9..f505ec653 100644 --- a/android/arch.go +++ b/android/arch.go @@ -811,10 +811,16 @@ func osMutator(bpctx blueprint.BottomUpMutatorContext) { } } -// Identifies the dependency from CommonOS variant to the os specific variants. -type commonOSTag struct{ blueprint.BaseDependencyTag } +type archDepTag struct { + blueprint.BaseDependencyTag + name string +} -var commonOsToOsSpecificVariantTag = commonOSTag{} +// Identifies the dependency from CommonOS variant to the os specific variants. +var commonOsToOsSpecificVariantTag = archDepTag{name: "common os to os specific"} + +// Identifies the dependency from arch variant to the common variant for a "common_first" multilib. +var firstArchToCommonArchDepTag = archDepTag{name: "first arch to common arch"} // Get the OsType specific variants for the current CommonOS variant. // @@ -831,7 +837,6 @@ func GetOsSpecificVariantsOfCommonOSVariant(mctx BaseModuleContext) []Module { } } }) - return variants } @@ -955,6 +960,12 @@ func archMutator(bpctx blueprint.BottomUpMutatorContext) { addTargetProperties(m, targets[i], multiTargets, i == 0) m.base().setArchProperties(mctx) } + + if multilib == "common_first" && len(modules) >= 2 { + for i := range modules[1:] { + mctx.AddInterVariantDependency(firstArchToCommonArchDepTag, modules[i+1], modules[0]) + } + } } func addTargetProperties(m Module, target Target, multiTargets []Target, primaryTarget bool) { diff --git a/java/java.go b/java/java.go index b06e604a9..615cbadfb 100644 --- a/java/java.go +++ b/java/java.go @@ -2472,12 +2472,11 @@ func (j *Binary) GenerateAndroidBuildActions(ctx android.ModuleContext) { j.wrapperFile = android.PathForSource(ctx, "build/soong/scripts/jar-wrapper.sh") } - // Depend on the installed jar so that the wrapper doesn't get executed by - // another build rule before the jar has been installed. - jarFile := ctx.PrimaryModule().(*Binary).installFile - + // The host installation rules make the installed wrapper depend on all the dependencies + // of the wrapper variant, which will include the common variant's jar file. This is + // verified by TestBinary. j.binaryFile = ctx.InstallExecutable(android.PathForModuleInstall(ctx, "bin"), - ctx.ModuleName(), j.wrapperFile, jarFile) + ctx.ModuleName(), j.wrapperFile) } } diff --git a/java/java_test.go b/java/java_test.go index a43e2a8c9..a727812b5 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -467,9 +467,8 @@ func TestBinary(t *testing.T) { barWrapperDeps := barWrapper.Output("bar").Implicits.Strings() // Test that the install binary wrapper depends on the installed jar file - if len(barWrapperDeps) != 1 || barWrapperDeps[0] != barJar { - t.Errorf("expected binary wrapper implicits [%q], got %v", - barJar, barWrapperDeps) + if g, w := barWrapperDeps, barJar; !android.InList(w, g) { + t.Errorf("expected binary wrapper implicits to contain %q, got %q", w, g) } } From 89226d9ef94040215ff9d97212d997a656f5d508 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 9 Oct 2020 19:00:54 -0700 Subject: [PATCH 3/3] Add jni_libs to host java binaries Add a property to support dependencies on JNI libraries for host java binaries. Fixes: 170389375 Test: TestBinary Change-Id: Ieeca3c3997615f0b17ae1f058b94e6c9ba929cab --- java/java.go | 12 ++++++++++-- java/java_test.go | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/java/java.go b/java/java.go index 615cbadfb..3ce188586 100644 --- a/java/java.go +++ b/java/java.go @@ -2432,6 +2432,10 @@ type binaryProperties struct { // Name of the class containing main to be inserted into the manifest as Main-Class. Main_class *string + + // Names of modules containing JNI libraries that should be installed alongside the host + // variant of the binary. + Jni_libs []string } type Binary struct { @@ -2473,8 +2477,8 @@ func (j *Binary) GenerateAndroidBuildActions(ctx android.ModuleContext) { } // The host installation rules make the installed wrapper depend on all the dependencies - // of the wrapper variant, which will include the common variant's jar file. This is - // verified by TestBinary. + // of the wrapper variant, which will include the common variant's jar file and any JNI + // libraries. This is verified by TestBinary. j.binaryFile = ctx.InstallExecutable(android.PathForModuleInstall(ctx, "bin"), ctx.ModuleName(), j.wrapperFile) } @@ -2483,6 +2487,10 @@ func (j *Binary) GenerateAndroidBuildActions(ctx android.ModuleContext) { func (j *Binary) DepsMutator(ctx android.BottomUpMutatorContext) { if ctx.Arch().ArchType == android.Common { j.deps(ctx) + } else { + // This dependency ensures the host installation rules will install the jni libraries + // when the wrapper is installed. + ctx.AddVariationDependencies(nil, jniLibTag, j.binaryProperties.Jni_libs...) } } diff --git a/java/java_test.go b/java/java_test.go index a727812b5..2fd4121aa 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -456,6 +456,14 @@ func TestBinary(t *testing.T) { name: "bar", srcs: ["b.java"], static_libs: ["foo"], + jni_libs: ["libjni"], + } + + cc_library_shared { + name: "libjni", + host_supported: true, + device_supported: false, + stl: "none", } `) @@ -466,10 +474,18 @@ func TestBinary(t *testing.T) { barWrapper := ctx.ModuleForTests("bar", buildOS+"_x86_64") barWrapperDeps := barWrapper.Output("bar").Implicits.Strings() + libjni := ctx.ModuleForTests("libjni", buildOS+"_x86_64_shared") + libjniSO := libjni.Rule("Cp").Output.String() + // Test that the install binary wrapper depends on the installed jar file if g, w := barWrapperDeps, barJar; !android.InList(w, g) { t.Errorf("expected binary wrapper implicits to contain %q, got %q", w, g) } + + // Test that the install binary wrapper depends on the installed JNI libraries + if g, w := barWrapperDeps, libjniSO; !android.InList(w, g) { + t.Errorf("expected binary wrapper implicits to contain %q, got %q", w, g) + } } func TestHostBinaryNoJavaDebugInfoOverride(t *testing.T) {