bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Support for Android NDK r22, r23, r24, r25

Open cpsauer opened this issue 3 years ago • 37 comments

Happened to see that the Android NDK r22 had been released. Creating an issue so folks can track bazel support for the latest NDK versions!

Update: r23, r24 now also released. Since it looks like Google's Bazel<->Android support is falling behind a bit, maybe just adding support for the latest would be easier and good enough?

For reference: Things were last updated for r21. Issue. Resolving PR.

cpsauer avatar Jan 23 '21 02:01 cpsauer

Just tried building with NDK 22, it failed with this:

In file included from /private/var/tmp/_bazel_steeve/42f5ad0f9796dcb2aba57da250d2fe89/sandbox/darwin-sandbox/1366/execroot/xxx/external/androidndk/ndk/sources/cxx-stl/llvm-libc++/include/stdlib.h:97:
/private/var/tmp/_bazel_steeve/42f5ad0f9796dcb2aba57da250d2fe89/sandbox/darwin-sandbox/1366/execroot/xxx/external/androidndk/ndk/sources/android/support/include/stdlib.h:31:15: fatal error: 'stdlib.h' file not found

At ndk/sources/android/support/include/stdlib.h:31:15 we find:

#include_next <stdlib.h>

steeve avatar Feb 23 '21 15:02 steeve

Also, #13191 Layout change appears to break bazel outright

oakad avatar Mar 25 '21 02:03 oakad

One thing I ran across that might raise the priority of this--and supporting newer API levels and NDKs more generally: Google Play is requiring API level 30, starting in August.

cpsauer avatar Mar 29 '21 23:03 cpsauer

For the record, here is the Build System Maintainers Guide for r22.

steeve avatar Mar 30 '21 10:03 steeve

Considering how big and important Android platform is, Bazel should assign some more priority to it.

The people are suffering with cmake here, for god's sake. :-)

oakad avatar Mar 31 '21 06:03 oakad

Considering how big and important Android platform is, Bazel should assign some more priority to it.

The bazel team is like you, me and any other team out there: flooded with tasks and constrained by time. If not support, let's at least show empathy. If you want to help, you could try taking a jab at this.

The people are suffering with cmake here, for god's sake. :-)

This one is bad though.

steeve avatar Mar 31 '21 11:03 steeve

The rules in question are in the older java part of Bazel. Similar c++ stuff was ported to starlark. Not something to undertake on one's own initiative (because effort is big and chances of merge are nil).

On the other hand, Bazel is a Google product and Android is also a Google product. Don't think it's somehow unreasonable to expect a bit more synergy between them.

oakad avatar Apr 01 '21 04:04 oakad

My second ever PR in the bazel core was adding support for NDK r21 (albeit probably a lot easier), so I tend to disagree. Not to say it'll be easy, but if somebody were to do it, I'm sure they would be glad and would gladly merge it. As they did with mine.

steeve avatar Apr 01 '21 08:04 steeve

Would adding support for NDK r22 be more complex than for r21? If not, it would be a fairly simple PR right? Or are there changes affecting Bazel?

EwoutH avatar Apr 20 '21 08:04 EwoutH

Would adding support for NDK r22 be more complex than for r21? If not, it would be a fairly simple PR right? Or are there changes affecting Bazel?

I'm not 100% sure, but it feels like the changes are more substantial than to r21.

steeve avatar Apr 20 '21 09:04 steeve

@ahumesky, since you reviewed the PR for the NDK r21, would you be able to help out with this issue?

EwoutH avatar Apr 28 '21 09:04 EwoutH

@cpsauer re: "Google Play is requiring API level 30, starting in August", I believe that ndk r21 supports API level 30, and it's an LTS release, so it should be supported for some time. I'll look at #13421

@EwoutH @steeve I don't know off-hand what would be involved, but it sounds like it would be closer to https://github.com/bazelbuild/bazel/commit/00e29b7cd80df6a2762bc8b4b035f5a99466f5f6 than https://github.com/bazelbuild/bazel/pull/11872

ahumesky avatar Jun 18 '21 22:06 ahumesky

[Updated to reflect the release of r23, which is LTS.]

cpsauer avatar Sep 01 '21 05:09 cpsauer

I started working on making NDK r23 work couple days ago, it seems there are at least 2 major issues

  1. deprecation of ${NDK_ROOT}/{platforms,sysroot}
  2. removal of GNU toolchain

I managed to make building a minimal hello world work on macOS and ubuntu with https://github.com/freedomtan/bazel/tree/ndk_r23_hacks

With --copt=-nostdinc++, I could be TFLite //tensorflow/lite/tools/benchmark:benchmark_model and //tensorflow/lite/examples/label_image:label_image for android_arm64.

bazel build --config android_arm64 //tensorflow/lite/tools/benchmark:benchmark_model   --copt=-nostdinc++

Dunno how to get rid of the need of -nostdinc++ yet, but basically it works. I'll clean up and send a PR later.

freedomtan avatar Sep 12 '21 23:09 freedomtan

Thanks @freedomtan, I'm happy to review a pull request for this if you're able to send one out.

ahumesky avatar Oct 04 '21 20:10 ahumesky

I have an update to share: I figured out a way to remove the need for --copt=-nostdinc++ when compiling Tensorflow. Basically there are a number of -isystem flags that are added to the crosstool, e.g.: https://github.com/bazelbuild/bazel/blob/b4c637c5e3af145295ad3f4ca732c228ab6a88cf/src/main/java/com/google/devtools/build/lib/bazel/rules/android/ndkcrosstools/r19/AndroidNdkCrosstoolsR19.java#L99-L100

