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

Support Android NDK 16 in Bazel #4068

Closed
thunderfyc opened this issue Nov 10, 2017 · 12 comments
Closed

Support Android NDK 16 in Bazel #4068

thunderfyc opened this issue Nov 10, 2017 · 12 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) type: feature request

Comments

@thunderfyc
Copy link

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

@aj-michael aj-michael added category: rules > android type: feature request P2 We'll consider working on this in future. (Assignee optional) labels Nov 10, 2017
@dajensen
Copy link

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
Copy link
Contributor

@ahumesky @jin @dkelmer

@aj-michael
Copy link
Contributor

@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
Copy link

aselle commented Nov 28, 2017

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

@angerson
Copy link
Contributor

angerson commented Nov 28, 2017

@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
Copy link
Contributor

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.

sb2nov pushed a commit to sb2nov/tensorflow that referenced this issue Dec 1, 2017
sb2nov pushed a commit to sb2nov/tensorflow that referenced this issue Dec 1, 2017
caisq pushed a commit to caisq/tensorflow that referenced this issue Dec 7, 2017
This change teaches the configure script how to search for Android NDK
and SDK installations and create new WORKSPACE rules pointing to them.
It also refactors many similar loop-over-user-input functions into using
a reusable method (not the more complex ones).

Specifying an SDK directory will further query for the available SDK API
levels and build tools versions, but it won't perform any compatibility
checks.

Like other settings, every android-related setting can be set beforehand
via an env param. The script will not ask for any Android settings if
there are already any android repository rules in the WORKSPACE.

The script will emit a warning if using an NDK version newer than 14 due
to bazelbuild/bazel#4068.

PiperOrigin-RevId: 177989785
@steeve
Copy link
Contributor

steeve commented Dec 20, 2017

Note that support was not that hard when we updated for gomobile: golang/mobile@3ef91fe#diff-bb8d1c5d57784117597766608f27e20c

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

@photex
Copy link

photex commented Jan 2, 2018

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 jin self-assigned this Jan 2, 2018
@jin jin added P1 I'll work on this now. (Assignee required) and removed P2 We'll consider working on this in future. (Assignee optional) labels Jan 2, 2018
bazel-io pushed a commit that referenced this issue Feb 7, 2018
Suggestion from @angersson on GitHub: #4068 (comment)

GITHUB: #4068
RELNOTES: Improved clarity of warning message for unsupported NDK revisions.
PiperOrigin-RevId: 184852316
@jin
Copy link
Member

jin commented Feb 17, 2018

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

bazel-io pushed a commit that referenced this issue Mar 2, 2018
NDK15 changelog:
https://android.googlesource.com/platform/ndk/+/ndk-r15-release/CHANGELOG.md

Notable changes for this CL:

- Clang has been updated to v5.0.300080.
- android-9 is not supported anymore. Anything lower than android-14 will default to android-14.
- NDK sysroot has been changed to %ndk%/sysroot
- Requires additional compilation flags because the headers are now unified in a single location across archs and API levels

Also removed NDK 10 checks in integration tests because it is old and we don't test against that anymore.

Fixes #3137
Paves the way for r16: #4068

RELNOTES[NEW]: Added Android NDK r15 support, including compatibility with Unified Headers.

PiperOrigin-RevId: 187648715
@jin
Copy link
Member

jin commented Mar 2, 2018

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
Copy link

zh794390558 commented Sep 21, 2018

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

@Kae10
Copy link

Kae10 commented Oct 11, 2018

@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?

luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    Changelog: https://android.googlesource.com/platform/ndk/+/ndk-release-r16/CHANGELOG.md

    > The deprecated headers have been removed. Unified Headers are now simply The Headers.

    Support for this was added in r15.

    > libc++ is out of beta and is now the preferred STL in the NDK. Starting in r17, libc++ is the default STL for CMake and standalone toolchains. If you manually selected a different STL, we strongly encourage you to move to libc++. For more details, see this blog post.

    We bind the default `//external:android/crosstool` in AndroidNdkRepositoryRule before the repository function is evaluated, so changing the default STL will affect the older NDK revisions too. Until we have a new mechanism to specify the default crosstool, users will need to use `--android_crosstool_top=@androidndk//:toolchain-libcpp` to use `libc++`.

    > GCC, armeabi, mips, mip64 are deprecated but still buildable/useable.

    We can remove support for these in the r17 configuration.

    There are no other actionable differences between r15 and r16, so we can reuse the r15 configuration for r16.

    RELNOTES: Added Android NDK r16 support. Use --cxxopt='-std=c++11` compile with the C++11 standard, and `--android_crosstool_top=@androidndk//:toolchain-libcpp` to use the `libc++` STL.

    Fixes bazelbuild/bazel#4068

    PiperOrigin-RevId: 187664390
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
    Suggestion from @angersson on GitHub: bazelbuild/bazel#4068 (comment)

    GITHUB: bazelbuild/bazel#4068
    RELNOTES: Improved clarity of warning message for unsupported NDK revisions.
    PiperOrigin-RevId: 184852316
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) type: feature request
Projects
None yet
Development

No branches or pull requests