Repository: bazelbuild/bazel
Issue: androidndk doesn't register toolchains
Labels: P2
team-Android

### Description of the problem / feature request:

`@androidndk` toolchains/compiler are not registered, and thus skipped from toolchain resolution.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Clone https://github.com/steeve/rules_go_bazel_toolchains and run following command
```
$ bazel build --config=android_arm64 //:main_cc
```

When removing the `--platforms` from the bazelrc (thus removing constraints), the build succeeds.

### What operating system are you running Bazel on?

macos

### What's the output of `bazel info release`?

`release 0.28.0`

###  Have you found anything relevant by searching the web?

https://github.com/bazelbuild/rules_go/issues/2089#issuecomment-510668841

### Any other information, logs, or outputs that you want to share?

All is at https://github.com/bazelbuild/rules_go/issues/2089#issuecomment-510663447

Issue comments:
I got it to work by adding the following in a custom build file:
```
toolchain(
    name = "android",
    target_compatible_with = [
        "@bazel_tools//platforms:android",
        "@bazel_tools//platforms:aarch64",
    ],
    toolchain = "@androidndk//:aarch64-linux-android-clang7.0.2-libcpp",
    toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
)
```

And then adding:
```
--extra_toolchains=//:android
```
The work to migrate the Android NDK rule to toolchains / platforms is ongoing. It may not happen for the native version of `android_ndk_repository` but will happen for the Starlark version.
@ahumesky the problem though is that crosstool_top is ignored when platforms are used it seems (in this case the platform is constraining on `android` and `aarch64`).
I took a stab at this: https://github.com/bazelbuild/bazel/pull/8918

Pull request review comments:
Why can't the existing android_ndk_repository rule handle this directly?
How do you register toolchains from a native RepositoryFunction? 
There's no direct native equivalent to `register_toolchains`, but it wouldn't be hard to add. I feel like the improved usability would be helpful (but this could be in a separate change).
nit: "Registering these toolchains tells Bazel to look for them in ..."
	
hmm, should we throw an error instead of defaulting to x86_64? e.g. if someone tries to select mips, or has a typo?