Through the internal discussion I mentioned on #14077, I learned that with more recent versions of clang, this actually appears to interfere with clang's automatic detection of includes. So at first glance, it would seem like a simple thing to just remove these for R23. The problem though is that these same fields are used elsewhere to create filegroups for picking up all these includes for sandboxing, and this code is shared across NDK versions: https://github.com/bazelbuild/bazel/blob/09c621e4cf5b968f4c6cdf905ab142d5961f9ddc/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java#L196-L200

It's a bit of a confusing mess to try to untangle build file / filegroup generation from crosstool generation, so instead I just generated an NDK build file and cc toolchain configuration file with what we have now, and then took out all the file generation logic for R23 and bundled static versions of the NDK build file and cc toolchain configuration file. Then in AndroidNdkRepositoryFunction, it just copies these bundled files into the symlinked output directory for the NDK. Now it's a simpler matter to edit the cc toolchain configs and filegroups. We've been wanting to move in this direction anyway for a while, especially since the NDK has gotten simpler (now clang-only, fewer STLs, etc).

I can share an in-progress change, which incorporates @freedomtan's #14077. The next steps are to set up bazel's CI with NDK 23, and add and test mac and windows support.

ahumesky avatar Nov 03 '21 18:11 ahumesky

Here are my changes so far: https://github.com/ahumesky/bazel/commit/3de69da0323bb0bd075611a4f12e590dd1204cf9

ahumesky avatar Nov 03 '21 20:11 ahumesky

真TMD无语,拖了一整年也不改,不知道支持一下NDK新版到底是要花多少时间,一个号称跨平台的工具链在跨平台方面做得跟逗你玩一样,这到底还是不是Google

iHuahua avatar Jan 24 '22 11:01 iHuahua

Is there any motion on this? It looks like PR#14077 has stalled.

My team, which uses Bazel, is eagerly waiting NDK r23 so we can use Android 12 APIs.

I'm surprised at how difficult it is to integrate a new Android NDK LTS release to Bazel, which was released, wow, back in August. Maybe ahumesky@3de69da will make upgrades easier by moving the majority of the work into BUILD files instead of Java inside Bazel.

@ahumesky are there any long-term plans to migrate Android repository rules to Starlark? It's a big chunk of work but may be the best solution for #14260.

I guess the issue is that neither the Android NDK team nor the Bazel team at Google officially supports the NDK-on-Bazel toolchain? Maybe AOSP on Bazel will speed things along?

jiawen avatar Feb 27 '22 21:02 jiawen

[Cross-ref: I wonder if the switch to LLD in newer sdks might bring more sane defaults, fixing #10837]

cpsauer avatar Mar 01 '22 05:03 cpsauer

@ahumesky could you give an update on the roadmap of the Android NDK for bazel?

jgavris avatar Mar 01 '22 14:03 jgavris

Any update on this issue? Thanks!

SunXuan90 avatar May 09 '22 07:05 SunXuan90

I see that PR https://github.com/bazelbuild/bazel/pull/14077 also looks a little stuck. This is blocking quite a few cleanups in my team's codebase, simply because on Android we're stuck with an aging clang version.

@cpsauer It seems NDK 24 is also now out: https://developer.android.com/ndk/downloads#r24-downloads

@ahumesky, would you have any update to share on the topic or any guidance on how we can help move this issue along? Thanks 🙏

EDIT: was linking to this issue instead of the open PR

kaliaumem avatar May 16 '22 12:05 kaliaumem

Thanks for the heads! Have changed the title/top comment of the feature request to reflect the release of r24.

cpsauer avatar May 16 '22 22:05 cpsauer

Googlers, is this not an issue internally for Blaze? I'd have guessed offhand that supporting new NDKs with Blaze/Bazel would have been important bc Google's main OS/build tool.

cpsauer avatar May 16 '22 23:05 cpsauer

Hi @ahumesky, is there any update on this?

I understand Everyone is busy and this is a large amount of work.

Bazel has worked great for my team and we'd like to keep using it.

Would just like to understand timelines - I need to decide whether to wait for this fix to land or move off of Bazel. A lot of our changes are blocked behind r21's old build of clang.

How can we help move this forward?

Thanks!

tndhagedorn avatar May 20 '22 15:05 tndhagedorn

@tndhagedorn this is Radhika and I am the PM at Bazel. i'd love to understand your urgency for NDK support.

radvani13 avatar Jul 07 '22 18:07 radvani13

Hey Radhika! I'd just caught up on the Bazel community update video where you introduced yourself. Great to run into you on GitHub!

[I'm not tndhagedorn, but I'm guessing offhand that he's referring to the new language features (and libraries that use them) unlocked by newer versions of clang. Newer NDKs would get C++20 support, for example, in addition to a more consistency LLVM toolchain that I think would solve a variety of things.]

cpsauer avatar Jul 08 '22 01:07 cpsauer

In general, the situation with NDK support is quite bad, and bazel 5 had broke it even further (see #14260).

It came to be that bazel support for iOS is better than for Android; at this rate we may as well simply switch to Apple only and call it a day. :-)

oakad avatar Jul 08 '22 07:07 oakad

@radvani13 for your reference, I wanted to try if I can use ARM's new SVE2 in recently cores to accelerate TensorFlow Lite, which I was a contributor. SVE2 related intrinsics are not supported by clang in older NDKs. As you know, the main build system for TensorFlow is bazel. That's why I hacked to have a somewhat functional NDK r23 PR https://github.com/bazelbuild/bazel/pull/14077.

freedomtan avatar Jul 08 '22 09:07 freedomtan