Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android target for a _transitive closure_ library aar #348

Open
mrdomino opened this issue Jul 29, 2015 · 30 comments
Open

Android target for a _transitive closure_ library aar #348

mrdomino opened this issue Jul 29, 2015 · 30 comments
Assignees
Labels
not stale Issues or PRs that are inactive but not considered stale P2 We'll consider working on this in future. (Assignee optional) team-Android Issues for Android team type: feature request

Comments

@mrdomino
Copy link
Contributor

One of my work projects is an Android library with a substantial JNI footprint, designed for inclusion in third-party Android apps. I'd love to be able to specify it as a single bazel target (say for an aar, which seems to be what the Android people want us to generate these days) that depends on my android_library and cc_library.

My current planned stopgap is to build an android_binary and then take the .jar and .so out of the target for redistribution. Improvements and suggestions welcome.

@damienmg damienmg added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Aug 4, 2015
@aj-michael
Copy link
Contributor

android_library has an implicit AAR output called name.aar.

As an example,

$ cat java/com/foo/BUILD
cc_library(
    name = "cc_lib",
    srcs = ["foo.cc"],
)
android_library(
    name = "android_lib",
    srcs = ["Foo.java"],
    deps = [":cc_lib"],
)
$ bazel build //java/com/foo:android_lib.aar
INFO: Found 1 target...
Target //java/com/foo:android_lib.aar up-to-date:
  bazel-bin/java/com/foo/android_lib.aar
INFO: Elapsed time: 4.138s, Critical Path: 3.59s
$

@aj-michael aj-michael reopened this Mar 17, 2017
@aj-michael
Copy link
Contributor

Augh, I should not have closed this. Per the documentation of android_library:

name.aar: An android 'aar' bundle containing the java archive and resources of this target. It does not contain the transitive closure.

https://bazel.build/versions/master/docs/be/android.html#android_library

So the .aar output of android_library will not contain the .so from the cc_library.

I'm going to use this as the tracking bug for support transitive-closure-aars from android_library.

@aj-michael aj-michael added P2 We'll consider working on this in future. (Assignee optional) and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Mar 17, 2017
@aj-michael aj-michael changed the title Android target for a library aar Android target for a _transitive closure_ library aar Mar 17, 2017
@aj-michael aj-michael added this to the 0.6 milestone Mar 17, 2017
@andrewharp
Copy link

andrewharp commented Mar 17, 2017

This would also be useful for TensorFlow so that we can build a full Android distributable in a single file (using only Bazel) during our nightly build process.

edit: We'd like to be able to package all relevant architectures in the same AAR, much like --fat_apk_cpu packages multiple target architectures in APKs.

@spinorx
Copy link

spinorx commented Oct 18, 2017

+1. Any update of expected support for this?

@ahumesky
Copy link
Contributor

We're working on getting our testing for android apps in better shape, this is something we'll likely work on after that.

@steeve
Copy link
Contributor

steeve commented Jan 9, 2018

hey guys, i'm having another issue: the cc_library on which my android_library depends is not built when building via name.aar.
And when I build via name, it seems it is not built for the android ndk (unable to include jni.h, for instance)

@balazsbanyai
Copy link

+1

@spinorx
Copy link

spinorx commented Jan 9, 2018

Use sample in https://github.com/spinorx/grpc_andr_ios to unblock urself till this is fixed.

@balazsbanyai
Copy link

balazsbanyai commented Jan 10, 2018