Patch that resolved the issue:
diff --git a/site/docs/android-ndk.md b/site/docs/android-ndk.md
index b75ca67d11e0de..d9fa1257388dec 100644
--- a/site/docs/android-ndk.md
+++ b/site/docs/android-ndk.md
@@ -255,6 +255,77 @@ cc_library(
 )
 ```
 
+## Integration with platforms and toolchains
+
+Bazel's configuration model is moving towards 
+[platforms](https://docs.bazel.build/versions/master/platforms.html) and 
+[toolchains](https://docs.bazel.build/versions/master/toolchains.html). If your
+build uses the `--platforms` flag to select for the architecture or operating system
+to build for, you will need to pass the `--extra_toolchains` flag to Bazel in
+order to use the NDK.
+
+For example, to integrate with the `android_arm64_cgo` toolchain provided by
+the Go rules, pass `--extra_toolchains=@androidndk//:all` in addition to the
+`--platforms` flag.
+
+```
+bazel build //my/cc:lib \
+  --platforms=@io_bazel_rules_go//go/toolchain:android_arm64_cgo \
+  --extra_toolchains=@androidndk//:all
+```
+
+You can also register them directly in the `WORKSPACE` file:
+
+```python
+android_ndk_repository(name = "androidndk")
+register_toolchains("@androidndk//:all")
+```
+
+By registering these toolchains, you're telling Bazel to look for these in the NDK BUILD file 
+(for NDK 20) when resolving architecture and operating system constraints:
+
+```python
+toolchain(
+  name = "x86-clang8.0.7-libcpp_toolchain",
+  toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
+  target_compatible_with = [
+      "@bazel_tools//platforms:android", 
+      "@bazel_tools//platforms:x86_32"
+  ],
+  toolchain = "@androidndk//:x86-clang8.0.7-libcpp",
+)
+
+toolchain(
+  name = "x86_64-clang8.0.7-libcpp_toolchain",
+  toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
+  target_compatible_with = [
+      "@bazel_tools//platforms:android", 
+      "@bazel_tools//platforms:x86_64"
+  ],
+  toolchain = "@androidndk//:x86_64-clang8.0.7-libcpp",
+)
+
+toolchain(
+  name = "arm-linux-androideabi-clang8.0.7-v7a-libcpp_toolchain",
+  toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
+  target_compatible_with = [
+      "@bazel_tools//platforms:android", 
+      "@bazel_tools//platforms:arm"
+  ],
+  toolchain = "@androidndk//:arm-linux-androideabi-clang8.0.7-v7a-libcpp",
+)
+
+toolchain(
+  name = "aarch64-linux-android-clang8.0.7-libcpp_toolchain",
+  toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
+  target_compatible_with = [
+      "@bazel_tools//platforms:android", 
+      "@bazel_tools//platforms:aarch64"
+  ],
+  toolchain = "@androidndk//:aarch64-linux-android-clang8.0.7-libcpp",
+)
+```
+
 ## How it works: introducing Android configuration transitions
 
 The `android_binary` rule can explicitly ask Bazel to build its dependencies in
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
index 375104ed4699ff..2629b38f05a3b4 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/AndroidNdkRepositoryFunction.java
@@ -201,6 +201,7 @@ private static String createCcToolchainRule(
     return ccToolchainTemplate
         .replace("%toolchainName%", toolchain.getToolchainIdentifier())
         .replace("%cpu%", toolchain.getTargetCpu())
+        .replace("%platform_cpu%", getPlatformCpuLabel(toolchain.getTargetCpu()))
         .replace("%compiler%", toolchain.getCompiler())
         .replace("%version%", version)
         .replace("%dynamicRuntimeLibs%", toolchain.getDynamicRuntimesFilegroup())
@@ -209,6 +210,17 @@ private static String createCcToolchainRule(
         .replace("%toolchainFileGlobs%", toolchainFileGlobs.toString().trim());
   }
 
+  private static String getPlatformCpuLabel(String targetCpu) {
+    // Create a mapping of CcToolchain CPU values to platform arch constraint values
+    // in @bazel_tools//platforms
+    switch (targetCpu) {
+      case "x86": return "x86_32";
+      case "armeabi-v7a": return "arm";
+      case "arm64-v8a": return "aarch64";
+      default: return "x86_64";
+    }
+  }
+
   private static String getTemplate(String templateFile) {
     try {
       return ResourceFileLoader.loadResource(AndroidNdkRepositoryFunction.class, templateFile);
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_ndk_cc_toolchain_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_ndk_cc_toolchain_template.txt
index d9bd1521568b41..d622f0c9626280 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_ndk_cc_toolchain_template.txt
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/android/android_ndk_cc_toolchain_template.txt
@@ -26,6 +26,16 @@ cc_toolchain_config(
     version = "%version%",
 )
 
+toolchain(
+    name = "%toolchainName%_toolchain",
+    target_compatible_with = [
+        "@bazel_tools//platforms:android",
+        "@bazel_tools//platforms:%platform_cpu%",
+    ],
+    toolchain = "@androidndk//:%toolchainName%",
+    toolchain_type = "@bazel_tools//tools/cpp:toolchain_type",
+)
+
 filegroup(
     name = "%toolchainName%-all_files",
     srcs = glob(["ndk/toolchains/%toolchainDirectory%/**"]) + glob([


Test patch:
diff --git a/src/test/shell/bazel/android/android_ndk_integration_test.sh b/src/test/shell/bazel/android/android_ndk_integration_test.sh
index c8e85e7ee304a4..2172ed6c54aa85 100755
--- a/src/test/shell/bazel/android/android_ndk_integration_test.sh
+++ b/src/test/shell/bazel/android/android_ndk_integration_test.sh
@@ -400,6 +400,44 @@ EOF
     --host_crosstool_top=@bazel_tools//tools/cpp:toolchain
 }
 
+function test_platforms_and_toolchains() {
+  create_new_workspace
+  setup_android_ndk_support
+  cat > BUILD <<EOF
+cc_binary(
+    name = "foo",
+    srcs = ["foo.cc"],
+    linkopts = ["-ldl", "-lm"],
+)
+
+platform(
+    name = 'android_arm',
+    constraint_values = ['@bazel_tools//platforms:arm', '@bazel_tools//platforms:android'],
+    visibility = ['//visibility:public'])
+EOF
+  cat > foo.cc <<EOF
+#include <string>
+#include <jni.h>
+#include <android/log.h>
+#include <cstdio>
+#include <iostream>
+
+using namespace std;
+int main(){
+  string foo = "foo";
+  string bar = "bar";
+  string foobar = foo + bar;
+  return 0;
+}
+EOF
+  assert_build //:foo \
+    --cpu=armeabi-v7a \
+    --crosstool_top=@androidndk//:toolchain-libcpp \
+    --host_crosstool_top=@bazel_tools//tools/cpp:toolchain \
+    --platforms=//:android_arm \
+    --extra_toolchains=@androidndk//:all
+}
+
 function test_crosstool_libcpp_with_multiarch() {
   create_new_workspace
   setup_android_sdk_support