Bazel: Support Android NDK 16 in Bazel

[thunderfyc]

At this moment, build with NDK 16 will cause fatal errors that "stdlib.h" "wchar.h", etc could not found,

[dajensen]

I encountered this as well. Didn't initially expect the NDK version to be the cause of those errors, and wasted time trying to hunt down what was wrong with my NDK path. Turns out it was just fine, but I needed to use an older version.

[aj-michael]

@ahumesky @jin @dkelmer

[aj-michael]

@dajensen , sorry to hear about your wasted time. Bazel should have emitted a warning that NDK 16 was unsupported. Perhaps we should make that warning an error.

[aselle]

Any eta on when NDK 16 will be supported @aj-michael?

[angerson]

@dajensen For what it's worth, the warning isn't very helpful:

WARNING: The major revision of the Android NDK referenced by android_ndk_repository rule 
'androidndk' is 16. The major revisions supported by Bazel are [10, 11, 12, 13, 14]. 
Defaulting to revision 14.

The warning text doesn't mean much to me, especially because the exact same message appeared with NDK 15 and the build still worked. "Defaulting to revision 14" actually indicates that Bazel will pretend whatever NDK installed is r14, but it seems like the message says "I'll change your version to rev 14 for you to avoid this problem". I spent ~4 hours trying to understand why the includes weren't compiling, since it looked like Bazel was warning a non-issue.

Since Bazel lacks support for the latest NDK in a world where installing old NDKs is not intuitive, and the failures are misleading, I think this has impacted TF quite a bit, as new users haven't been able to follow the demo instructions for the newly released TF Lite for a couple weeks. There's a lot of confusion in the issues referenced above, and many Stack Overflow questions related to this same problem with the NDK (users asking why important headers are missing, etc.) We weren't able to catch the issue sooner because the importance of the NDK version is so low-profile.

I think most of this could be alleviated with a much stronger NDK warning -- an Error that explains what's going on would be really helpful, e.g.:

ERROR: The major revision of the Android NDK referenced by android_ndk_repository rule
'androidndk' is 16. The major revisions supported by Bazel are [10, 11, 12, 13, 14].
Please download a supported NDK version.
See github.com/bazelbuild/bazel/issues/XXXX for details.

Or if it stays as a warning:

WARNING: The major revision of the Android NDK referenced by android_ndk_repository rule
'androidndk' is 16. The major revisions supported by Bazel are [10, 11, 12, 13, 14].
Bazel will attempt to treat r16 as if it were r14. This may cause compilation and linkage problems.
Please download a supported NDK version.
See github.com/bazelbuild/bazel/issues/XXXX for details.

NDK 16 support would be awesome too, but this would alleviate future incompatibilities. I think addressing this somehow should be higher priority than P2.

In the meantime, I'm going to update our build instructions to work around the NDK trouble.

(May be interesting for @martinwicke @gunan @petewarden)

[ahumesky]

We can certainly update the warning text, your 2nd suggestion sounds good. We made this a warning instead of an error on the reasoning that future NDK versions would be backwards compatible with the crosstool, but that hasn't always worked out.

We've been focused on getting our android testing support working in bazel (robolectric testing, emulator testing), so we haven't had much time to work on NDK integration. I'll see about getting this higher priority.

[steeve]

Note that support was not that hard when we updated for gomobile: https://github.com/golang/mobile/commit/3ef91fec25dd7d4bdd8075088e44cbf236a84e46#diff-bb8d1c5d57784117597766608f27e20c

That is, just a matter for properly setting isystem, sysroot and defining __ANDROID_API__ to the proper version.

[photex]

Seeing as ndk16 is apparently what you'll be getting with a fresh android sdk install and using the sdkmanager to install ndk-bundle it would be great to have this resolved sooner, or perhaps more information about the issue in the getting started docs. :D

[jin]

This is being worked on. Please stay tuned here for updates.

[jin]

Hi everyone, NDK r16 support has been merged into master. It's not in a release yet, so you will need to build Bazel from master to use it. Please open a new issue if you stumble upon any bugs or have a feature request. Thanks!

[zh794390558]

@jin which NDK version does tf.contrib.makefile support now ?

[Kae10]

@jin which NDK version does tf.contrib.makefile support now ?

Now support NDK r16. But I encountered same issue when I tried to execute "compile_android_protobuf.sh" at this page: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/contrib/makefile/README.md

Is there any reference or solution about it?