@spinorx thank you very much. I can build the app using the provided project, but the android_library target (eg. bazel build --config android //android:GreeterClient) won't build in this example either.

@spinorx
Copy link

spinorx commented Jan 10, 2018

Yeah, android_library does not propogate needed config to cc_library yet. See detailed discussion in: #3924

@aj-michael
Copy link
Contributor

@steeve, see my response in #4407 (comment) if you want to build the cc_library for Android directly. Note that that will not get you a .so. To build a .so outside of android_binary, you need a cc_binary.

@steeve
Copy link
Contributor

steeve commented Jan 10, 2018

@aj-michael thanks! however, I'm trying to build an aar (not an apk) file that depends on the cc_library.
I would assume the toolchain configuration to be transitive to the cc_library with android_library the way android_binary does.

Also, there is the case of fat apks, which is better handled by building for the Android env via android_{binary,library} rules imho.

@aj-michael
Copy link
Contributor

Yes, I was responding to your comment:

And when I build via name, it seems it is not built for the android ndk (unable to include jni.h, for instance)

android_library .aar output does not currently support bundling native libraries. This is a known deficiency that I believe @dkelmer has plans to work on.

If you're so inclined, you could workaround this deficiency by creating an additional android_binary and a genrule for the android_library, something like this:

genrule(
    name = "aar_with_jni",
    srcs = [
        ":abin_unsigned.apk",
        ":alib.aar",
    ],
    outs = ["alib_with_jni.aar"],
    cmd = """
cp $(location :alib.aar) $(location :alib_with_jni.aar)
chmod +w $(location :alib_with_jni.aar)
origdir=$$PWD
cd $$(mktemp -d)
unzip $$origdir/$(location :abin_unsigned.apk) "lib/*"
cp -r lib jni
zip -r $$origdir/$(location :alib_with_jni.aar) jni/*/*.so
""",
)

android_binary(
    name = "abin",
    manifest = "AndroidManifest.xml",
    custom_package = "does.not.matter",
    deps = [":alib"],
)

android_library(
    name = "alib",
    srcs = ["A.java"],
    deps = [":clib"],
)

cc_library(
    name = "clib",
    srcs = ["c.cc"],
)

@steeve
Copy link
Contributor

steeve commented Jan 10, 2018

nice, I hadn't though about this! thanks @aj-michael

@aj-michael
Copy link
Contributor

This is a common enough request (JNI in .aar) that I decided to throw together an example repository for how to workaround it:

https://github.com/aj-michael/aar_with_jni

@andrewharp
Copy link

andrewharp commented Jan 10, 2018

FWIW you can just use zip -ur to add files to an existing archive (we do this to create the TensorFlow AAR for release), no need to unzip first.

@jcayzac
Copy link

jcayzac commented Jan 15, 2018

@aj-michael adding to this discussion, in my understanding the aar produced by bazel are also "incomplete" in the sense there is no (obvious) way to specify a consumer proguard file yet.

Should I file a separate issue for this, or better keep it here together with the related jni problem?

@aj-michael
Copy link
Contributor

@jcayzac , please file a separate issue. If I understand what you mean, that is probably a much easier problem to solve. The JNI issue is a little tricky because at the moment android_library doesn't do any linking.

@jin jin added team-Android Issues for Android team and removed category: rules > android labels Aug 11, 2018
@jin jin removed this from the 0.6 milestone Aug 14, 2018
@cpsauer
Copy link
Contributor

cpsauer commented Dec 18, 2020

Ran across this while looking for a workaround for our team. Can confirm that there are still people who'd use this :) (looks like Tensorflow/MediaPipe are also using AJ's good workaround.)

Any updates on official support?

@steeve
Copy link
Contributor

steeve commented Dec 18, 2020

Once starlark rules_android lands (they are in alpha iirc), it'll be much easier to add it

@cpsauer
Copy link
Contributor

cpsauer commented Mar 13, 2021

After some more time hacking in and around the issue--and thinking about what we'd want in an eventual distributable AAR rule:

I think a key feature will be to not include the transitive dependencies downloaded from Maven. And to be able to push the generated AAR up to Maven, listing pointers to those Maven-based dependencies.

Unlike an APK where you want all the dependencies inside, with an AAR, you're (usually) trying to add a new node to the dependency graph of Maven dependencies. But you may very well want to structure your code inside that AAR into multiple bazel rules; it's your internal structure that you want bundled up transitively.

If anyone's got a good solution for this, I'd love to learn from you, if you're willing. We've just run into this, and it's a bit tricky to hack around at the moment, since Maven AAR imports seem to be either "always bundle", or "neverlink", when you want "link in a true APK, don't link in the AAR we're hacking out of an APK".

@cpsauer
Copy link
Contributor

cpsauer commented Mar 13, 2021

Also, another learning that might be useful to others:

Heads for others reading this issue that the current workaround it doesn't copy transitive java dependencies into the aar. I'm working on a version that does....but in the meantime Google libraries (like Tensorflow, etc.) are working around this workaround by filegrouping all the Java files into one android_library rather than have a Java build graph.

@cpsauer
Copy link
Contributor

cpsauer commented Mar 17, 2021

In case it's useful for thinking about the interface: looks like FB's buck has this pretty well handled: https://buck.build/rule/android_aar.html

@cpsauer
Copy link
Contributor

cpsauer commented Jun 9, 2021

@ahumesky, is this functionality you think will be rolled into your rules_android work in the near term? Wanted to check in before I do a bunch of hand-rolling here.

@ahumesky
Copy link
Contributor

Unfortunately no, this is unlikely to be part of the Starlark android rules in the near term. We have mechanisms for this internally, but we're not in a position to open source them right now, and they may not even work externally. At some point it may make sense to integrate community contributions for this.

@cpsauer
Copy link
Contributor

cpsauer commented Jun 11, 2021

Bummer, but great to know. Thank you, Alex.

(I'm sure you guys already know all about why it's wanted, given, e.g. Tensorflow's need, so I won't say more on that.)

If you want what I end up building, lmk.

bazel-io pushed a commit that referenced this issue Aug 12, 2021
  - 3c7ec07fe0418446ffdb1a04c671a3810d74ae30 [cas] Add cas package (#300) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 28fa42989a6c2d05cddb6b42494f34ef742c3de9 [cas] Implement file reading (#302) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 69c6642c3a006636256c1d2e591899f07d4c74bf Simplify caching packages (#303) by nodirg <56001730+nodirg@users.noreply.github.com>
  - f7087af662fe5481f2e75abd652af46d9a376534 [cas] Implement presence check (#304) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 847bca232f884b1e7ee059c64317a5140f11477e Simplify caching packages further (#306) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 989513ef4a567a812f21b27f2a5d83f1a2145600 Rename singleflightcache.Cache (#310) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 7eceb37537dec7c50e52d4f1af1a5d671c267144 [cas] Fix the build (#308) by nodirg <56001730+nodirg@users.noreply.github.com>
  - fd877b05ba2ed611f6d34e711da05917b729eff9 [cas] Add retries (#311) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 5bc303584ef03ded33f03fa2976015e38da9c050 [cas] Add support for Symlinks. (#309) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 4cfed65947cba54af1b2259d9e3facf7dd3007b9 [cas] Implement batch upload (#307) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 144126c43e73aeeabdbeb8bc4ef0290fa7a91ba7 [cas] Move file IO semaphore to Client (#313) by nodirg <56001730+nodirg@users.noreply.github.com>
  - e1da041171a715b7bab3178acfe1cd774b9f5019 [cas] Reuse file read buffers (#317) by nodirg <56001730+nodirg@users.noreply.github.com>
  - e9184e44947661852a8887f3183e00c346208aaa [cas] Add UploadOptions (#314) by nodirg <56001730+nodirg@users.noreply.github.com>
  - e2bd6c8e2d6bc183ff059c688a604ba1ac18b840 [cas] Implement ServerCapabilities check (#315) by nodirg <56001730+nodirg@users.noreply.github.com>
  - b00d91e726265f23ad0a88730545831b8d5519ef [cas] Add RPCConfig (#318) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 1c678dec65b62e49840419ab777c7b6ce65cfd76 [cas] Improve error messages (#321) by nodirg <56001730+nodirg@users.noreply.github.com>
  - ad8d2cfffe1f3728469a8dd5a7531b9157280ecd [cas] Limit FindMissingBlobs concurrency (#319) by nodirg <56001730+nodirg@users.noreply.github.com>
  - b1b54ee4d55b5d5bd71ca2df4353058515581697 [cas] Increase FindMissingBlobs concurrency to 256 (#323) by nodirg <56001730+nodirg@users.noreply.github.com>
  - b4a0e12d87c946ced360d723d743be1f57a47995 [cas] Move file IO buffering deeper (#322) by nodirg <56001730+nodirg@users.noreply.github.com>
  - abb14633e09633368f06d8e4fd28bd1553509061 [cas] Add filtering/callback (#316) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 7447b28dd69e22848ee850936a1cdd28e2d0e20b Add Mtime to the file metadata cache. (#326) by ramymedhat <abdelaal@google.com>
  - 7182b476eb6260fae9ba2bd8995b97a7096e340c [cas] Implement streaming (#320) by nodirg <56001730+nodirg@users.noreply.github.com>
  - b0605647bbe2ff7d046a286c3023e7714376fb83 include file path in upload error (#327) by Takuto Ikuta <tikuta@google.com>
  - 3b602dd48f7f63a76cb1087a10355b3c668d583f [cas] Implement per-request timeouts in a stream (#325) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 395c674af7a9cd696dfd1f2b4a950f6899ccb3a0 remove unused variables (#332) by Takuto Ikuta <tikuta@google.com>
  - 45f49a9529f755fb586fe2f8bf3e78b1eae39e81 [cas] Unembed cas.Client.ClientConfig. (#333) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 2a9b29928abe867026e37833fa480ed16238df7a [cas] Rename UploadOptions.Callback to Prelude (#334) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 80ea864b211ee3d87e14f4be0ca8d17e5917062c [cas] Read files once (#335) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 0e577525a2dce2d0e7bddd80927c8901d28bf0fb [cas] PathExclude: use forward-slash-separated paths (#341) by nodirg <56001730+nodirg@users.noreply.github.com>
  - e155d015bcc4c9eb9978572422e3404f26220700 Add useful error message for uploading files. (#339) by bansalvinayak <vinayakbansal@google.com>
  - 3dfb518d390280a2ffef5a85ab1852a2a526a983 [cas] Rename UploadInput to PathSpec (#342) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 752e4efb2631b45f5c27120f83d517cc2b2846d2 google/uuid -> pborman/uuid (#344) by Rubens Farias <rubensf@google.com>
  - d94f8a8ba888d384686a7cd74d8a9d0795ba4b6d Small tweaks to appease internal import checks. (#346) by Rubens Farias <rubensf@google.com>
  - f9d52cdef1c3aa8612d9c3ed7a0b65f96e55d870 Catch another pool check (#347) by Rubens Farias <rubensf@google.com>
  - ead1458eda2b7c756138429121a3e22bd5c9aa5a Preserving symlink or not can be configured from Command.... by Yoshisato Yanagisawa <yoshisato.yanagisawa@gmail.com>
  - f831c118b9c9e1dd3e857fbe43da7c992c91b20f make (*Chunker).Reset returns error (#348) by Takuto Ikuta <tikuta@google.com>
  - 1a7d2a4198fa0eb8515593b5259f495cdacf75ab [cas] Require PathSpec.Path to be absolute (#345) by nodirg <56001730+nodirg@users.noreply.github.com>
  - dd2d3976ed7c6482f361de7d24e24d5a8683c56c Upgrade zstdpool-syncpool for DecoderWrapper.Close bugfix... by Mostyn Bramley-Moore <mostyn@antipode.se>
  - 8544bdc0f3112900675a72decdb7a978cece0be7 [cas] Clean PathSpec.Path (#352) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 882e3342509eb038dc04bf44581e5085c0320d16 [cas] Add UploadResult.Digest() (#340) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 5d4d813411299a285f113cb541f2c5750746cdfa chunker: remove unused field from Chunker (#355) by Takuto Ikuta <tikuta@google.com>
  - a5af2d4316599a3fea87a459290e5f88a022a43c Add return value names in singleflightcache (#356) by nodirg <56001730+nodirg@users.noreply.github.com>
  - e7ea26b93b496d4d30e98390682faf6f69f84cb7 [cas] Fix joinFilePathsFast (#361) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 5a8daf747858747b2bf6bdd59e7b07dedc17a244 [cas] Refactor code (#359) by nodirg <56001730+nodirg@users.noreply.github.com>
  - dd6c290b2ce791f3a3e56583a9632f7b538e8a05 [cas] Simplify UploadResult.Digest() signature (#362) by nodirg <56001730+nodirg@users.noreply.github.com>
  - 05222e7e8939959878a5798e51cf52e911feafaf Fix lint (#357) by nodirg <56001730+nodirg@users.noreply.github.com>
  - b2689fabc306d2cd20356f30b7f99eec445d9212 [cas] Refactor Digest() (#363) by nodirg <56001730+nodirg@users.noreply.github.com>
  - c672e5baca9280d181c7e2e87b3d9afd01650893 [cas] Add PathSpec.Allowlist (#360) by nodirg <56001730+nodirg@users.noreply.github.com>
  - f9e6595d5634ac4d4221ba6dca9f5c2e64114b3f add size check (#365) by Takuto Ikuta <tikuta@google.com>
  - 3d0cf1be08dd52d77204ad1be7e70fdd1e48c206 Revert "Add useful error message for uploading files. (#3... by Rubens Farias <rubensf@google.com>
  - 3ddc89f3e2b39308101060b3eeaa10a919cae13e "Wrap" gRPC error codes. (#367) by Rubens Farias <rubensf@google.com>
  - d965bf95d0af9d88d90e9723aee6b332dc3ef93e cas: fix deadlock (#368) by Takuto Ikuta <tikuta@google.com>
  - 3c4ce9170b6c5a5d64bcc4feecc5bcdf5dd1f101 allow to use streamBufSize larger than 32KiB (#369) by Takuto Ikuta <tikuta@google.com>
  - 21d6adc44e550f7aa5c4e9b3fedab838920a9632 use semaphore for large file upload (#370) by Takuto Ikuta <tikuta@google.com>
  - 1cec173a5bf76c02f435050d6fc9a02e1ccea637 update remote-apis (#371) by Takuto Ikuta <tikuta@google.com>
  - e96eb06339fb616167ee02535ab54d9c3a382232 add more log around upload (#372) by Takuto Ikuta <tikuta@google.com>
  - 3f34e744d83161ddcb602121b45b0b33185a36c5 Make glog import consistent. (#373) by Rubens Farias <rubensf@google.com>
  - 3db822c86088434a4d2d53ec2d3b889b9f8cf331 Remove typos (#374) by Rubens Farias <rubensf@google.com>

PiperOrigin-RevId: 390319167
@cpsauer
Copy link
Contributor

cpsauer commented Mar 9, 2022

Hey all! We ended up building something pretty sweet to fix this.

The rule creates AARs that contain the transitive closure of their dependencies. It also crawls the graph to auto-generate POM files with dependencies, and lets you deploy to maven local or upload to a remote maven repository, including those of your own making, like S3 buckets. It also works with ProGuard on the AAR itself, plays nice with Bazel platforms, and provides control over resource exporting.

If enough people want this, too, we'll work on open sourcing (or contributing!) it. Maybe rules_jvm_external would be the most natural place. Say something if you want that.

@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jun 13, 2023
@cpsauer
Copy link
Contributor

cpsauer commented Jun 14, 2023

@bazelbuild/triage, this issue is definitely still relevant for all the folks tracking.

(@bazelbuild/triage didn't seem to create a tag last I tried (no link) so I'm going to also tag @sgowroji and @Pavank1992 manually. Please coach me if you'd have preferred otherwise--and maybe update the bot's instructions)

@Pavank1992 Pavank1992 added not stale Issues or PRs that are inactive but not considered stale and removed stale Issues or PRs that are stale (no activity for 30 days) labels Jun 14, 2023
@nkoroste
Copy link
Contributor

Hey all! We ended up building something pretty sweet to fix this.

The rule creates AARs that contain the transitive closure of their dependencies. It also crawls the graph to auto-generate POM files with dependencies, and lets you deploy to maven local or upload to a remote maven repository, including those of your own making, like S3 buckets. It also works with ProGuard on the AAR itself, plays nice with Bazel platforms, and provides control over resource exporting.

If enough people want this, too, we'll work on open sourcing (or contributing!) it. Maybe rules_jvm_external would be the most natural place. Say something if you want that.

@cpsauer did you end up open-sourcing your solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not stale Issues or PRs that are inactive but not considered stale P2 We'll consider working on this in future. (Assignee optional) team-Android Issues for Android team type: feature request
Projects
None yet
Development

No branches or pull requests